Commit 98d93c3a authored by Timothy Loh's avatar Timothy Loh Committed by Commit Bot

Don't display pinned Crostini apps if Crostini UI is not allowed

Due to either syncing or using multi-profile, we can get into a state
where synced pin state lists Crostini apps although we shouldn't be
displaying them. This results in weirdness like blank-space icons for
the Terminal, or penguin icons for apps with icons, when these pinned
items are supposed to be hidden. Clicking the shelf icons does nothing
due to https://crrev.com/c/1212373. This CL prevents these broken
pinned apps from appearing.

Bug: 869266
Change-Id: I13ca91b8055cf43003bf1d467ca28161eb289e29
Reviewed-on: https://chromium-review.googlesource.com/1215532Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590196}
parent 417ad521
...@@ -37,6 +37,8 @@ ...@@ -37,6 +37,8 @@
#include "base/values.h" #include "base/values.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/chromeos/arc/arc_util.h" #include "chrome/browser/chromeos/arc/arc_util.h"
#include "chrome/browser/chromeos/crostini/crostini_test_helper.h"
#include "chrome/browser/chromeos/crostini/crostini_util.h"
#include "chrome/browser/chromeos/login/demo_mode/demo_session.h" #include "chrome/browser/chromeos/login/demo_mode/demo_session.h"
#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h" #include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
#include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_service.h"
...@@ -919,6 +921,8 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest { ...@@ -919,6 +921,8 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {
result += "Platform_App"; result += "Platform_App";
} else if (app == arc_support_host_->id()) { } else if (app == arc_support_host_->id()) {
result += "Play Store"; result += "Play Store";
} else if (app == kCrostiniTerminalId) {
result += "Terminal";
} else { } else {
bool arc_app_found = false; bool arc_app_found = false;
for (const auto& arc_app : arc_test_.fake_apps()) { for (const auto& arc_app : arc_test_.fake_apps()) {
...@@ -4630,3 +4634,37 @@ TEST_F(ChromeLauncherControllerDemoModeTest, PinnedAppsOffline) { ...@@ -4630,3 +4634,37 @@ TEST_F(ChromeLauncherControllerDemoModeTest, PinnedAppsOffline) {
EXPECT_EQ(AppListControllerDelegate::PIN_EDITABLE, EXPECT_EQ(AppListControllerDelegate::PIN_EDITABLE,
GetPinnableForAppID(online_only_app_id, profile())); GetPinnableForAppID(online_only_app_id, profile()));
} }
TEST_F(ChromeLauncherControllerTest, CrostiniTerminalPinUnpin) {
InitLauncherController();
const std::string app_id = kCrostiniTerminalId;
EXPECT_FALSE(launcher_controller_->IsAppPinned(app_id));
// Load pinned Terminal from prefs without Crostini UI being allowed
base::ListValue value;
InsertPrefValue(&value, 0, app_id);
StartPrefSyncServiceForPins(value);
EXPECT_EQ("Back, AppList, Chrome", GetPinnedAppStatus());
// Reload after allowing Crostini UI
crostini::CrostiniTestHelper test_helper(profile());
StopPrefSyncService();
StartPrefSyncServiceForPins(value);
EXPECT_EQ("Back, AppList, Chrome, Terminal", GetPinnedAppStatus());
EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[3].type);
EXPECT_EQ(ash::STATUS_CLOSED, model_->items()[3].status);
EXPECT_TRUE(launcher_controller_->IsAppPinned(app_id));
// Unpin the Terminal
launcher_controller_->UnpinAppWithID(app_id);
EXPECT_EQ(3, model_->item_count());
EXPECT_FALSE(launcher_controller_->IsAppPinned(app_id));
// Pin Terminal again.
launcher_controller_->PinAppWithID(app_id);
EXPECT_EQ("Back, AppList, Chrome, Terminal", GetPinnedAppStatus());
EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[3].type);
EXPECT_EQ(ash::STATUS_CLOSED, model_->items()[3].status);
EXPECT_TRUE(launcher_controller_->IsAppPinned(app_id));
}
...@@ -173,8 +173,10 @@ bool LauncherControllerHelper::IsValidIDForCurrentUser( ...@@ -173,8 +173,10 @@ bool LauncherControllerHelper::IsValidIDForCurrentUser(
crostini::CrostiniRegistryService* registry_service = crostini::CrostiniRegistryService* registry_service =
crostini::CrostiniRegistryServiceFactory::GetForProfile(profile_); crostini::CrostiniRegistryServiceFactory::GetForProfile(profile_);
if (registry_service && registry_service->IsCrostiniShelfAppId(id)) if (registry_service && registry_service->IsCrostiniShelfAppId(id)) {
return registry_service->GetRegistration(id).has_value(); return IsCrostiniUIAllowedForProfile(profile_) &&
registry_service->GetRegistration(id).has_value();
}
if (app_list::IsInternalApp(id)) if (app_list::IsInternalApp(id))
return true; return true;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment