Commit d568297c authored by James Cook's avatar James Cook Committed by Commit Bot

lacros: Pin Lacros icon to shelf when the flag is set

Make it behave like a secondary browser icon:
* Always pin it when the flag is set, similar to policy-pinned apps
* Default position is to the right of the browser icon
* Context menu does not contain an "Unpin" item

Also change the app name from "LaCrOS" to "Lacros", since we've settled
on the simpler capitalization and I'm adding a test that references
the name.

Bug: 1127153
Test: added to unit_tests
Change-Id: I08ba806d481580988f2be284d8d1ccb6210ea8ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2405737
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: default avatarNancy Wang <nancylingwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806241}
parent 9f3a9783
...@@ -32,7 +32,7 @@ apps::mojom::AppPtr LacrosApps::GetLacrosApp(bool is_ready) { ...@@ -32,7 +32,7 @@ apps::mojom::AppPtr LacrosApps::GetLacrosApp(bool is_ready) {
apps::mojom::AppPtr app = apps::PublisherBase::MakeApp( apps::mojom::AppPtr app = apps::PublisherBase::MakeApp(
apps::mojom::AppType::kLacros, extension_misc::kLacrosAppId, apps::mojom::AppType::kLacros, extension_misc::kLacrosAppId,
apps::mojom::Readiness::kReady, apps::mojom::Readiness::kReady,
"LaCrOS", // TODO(jamescook): Localized name. "Lacros", // TODO(jamescook): Localized name.
apps::mojom::InstallSource::kSystem); apps::mojom::InstallSource::kSystem);
app->icon_key = NewIconKey(is_ready ? State::kReady : State::kLoading); app->icon_key = NewIconKey(is_ready ? State::kReady : State::kLoading);
app->searchable = apps::mojom::OptionalBool::kTrue; app->searchable = apps::mojom::OptionalBool::kTrue;
...@@ -71,9 +71,12 @@ apps::mojom::IconKeyPtr LacrosApps::NewIconKey(State state) { ...@@ -71,9 +71,12 @@ apps::mojom::IconKeyPtr LacrosApps::NewIconKey(State state) {
void LacrosApps::Connect( void LacrosApps::Connect(
mojo::PendingRemote<apps::mojom::Subscriber> subscriber_remote, mojo::PendingRemote<apps::mojom::Subscriber> subscriber_remote,
apps::mojom::ConnectOptionsPtr opts) { apps::mojom::ConnectOptionsPtr opts) {
bool is_ready = crosapi::BrowserManager::Get()->IsReady(); auto* browser_manager = crosapi::BrowserManager::Get();
if (!is_ready) { bool is_ready = true;
crosapi::BrowserManager::Get()->SetLoadCompleteCallback(base::BindOnce( // |browser_manager| may be null in tests. For tests, assume Lacros is ready.
if (browser_manager && !browser_manager->IsReady()) {
is_ready = false;
browser_manager->SetLoadCompleteCallback(base::BindOnce(
&LacrosApps::OnLoadComplete, weak_factory_.GetWeakPtr())); &LacrosApps::OnLoadComplete, weak_factory_.GetWeakPtr()));
} }
std::vector<apps::mojom::AppPtr> apps; std::vector<apps::mojom::AppPtr> apps;
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "ash/public/cpp/app_list/internal_app_id_constants.h" #include "ash/public/cpp/app_list/internal_app_id_constants.h"
#include "ash/public/cpp/ash_pref_names.h" #include "ash/public/cpp/ash_pref_names.h"
#include "base/values.h" #include "base/values.h"
#include "chrome/browser/chromeos/crosapi/browser_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/prefs/pref_service_syncable_util.h" #include "chrome/browser/prefs/pref_service_syncable_util.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
...@@ -610,6 +611,19 @@ std::vector<ash::ShelfID> GetPinnedAppsFromSync( ...@@ -610,6 +611,19 @@ std::vector<ash::ShelfID> GetPinnedAppsFromSync(
InsertPinsAfterChromeAndBeforeFirstPinnedApp( InsertPinsAfterChromeAndBeforeFirstPinnedApp(
helper, syncable_service, GetAppsPinnedByPolicy(helper), &pin_infos); helper, syncable_service, GetAppsPinnedByPolicy(helper), &pin_infos);
// If Lacros is enabled and allowed for this user type, ensure the Lacros icon
// is pinned.
if (chromeos::features::IsLacrosSupportEnabled() &&
crosapi::browser_util::IsLacrosAllowed()) {
syncer::StringOrdinal lacros_position =
syncable_service->GetPinPosition(extension_misc::kLacrosAppId);
if (!lacros_position.IsValid()) {
// If Lacros isn't already pinned, add it to the right of the Chrome icon.
InsertPinsAfterChromeAndBeforeFirstPinnedApp(
helper, syncable_service, {extension_misc::kLacrosAppId}, &pin_infos);
}
}
// Sort pins according their ordinals. // Sort pins according their ordinals.
std::sort(pin_infos.begin(), pin_infos.end(), ComparePinInfo()); std::sort(pin_infos.begin(), pin_infos.end(), ComparePinInfo());
......
...@@ -530,9 +530,11 @@ bool AppServiceShelfContextMenu::ShouldAddPinMenu() { ...@@ -530,9 +530,11 @@ bool AppServiceShelfContextMenu::ShouldAddPinMenu() {
} }
case apps::mojom::AppType::kCrostini: case apps::mojom::AppType::kCrostini:
case apps::mojom::AppType::kExtension: case apps::mojom::AppType::kExtension:
case apps::mojom::AppType::kLacros:
case apps::mojom::AppType::kWeb: case apps::mojom::AppType::kWeb:
return true; return true;
case apps::mojom::AppType::kLacros:
// Lacros behaves like the Chrome browser icon and cannot be unpinned.
return false;
case apps::mojom::AppType::kUnknown: case apps::mojom::AppType::kUnknown:
// Type kUnknown is used for "unregistered" Crostini apps, which do not // Type kUnknown is used for "unregistered" Crostini apps, which do not
// have a .desktop file and can only be closed, not pinned. // have a .desktop file and can only be closed, not pinned.
......
...@@ -1045,6 +1045,23 @@ class ChromeLauncherControllerSplitSettingsSyncTest ...@@ -1045,6 +1045,23 @@ class ChromeLauncherControllerSplitSettingsSyncTest
base::test::ScopedFeatureList feature_list_; base::test::ScopedFeatureList feature_list_;
}; };
// Tests for Lacros integration. Exists as a separate class because the feature
// must be initialized before ChromeLauncherControllerTest::SetUp().
class ChromeLauncherControllerLacrosTest : public ChromeLauncherControllerTest {
public:
ChromeLauncherControllerLacrosTest() {
feature_list_.InitAndEnableFeature(chromeos::features::kLacrosSupport);
}
ChromeLauncherControllerLacrosTest(
const ChromeLauncherControllerLacrosTest&) = delete;
ChromeLauncherControllerLacrosTest& operator=(
const ChromeLauncherControllerLacrosTest&) = delete;
~ChromeLauncherControllerLacrosTest() override = default;
private:
base::test::ScopedFeatureList feature_list_;
};
class ChromeLauncherControllerExtendedShelfTest class ChromeLauncherControllerExtendedShelfTest
: public ChromeLauncherControllerWithArcTest { : public ChromeLauncherControllerWithArcTest {
protected: protected:
...@@ -1364,6 +1381,19 @@ TEST_P(ChromeLauncherControllerSplitSettingsSyncTest, DefaultApps) { ...@@ -1364,6 +1381,19 @@ TEST_P(ChromeLauncherControllerSplitSettingsSyncTest, DefaultApps) {
EXPECT_EQ("Chrome, Gmail, Doc, Youtube", GetPinnedAppStatus()); EXPECT_EQ("Chrome, Gmail, Doc, Youtube", GetPinnedAppStatus());
} }
TEST_P(ChromeLauncherControllerLacrosTest, LacrosPinnedByDefault) {
// Checking to see if Lacros is allowed requires a user.
auto user_manager = std::make_unique<chromeos::FakeChromeUserManager>();
auto* fake_user_manager = user_manager.get();
user_manager::ScopedUserManager scoped_user_manager(std::move(user_manager));
AccountId account_id = AccountId::FromUserEmail("user@example.com");
fake_user_manager->AddUser(account_id);
fake_user_manager->LoginUser(account_id);
InitLauncherController();
EXPECT_EQ("Chrome, Lacros", GetPinnedAppStatus());
}
TEST_P(ChromeLauncherControllerExtendedShelfTest, ExtendedShefDefault) { TEST_P(ChromeLauncherControllerExtendedShelfTest, ExtendedShefDefault) {
base::test::ScopedFeatureList scoped_feature_list; base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeatureWithParameters( scoped_feature_list.InitAndEnableFeatureWithParameters(
...@@ -5036,6 +5066,12 @@ INSTANTIATE_TEST_SUITE_P( ...@@ -5036,6 +5066,12 @@ INSTANTIATE_TEST_SUITE_P(
::testing::Values(std::make_pair(ProviderType::kBookmarkApps, false), ::testing::Values(std::make_pair(ProviderType::kBookmarkApps, false),
std::make_pair(ProviderType::kWebApps, false))); std::make_pair(ProviderType::kWebApps, false)));
INSTANTIATE_TEST_SUITE_P(
All,
ChromeLauncherControllerLacrosTest,
::testing::Values(std::make_pair(ProviderType::kBookmarkApps, false),
std::make_pair(ProviderType::kWebApps, false)));
INSTANTIATE_TEST_SUITE_P( INSTANTIATE_TEST_SUITE_P(
All, All,
ChromeLauncherControllerExtendedShelfTest, ChromeLauncherControllerExtendedShelfTest,
......
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