system-web-apps: add EnableAllSystemWebApps chrome feature

This CL adds a EnableAllSystemWebApps chrome feature, when enabled
SystemWebAppManager will enable and install all registered system apps,
and ignoring each system app's respective feature flag.

This is useful for testing.

This CL migrates the existing g_enable_all_system_web_apps_for_testing
to this new feature, and adds a kNumberOfRegisteredSystemApps to verify
all the apps can be installed successfully.

Bug: 1095524
Change-Id: Iba595e7d94ff20076a6bfb36e0a91cee09c897bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2366952Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Commit-Queue: Jiewei Qian  <qjw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800906}

chrome/browser/web_applications/system_web_app_manager.cc

@@ -73,10 +73,6 @@ const char kFileHandlingOriginTrial[] = "FileHandling";
 // bailing out.
 const int kInstallFailureAttempts = 3;
 
-// When set to |true|, SystemWebAppManager will enable all registered System
-// Apps, regardless of their respective feature flag.
-bool g_enable_all_system_web_apps_for_testing = false;
-
 // Use #if defined to avoid compiler error on unused function.
 #if defined(OS_CHROMEOS)
 
@@ -284,7 +280,7 @@ const char SystemWebAppManager::kInstallDurationHistogramName[];
 
 // static
 bool SystemWebAppManager::IsAppEnabled(SystemAppType type) {
-  if (g_enable_all_system_web_apps_for_testing)
+  if (base::FeatureList::IsEnabled(features::kEnableAllSystemWebApps))
     return true;
 
 #if defined(OS_CHROMEOS)
@@ -321,11 +317,6 @@ bool SystemWebAppManager::IsAppEnabled(SystemAppType type) {
 #endif  // OS_CHROMEOS
 }
 
-// static
-void SystemWebAppManager::EnableAllSystemAppsForTesting() {
-  g_enable_all_system_web_apps_for_testing = true;
-}
-
 SystemWebAppManager::SystemWebAppManager(Profile* profile)
     : profile_(profile),
       on_apps_synchronized_(new base::OneShotEvent()),

chrome/browser/web_applications/system_web_app_manager.h

@@ -59,6 +59,9 @@ enum class SystemAppType {
   TELEMETRY,
   SAMPLE,
 #endif  // !defined(OFFICIAL_BUILD)
+
+  // When adding a new system app, update system_web_app_manager_browsertest.cc
+  // |GetExpectedNumberOfInstalledSystemApps| method accordingly.
 };
 
 using OriginTrialsMap = std::map<url::Origin, std::vector<std::string>>;
@@ -156,11 +159,6 @@ class SystemWebAppManager {
 
   static bool IsEnabled();
 
-  // This call will instruct System Web App Manager to include all registered
-  // System Apps for installation. Must be called before SystemWebAppManager is
-  // constructed.
-  static void EnableAllSystemAppsForTesting();
-
   // The SystemWebAppManager is disabled in browser tests by default because it
   // pollutes the startup state (several tests expect the Extensions state to be
   // clean).

chrome/browser/web_applications/system_web_app_manager_browsertest.cc

@@ -1119,21 +1119,36 @@ class SystemWebAppManagerUpgradeBrowserTest
  public:
   SystemWebAppManagerUpgradeBrowserTest()
       : SystemWebAppManagerBrowserTest(/*install_mock=*/false) {
-    SystemWebAppManager::EnableAllSystemAppsForTesting();
+    features_.InitAndEnableFeature(features::kEnableAllSystemWebApps);
   }
   ~SystemWebAppManagerUpgradeBrowserTest() override = default;
+
+  unsigned int GetExpectedNumberOfInstalledSystemApps() {
+#if defined(OFFICIAL_BUILD)
+    return 8;
+#else
+    // TODO(http://crbug.com/1120208): Telemetry isn't available for install
+    // unless its flag is enabled. So it's not included here. Update this after
+    // Telemetry is available for install without a flag.
+    return 9;
+#endif  // defined(OFFICIAL_BUILD)
+  }
+
+ private:
+  base::test::ScopedFeatureList features_;
 };
 
 IN_PROC_BROWSER_TEST_P(SystemWebAppManagerUpgradeBrowserTest, PRE_Upgrade) {
   WaitForTestSystemAppInstall();
-  EXPECT_GE(GetManager().GetAppIds().size(), 1U);
+  EXPECT_GE(GetExpectedNumberOfInstalledSystemApps(),
+            GetManager().GetAppIds().size());
 }
 
 IN_PROC_BROWSER_TEST_P(SystemWebAppManagerUpgradeBrowserTest, Upgrade) {
   WaitForTestSystemAppInstall();
   const auto& app_ids = GetManager().GetAppIds();
 
-  EXPECT_GE(app_ids.size(), 1U);
+  EXPECT_EQ(GetExpectedNumberOfInstalledSystemApps(), app_ids.size());
 
   for (const auto& app_id : app_ids) {
     const auto type = GetManager().GetSystemAppTypeForAppId(app_id).value();

chrome/common/chrome_features.cc

@@ -292,6 +292,11 @@ const base::Feature kDownloadsLocationChange{"DownloadsLocationChange",
                                              base::FEATURE_ENABLED_BY_DEFAULT};
 #endif
 
+// Enables all registered system web apps, regardless of their respective
+// feature flags.
+const base::Feature kEnableAllSystemWebApps{"EnableAllSystemWebApps",
+                                            base::FEATURE_DISABLED_BY_DEFAULT};
+
 // Disables ambient authentication in guest sessions.
 const base::Feature kEnableAmbientAuthenticationInGuestSession{
     "EnableAmbientAuthenticationInGuestSession",

chrome/common/chrome_features.h

@@ -196,6 +196,9 @@ COMPONENT_EXPORT(CHROME_FEATURES)
 extern const base::Feature kDownloadsLocationChange;
 #endif
 
+COMPONENT_EXPORT(CHROME_FEATURES)
+extern const base::Feature kEnableAllSystemWebApps;
+
 COMPONENT_EXPORT(CHROME_FEATURES)
 extern const base::Feature kEnableAmbientAuthenticationInGuestSession;