From ef39c6edb528e1d312bb19abf60bb784abdc8025 Mon Sep 17 00:00:00 2001 From: Tim Sergeant Date: Mon, 8 Aug 2022 00:38:26 +0000 Subject: [PATCH] Preloads: Allow cloud-gaming targeted web app preloads in Lacros Some CrOS devices have the "CloudGamingDevice" feature flag enabled on ash-chrome, which can be used to control web app preloads. When lacros-primary is enabled, we need to pass the state of the feature flag over crosapi to lacros, since lacros is responsible for handling web app preloads. Bug: 1345541 Change-Id: I079ce2fd4a70c71f9fd5b9846ffd6eafe4ecd381 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3802950 Reviewed-by: Hidehiko Abe Reviewed-by: Xiyuan Xia Commit-Queue: Tim Sergeant Reviewed-by: Alan Cutter Cr-Commit-Position: refs/heads/main@{#1032395} --- chrome/browser/ash/crosapi/crosapi_util.cc | 3 ++ chrome/browser/web_applications/BUILD.gn | 7 +++ .../preinstalled_app_install_features.cc | 23 ++++++++- ...reinstalled_web_app_manager_browsertest.cc | 49 +++++++++++++++++++ chromeos/constants/chromeos_features.cc | 14 ++++-- chromeos/crosapi/mojom/crosapi.mojom | 8 ++- chromeos/startup/browser_params_proxy.cc | 4 ++ chromeos/startup/browser_params_proxy.h | 2 + 8 files changed, 103 insertions(+), 7 deletions(-) diff --git a/chrome/browser/ash/crosapi/crosapi_util.cc b/chrome/browser/ash/crosapi/crosapi_util.cc index 933b1425c2501..c46ce89253dd1 100644 --- a/chrome/browser/ash/crosapi/crosapi_util.cc +++ b/chrome/browser/ash/crosapi/crosapi_util.cc @@ -36,6 +36,7 @@ #include "chromeos/components/cdm_factory_daemon/mojom/browser_cdm_factory.mojom.h" #include "chromeos/components/remote_apps/mojom/remote_apps.mojom.h" #include "chromeos/components/sensors/mojom/cros_sensor_service.mojom.h" +#include "chromeos/constants/chromeos_features.h" #include "chromeos/constants/devicetype.h" #include "chromeos/crosapi/cpp/crosapi_constants.h" #include "chromeos/crosapi/mojom/app_service.mojom.h" @@ -572,6 +573,8 @@ mojom::BrowserInitParamsPtr GetBrowserInitParams( tts_crosapi_util::ShouldEnableLacrosTtsSupport(); params->enable_float_window = base::FeatureList::IsEnabled(chromeos::wm::features::kFloatWindow); + params->is_cloud_gaming_device = + chromeos::features::IsCloudGamingDeviceEnabled(); return params; } diff --git a/chrome/browser/web_applications/BUILD.gn b/chrome/browser/web_applications/BUILD.gn index 987a6469b5f24..596d66cfad7ce 100644 --- a/chrome/browser/web_applications/BUILD.gn +++ b/chrome/browser/web_applications/BUILD.gn @@ -691,6 +691,13 @@ source_set("web_applications_browser_tests") { "//chrome/browser/ash/system_web_apps/test_support:test_support_ui", ] } + + if (is_chromeos_lacros) { + deps += [ + "//chromeos/constants", + "//chromeos/startup", + ] + } } group("browser_tests") { diff --git a/chrome/browser/web_applications/preinstalled_app_install_features.cc b/chrome/browser/web_applications/preinstalled_app_install_features.cc index 890e87f8dafd2..1e8fb818b2091 100644 --- a/chrome/browser/web_applications/preinstalled_app_install_features.cc +++ b/chrome/browser/web_applications/preinstalled_app_install_features.cc @@ -25,7 +25,6 @@ constexpr const base::Feature* kPreinstalledAppInstallFeatures[] = { &kCursiveStylusPreinstall, &kCursiveManagedStylusPreinstall, &kMessagesPreinstall, - &::chromeos::features::kCloudGamingDevice, #endif }; @@ -33,6 +32,22 @@ bool g_always_enabled_for_testing = false; namespace { +struct FeatureWithEnabledFunction { + const char* const name; + bool (*enabled_func)(); +}; + +// Features which have a function to be run to determine whether they are +// enabled. Prefer using a base::Feature with |kPreinstalledAppInstallFeatures| +// when possible. +const FeatureWithEnabledFunction + kPreinstalledAppInstallFeaturesWithEnabledFunctions[] = { +#if BUILDFLAG(IS_CHROMEOS) + {chromeos::features::kCloudGamingDevice.name, + &chromeos::features::IsCloudGamingDeviceEnabled} +#endif +}; + // Checks if the feature being passed matches any of the migration features // above. bool IsMigrationFeature(const base::Feature& feature) { @@ -105,6 +120,12 @@ bool IsPreinstalledAppInstallFeatureEnabled(base::StringPiece feature_name, return base::FeatureList::IsEnabled(*feature); } + for (const auto& feature : + kPreinstalledAppInstallFeaturesWithEnabledFunctions) { + if (feature.name == feature_name) + return feature.enabled_func(); + } + return false; } diff --git a/chrome/browser/web_applications/preinstalled_web_app_manager_browsertest.cc b/chrome/browser/web_applications/preinstalled_web_app_manager_browsertest.cc index 151d540170e1c..7291e4e90fbcc 100644 --- a/chrome/browser/web_applications/preinstalled_web_app_manager_browsertest.cc +++ b/chrome/browser/web_applications/preinstalled_web_app_manager_browsertest.cc @@ -53,6 +53,10 @@ #include "ui/events/devices/device_data_manager_test_api.h" #include "ui/events/devices/touchscreen_device.h" +#if BUILDFLAG(IS_CHROMEOS) +#include "chromeos/constants/chromeos_features.h" +#endif + #if BUILDFLAG(IS_CHROMEOS_ASH) #include "ash/public/cpp/test/app_list_test_api.h" #include "chrome/browser/ui/app_list/app_list_client_impl.h" @@ -60,6 +64,11 @@ #include "chrome/browser/ui/app_list/app_list_syncable_service_factory.h" #endif +#if BUILDFLAG(IS_CHROMEOS_LACROS) +#include "chromeos/crosapi/mojom/crosapi.mojom.h" +#include "chromeos/startup/browser_init_params.h" +#endif + namespace web_app { namespace { @@ -1265,6 +1274,46 @@ IN_PROC_BROWSER_TEST_F(PreinstalledWebAppManagerBrowserTest, } } +class PreinstalledWebAppManagerWithCloudGamingBrowserTest + : public PreinstalledWebAppManagerBrowserTest { + public: + PreinstalledWebAppManagerWithCloudGamingBrowserTest() { +#if BUILDFLAG(IS_CHROMEOS_LACROS) + auto params = crosapi::mojom::BrowserInitParams::New(); + params->is_cloud_gaming_device = true; + chromeos::BrowserInitParams::SetInitParamsForTests(std::move(params)); +#else + scoped_feature_list_.InitAndEnableFeature( + chromeos::features::kCloudGamingDevice); +#endif + } + + private: + base::test::ScopedFeatureList scoped_feature_list_; +}; + +// Tests that the custom behavior for the "CloudGamingDevice" feature works on +// both Ash and Lacros. +IN_PROC_BROWSER_TEST_F(PreinstalledWebAppManagerWithCloudGamingBrowserTest, + GateOnCloudGamingFeature) { + ASSERT_TRUE(embedded_test_server()->Start()); + + constexpr char kAppConfigTemplate[] = + R"({ + "app_url": "$1", + "launch_container": "window", + "user_type": ["unmanaged"], + "feature_name": "$2" + })"; + std::string app_config = base::ReplaceStringPlaceholders( + kAppConfigTemplate, + {GetAppUrl().spec(), chromeos::features::kCloudGamingDevice.name}, + nullptr); + + EXPECT_EQ(SyncPreinstalledAppConfig(GetAppUrl(), app_config), + webapps::InstallResultCode::kSuccessNewInstall); +} + #endif // BUILDFLAG(IS_CHROMEOS) INSTANTIATE_TEST_SUITE_P(All, diff --git a/chromeos/constants/chromeos_features.cc b/chromeos/constants/chromeos_features.cc index cfc469bc66c34..978d7101e368d 100644 --- a/chromeos/constants/chromeos_features.cc +++ b/chromeos/constants/chromeos_features.cc @@ -3,11 +3,14 @@ // found in the LICENSE file. #include "chromeos/constants/chromeos_features.h" + #include "base/feature_list.h" -namespace chromeos { +#if BUILDFLAG(IS_CHROMEOS_LACROS) +#include "chromeos/startup/browser_params_proxy.h" +#endif -namespace features { +namespace chromeos::features { // Enables or disables more filtering out of phones from the Bluetooth UI. const base::Feature kBluetoothPhoneFilter{"BluetoothPhoneFilter", @@ -51,7 +54,11 @@ const base::Feature kQuickAnswersForMoreLocales{ "QuickAnswersForMoreLocales", base::FEATURE_DISABLED_BY_DEFAULT}; bool IsCloudGamingDeviceEnabled() { +#if BUILDFLAG(IS_CHROMEOS_LACROS) + return chromeos::BrowserParamsProxy::Get()->IsCloudGamingDevice(); +#else return base::FeatureList::IsEnabled(kCloudGamingDevice); +#endif } bool IsDarkLightModeEnabled() { @@ -70,5 +77,4 @@ bool IsQuickAnswersForMoreLocalesEnabled() { return base::FeatureList::IsEnabled(kQuickAnswersForMoreLocales); } -} // namespace features -} // namespace chromeos +} // namespace chromeos::features diff --git a/chromeos/crosapi/mojom/crosapi.mojom b/chromeos/crosapi/mojom/crosapi.mojom index 7070b34680a6e..3300e20a1e404 100644 --- a/chromeos/crosapi/mojom/crosapi.mojom +++ b/chromeos/crosapi/mojom/crosapi.mojom @@ -872,8 +872,8 @@ enum OpenUrlFrom { // parameters here. If a new parameter is added and its value is only known after // the user has logged in, please update BrowserPostLoginParams as well. // -// Next version: 52 -// Next id: 52 +// Next version: 53 +// Next id: 53 [Stable, RenamedFrom="crosapi.mojom.LacrosInitParams"] struct BrowserInitParams { // This is ash-chrome's version of the Crosapi interface. This is used by @@ -1198,6 +1198,10 @@ struct BrowserInitParams { // multitask menu will be enabled. [MinVersion=51] bool enable_float_window@51; + + // True if this is a device with cloud gaming features enabled. + [MinVersion=52] + bool is_cloud_gaming_device@52; }; // BrowserPostLoginParams is the subset of parameters in BrowserInitParams diff --git a/chromeos/startup/browser_params_proxy.cc b/chromeos/startup/browser_params_proxy.cc index fa1e9782aba73..5f46db9bb03cc 100644 --- a/chromeos/startup/browser_params_proxy.cc +++ b/chromeos/startup/browser_params_proxy.cc @@ -234,4 +234,8 @@ bool BrowserParamsProxy::IsFloatWindowEnabled() const { return BrowserInitParams::Get()->enable_float_window; } +bool BrowserParamsProxy::IsCloudGamingDevice() const { + return BrowserInitParams::Get()->is_cloud_gaming_device; +} + } // namespace chromeos diff --git a/chromeos/startup/browser_params_proxy.h b/chromeos/startup/browser_params_proxy.h index 6ba93ccbc5bce..3762dd915249f 100644 --- a/chromeos/startup/browser_params_proxy.h +++ b/chromeos/startup/browser_params_proxy.h @@ -118,6 +118,8 @@ class COMPONENT_EXPORT(CHROMEOS_STARTUP) BrowserParamsProxy { bool IsFloatWindowEnabled() const; + bool IsCloudGamingDevice() const; + private: friend base::NoDestructor;