From 9d2f70d058c892d0e9024bbbf6909d72921fbbef Mon Sep 17 00:00:00 2001 From: Di Wu Date: Mon, 8 Aug 2022 05:47:00 +0000 Subject: [PATCH] [Lacros] Expose `LacrosMode` status in `LacrosInfo` This CL adds the `LacrosMode` in the `LacrosInfo` struct that is exposed by `autotest_private_api` to tast tests. So that tast tests can use `LacrosInfo` to figure out whether it's testing against Lacros Only or Lacros Primary, or any other listed mode. The four supported modes are `Disabled`, `SideBySide`, `Primary` and `Only`. Ultimately, this CL is one of a series of CLs that enable tast tests to test LacrosOnly scenarios. Bug: 1328994,1328851 Change-Id: I29747b3e0c462e50bda560819fd1079cd5908f47 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3783852 Reviewed-by: Ben Wells Reviewed-by: Hidehiko Abe Commit-Queue: Di Wu Cr-Commit-Position: refs/heads/main@{#1032436} --- chrome/browser/ash/crosapi/browser_util.cc | 10 +++ chrome/browser/ash/crosapi/browser_util.h | 22 ++++++ .../ash/crosapi/browser_util_unittest.cc | 72 ++++++++++++++++++- .../autotest_private/autotest_private_api.cc | 18 +++++ .../autotest_private/autotest_private_api.h | 2 + .../extensions/api/autotest_private.idl | 13 +++- 6 files changed, 134 insertions(+), 3 deletions(-) diff --git a/chrome/browser/ash/crosapi/browser_util.cc b/chrome/browser/ash/crosapi/browser_util.cc index b091718b9167c..a451b2903b057 100644 --- a/chrome/browser/ash/crosapi/browser_util.cc +++ b/chrome/browser/ash/crosapi/browser_util.cc @@ -628,6 +628,16 @@ bool IsLacrosPrimaryBrowserForMigration(const user_manager::User* user, return base::FeatureList::IsEnabled(chromeos::features::kLacrosPrimary); } +LacrosMode GetLacrosMode() { + if (!IsAshWebBrowserEnabled()) + return LacrosMode::kOnly; + if (IsLacrosPrimaryBrowser()) + return LacrosMode::kPrimary; + if (IsLacrosEnabled()) + return LacrosMode::kSideBySide; + return LacrosMode::kDisabled; +} + void SetLacrosPrimaryBrowserForTest(absl::optional value) { g_lacros_primary_browser_for_test = value; } diff --git a/chrome/browser/ash/crosapi/browser_util.h b/chrome/browser/ash/crosapi/browser_util.h index d8edb49384442..1bc08e18b6b7e 100644 --- a/chrome/browser/ash/crosapi/browser_util.h +++ b/chrome/browser/ash/crosapi/browser_util.h @@ -97,6 +97,24 @@ enum class MigrationMode { kMove = 1, // Migrate using `MoveMigrator`. }; +// Specifies the mode Lacros is currently running. +// This enum is different from LacrosAvailability in the way that +// it describes the mode Lacros is running at a given point in time +// which can be influenced by multiple factors such as flags, +// while LacrosAvailability describes the policy that dictates how +// Lacros is supposed to be launched. +enum class LacrosMode { + // Indicates that Lacros is disabled. Ash is the only browser. + kDisabled = 0, + // Indicates that Lacros is enabled but Ash browser is the + // primary browser. + kSideBySide = 1, + // Indicates that Lacros is the primary browser. Ash is also available. + kPrimary = 2, + // Indicates that Lacros is the only available browser. + kOnly = 3, +}; + extern const ComponentInfo kLacrosDogfoodCanaryInfo; extern const ComponentInfo kLacrosDogfoodDevInfo; extern const ComponentInfo kLacrosDogfoodBetaInfo; @@ -207,6 +225,10 @@ bool IsLacrosPrimaryBrowser(); bool IsLacrosPrimaryBrowserForMigration(const user_manager::User* user, PolicyInitState policy_init_state); +// Returns the current mode Lacros is running. +// See LacrosMode definition for a full list of modes. +LacrosMode GetLacrosMode(); + // Forces IsLacrosPrimaryBrowser() to return true or false for testing. // Passing absl::nullopt will reset the state. void SetLacrosPrimaryBrowserForTest(absl::optional value); diff --git a/chrome/browser/ash/crosapi/browser_util_unittest.cc b/chrome/browser/ash/crosapi/browser_util_unittest.cc index f8f59c75ae367..3e987a47cb3b1 100644 --- a/chrome/browser/ash/crosapi/browser_util_unittest.cc +++ b/chrome/browser/ash/crosapi/browser_util_unittest.cc @@ -404,7 +404,7 @@ TEST_F(BrowserUtilTest, IsAshWebBrowserDisabledByFlags) { } } -TEST_F(LacrosSupportBrowserUtilTest, LacrosPrimaryBrowserByFlags) { +TEST_F(LacrosSupportBrowserUtilTest, LacrosPrimaryOrOnlyBrowserByFlags) { AddRegularUser("user@test.com"); const user_manager::User* const user = ash::ProfileHelper::Get()->GetUserByProfile(&testing_profile_); @@ -412,6 +412,19 @@ TEST_F(LacrosSupportBrowserUtilTest, LacrosPrimaryBrowserByFlags) { EXPECT_FALSE(browser_util::IsLacrosPrimaryBrowser()); EXPECT_FALSE(browser_util::IsLacrosPrimaryBrowserForMigration( user, browser_util::PolicyInitState::kAfterInit)); + EXPECT_EQ(browser_util::LacrosMode::kDisabled, + browser_util::GetLacrosMode()); + } + + // Just enabling LacrosSupport feature is not enough. + { + base::test::ScopedFeatureList feature_list; + feature_list.InitAndEnableFeature(chromeos::features::kLacrosSupport); + EXPECT_FALSE(browser_util::IsLacrosPrimaryBrowser()); + EXPECT_FALSE(browser_util::IsLacrosPrimaryBrowserForMigration( + user, browser_util::PolicyInitState::kAfterInit)); + EXPECT_EQ(browser_util::LacrosMode::kSideBySide, + browser_util::GetLacrosMode()); } // Just enabling LacrosPrimary feature is not enough. @@ -421,9 +434,11 @@ TEST_F(LacrosSupportBrowserUtilTest, LacrosPrimaryBrowserByFlags) { EXPECT_FALSE(browser_util::IsLacrosPrimaryBrowser()); EXPECT_FALSE(browser_util::IsLacrosPrimaryBrowserForMigration( user, browser_util::PolicyInitState::kAfterInit)); + EXPECT_EQ(browser_util::LacrosMode::kDisabled, + browser_util::GetLacrosMode()); } - // Both LacrosPrimary and LacrosSupport are needed. + // Both LacrosPrimary and LacrosSupport are needed for LacrosPrimary { base::test::ScopedFeatureList feature_list; feature_list.InitWithFeatures({chromeos::features::kLacrosPrimary, @@ -432,6 +447,21 @@ TEST_F(LacrosSupportBrowserUtilTest, LacrosPrimaryBrowserByFlags) { EXPECT_TRUE(browser_util::IsLacrosPrimaryBrowser()); EXPECT_TRUE(browser_util::IsLacrosPrimaryBrowserForMigration( user, browser_util::PolicyInitState::kAfterInit)); + EXPECT_EQ(browser_util::LacrosMode::kPrimary, + browser_util::GetLacrosMode()); + } + + // All LacrosPrimary, LacrosOnly and LacrosSupport are needed for LacrosOnly + { + base::test::ScopedFeatureList feature_list; + feature_list.InitWithFeatures( + {chromeos::features::kLacrosPrimary, chromeos::features::kLacrosSupport, + chromeos::features::kLacrosOnly}, + {}); + EXPECT_TRUE(browser_util::IsLacrosPrimaryBrowser()); + EXPECT_TRUE(browser_util::IsLacrosPrimaryBrowserForMigration( + user, browser_util::PolicyInitState::kAfterInit)); + EXPECT_EQ(browser_util::LacrosMode::kOnly, browser_util::GetLacrosMode()); } } @@ -449,6 +479,7 @@ TEST_F(BrowserUtilTest, LacrosPrimaryBrowser) { EXPECT_TRUE(browser_util::IsLacrosPrimaryBrowser()); EXPECT_TRUE(browser_util::IsLacrosPrimaryBrowserForMigration( user, browser_util::PolicyInitState::kAfterInit)); + EXPECT_EQ(browser_util::LacrosMode::kPrimary, browser_util::GetLacrosMode()); } TEST_F(BrowserUtilTest, LacrosPrimaryBrowserAllowed) { @@ -476,6 +507,8 @@ TEST_F(BrowserUtilTest, ManagedAccountLacrosPrimary) { EXPECT_FALSE(browser_util::IsLacrosPrimaryBrowser()); EXPECT_FALSE(browser_util::IsLacrosPrimaryBrowserForMigration( user, browser_util::PolicyInitState::kAfterInit)); + EXPECT_EQ(browser_util::LacrosMode::kDisabled, + browser_util::GetLacrosMode()); } { @@ -486,6 +519,8 @@ TEST_F(BrowserUtilTest, ManagedAccountLacrosPrimary) { EXPECT_FALSE(browser_util::IsLacrosPrimaryBrowser()); EXPECT_FALSE(browser_util::IsLacrosPrimaryBrowserForMigration( user, browser_util::PolicyInitState::kAfterInit)); + EXPECT_EQ(browser_util::LacrosMode::kSideBySide, + browser_util::GetLacrosMode()); } { @@ -496,6 +531,8 @@ TEST_F(BrowserUtilTest, ManagedAccountLacrosPrimary) { EXPECT_TRUE(browser_util::IsLacrosPrimaryBrowser()); EXPECT_TRUE(browser_util::IsLacrosPrimaryBrowserForMigration( user, browser_util::PolicyInitState::kAfterInit)); + EXPECT_EQ(browser_util::LacrosMode::kPrimary, + browser_util::GetLacrosMode()); } { @@ -506,6 +543,7 @@ TEST_F(BrowserUtilTest, ManagedAccountLacrosPrimary) { EXPECT_TRUE(browser_util::IsLacrosPrimaryBrowser()); EXPECT_TRUE(browser_util::IsLacrosPrimaryBrowserForMigration( user, browser_util::PolicyInitState::kAfterInit)); + EXPECT_EQ(browser_util::LacrosMode::kOnly, browser_util::GetLacrosMode()); } } @@ -997,6 +1035,36 @@ TEST_F(BrowserUtilTest, LacrosGoogleRolloutPrimary) { EXPECT_TRUE(browser_util::IsLacrosPrimaryBrowser()); EXPECT_TRUE(browser_util::IsLacrosPrimaryBrowserForMigration( user, browser_util::PolicyInitState::kAfterInit)); + // What you see here is Finch overrides policy. + // This is due to a special logic only to Googlers. + // See IsAshWebBrowserEnabled() impl for more details. + EXPECT_EQ(browser_util::LacrosMode::kOnly, browser_util::GetLacrosMode()); + EXPECT_FALSE(browser_util::IsAshWebBrowserEnabled()); + EXPECT_FALSE(browser_util::IsAshWebBrowserEnabledForMigration( + user, browser_util::PolicyInitState::kAfterInit)); +} + +TEST_F(BrowserUtilTest, LacrosGoogleRolloutOnly) { + AddRegularUser("user@google.com"); + const user_manager::User* const user = + ash::ProfileHelper::Get()->GetUserByProfile(&testing_profile_); + // Lacros availability is set by policy to only. + ScopedLacrosAvailabilityCache cache(LacrosAvailability::kLacrosOnly); + + // We enable 3 features: LacrosSupport, LacrosPrimary, LacrosOnly + base::test::ScopedFeatureList feature_list; + feature_list.InitWithFeatures( + {chromeos::features::kLacrosSupport, chromeos::features::kLacrosPrimary, + chromeos::features::kLacrosOnly}, + {}); + + // Check that Lacros is allowed, enabled, and set to lacros-only. + EXPECT_TRUE(browser_util::IsLacrosAllowedToBeEnabled()); + EXPECT_TRUE(browser_util::IsLacrosEnabled()); + EXPECT_TRUE(browser_util::IsLacrosPrimaryBrowser()); + EXPECT_TRUE(browser_util::IsLacrosPrimaryBrowserForMigration( + user, browser_util::PolicyInitState::kAfterInit)); + EXPECT_EQ(browser_util::LacrosMode::kOnly, browser_util::GetLacrosMode()); EXPECT_FALSE(browser_util::IsAshWebBrowserEnabled()); EXPECT_FALSE(browser_util::IsAshWebBrowserEnabledForMigration( user, browser_util::PolicyInitState::kAfterInit)); diff --git a/chrome/browser/chromeos/extensions/autotest_private/autotest_private_api.cc b/chrome/browser/chromeos/extensions/autotest_private/autotest_private_api.cc index ef92d68007089..88b887fc6082a 100644 --- a/chrome/browser/chromeos/extensions/autotest_private/autotest_private_api.cc +++ b/chrome/browser/chromeos/extensions/autotest_private/autotest_private_api.cc @@ -1972,6 +1972,22 @@ AutotestPrivateGetLacrosInfoFunction::ToLacrosState( } } +// static +api::autotest_private::LacrosMode +AutotestPrivateGetLacrosInfoFunction::ToLacrosMode( + crosapi::browser_util::LacrosMode lacrosMode) { + switch (lacrosMode) { + case crosapi::browser_util::LacrosMode::kDisabled: + return api::autotest_private::LacrosMode::LACROS_MODE_DISABLED; + case crosapi::browser_util::LacrosMode::kSideBySide: + return api::autotest_private::LacrosMode::LACROS_MODE_SIDEBYSIDE; + case crosapi::browser_util::LacrosMode::kPrimary: + return api::autotest_private::LacrosMode::LACROS_MODE_PRIMARY; + case crosapi::browser_util::LacrosMode::kOnly: + return api::autotest_private::LacrosMode::LACROS_MODE_ONLY; + } +} + ExtensionFunction::ResponseAction AutotestPrivateGetLacrosInfoFunction::Run() { DVLOG(1) << "AutotestPrivateGetLacrosInfoFunction"; auto* browser_manager = crosapi::BrowserManager::Get(); @@ -1981,6 +1997,8 @@ ExtensionFunction::ResponseAction AutotestPrivateGetLacrosInfoFunction::Run() { result->SetBoolKey("isKeepAlive", browser_manager->IsKeepAliveEnabled()); result->SetStringKey("lacrosPath", browser_manager->lacros_path().MaybeAsASCII()); + result->SetStringKey("mode", api::autotest_private::ToString(ToLacrosMode( + crosapi::browser_util::GetLacrosMode()))); return RespondNow( OneArgument(base::Value::FromUniquePtrValue(std::move(result)))); } diff --git a/chrome/browser/chromeos/extensions/autotest_private/autotest_private_api.h b/chrome/browser/chromeos/extensions/autotest_private/autotest_private_api.h index 3e3986bce066b..246620d4d74f2 100644 --- a/chrome/browser/chromeos/extensions/autotest_private/autotest_private_api.h +++ b/chrome/browser/chromeos/extensions/autotest_private/autotest_private_api.h @@ -336,6 +336,8 @@ class AutotestPrivateGetLacrosInfoFunction : public ExtensionFunction { ResponseAction Run() override; static api::autotest_private::LacrosState ToLacrosState( crosapi::BrowserManager::State state); + static api::autotest_private::LacrosMode ToLacrosMode( + crosapi::browser_util::LacrosMode lacrosMode); }; class AutotestPrivateGetArcAppFunction : public ExtensionFunction { diff --git a/chrome/common/extensions/api/autotest_private.idl b/chrome/common/extensions/api/autotest_private.idl index 6f55e84c5e012..d40b0bc06d7e1 100644 --- a/chrome/common/extensions/api/autotest_private.idl +++ b/chrome/common/extensions/api/autotest_private.idl @@ -265,7 +265,7 @@ namespace autotestPrivate { callback IsLacrosPrimaryBrowserCallback = void (boolean primary); - // crosapi::BrowserManager::State + // A mapping of crosapi::BrowserManager::State enum LacrosState { NotInitialized, Mounting, @@ -277,6 +277,14 @@ namespace autotestPrivate { Terminating }; + // A mapping of crosapi::browser_util::LacrosMode + enum LacrosMode { + Disabled, + SideBySide, + Primary, + Only + }; + dictionary LacrosInfo { // The state of lacros. LacrosState state; @@ -285,6 +293,9 @@ namespace autotestPrivate { // Path to lacros-chrome directory. Note that this may change over time if // omaha is used. This also may be empty if lacros is not running. DOMString lacrosPath; + // Specifies the mode Lacros is currently running. + // For a full list of supported mode, see LacrosMode enum definition. + LacrosMode lacrosMode; }; callback GetLacrosInfoCallback = void (LacrosInfo info);