Add "only_for_new_users" to default web app configs

This CL adds a config parameter "only_for_new_users" which prevents
installation of a default web app if the user profile has used Chrome
already. This allows us to specifically target new users only for
new default web app rollouts.

Bug: 1145032
Change-Id: Id222d359b9bf9645e676de12ef9ea23039710a46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2515931
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Glen Robertson <glenrob@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823482}

chrome/browser/web_applications/components/external_install_options.h

@@ -76,6 +76,10 @@ struct ExternalInstallOptions {
   // uninstalled it.
   bool override_previous_user_uninstall = false;
 
+  // Whether the app should only be installed if the user is using Chrome for
+  // the first time.
+  bool only_for_new_users = false;
+
   // This must only be used by pre-installed default or system apps that are
   // valid PWAs if loading the real service worker is too costly to verify
   // programmatically.

chrome/browser/web_applications/components/externally_installed_web_app_prefs.cc

@@ -177,6 +177,14 @@ base::Optional<AppId> ExternallyInstalledWebAppPrefs::LookupAppId(
   return base::nullopt;
 }
 
+bool ExternallyInstalledWebAppPrefs::HasNoApps() const {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  const base::DictionaryValue* dict =
+      pref_service_->GetDictionary(prefs::kWebAppsExtensionIDs);
+  return dict->DictEmpty();
+}
+
 base::Optional<AppId> ExternallyInstalledWebAppPrefs::LookupPlaceholderAppId(
     const GURL& url) const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

chrome/browser/web_applications/components/externally_installed_web_app_prefs.h

@@ -54,6 +54,7 @@ class ExternallyInstalledWebAppPrefs {
               const AppId& app_id,
               ExternalInstallSource install_source);
   base::Optional<AppId> LookupAppId(const GURL& url) const;
+  bool HasNoApps() const;
 
   // Returns an id if there is a placeholder app for |url|. Note that nullopt
   // does not mean that there is no app for |url| just that there is no

chrome/browser/web_applications/external_web_app_manager.cc

@@ -28,6 +28,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/apps/user_type_filter.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/web_applications/components/externally_installed_web_app_prefs.h"
 #include "chrome/browser/web_applications/components/pending_app_manager.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_install_utils.h"
@@ -36,6 +37,9 @@
 #include "chrome/browser/web_applications/preinstalled_web_apps.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/common/pref_names.h"
+#include "components/pref_registry/pref_registry_syncable.h"
+#include "components/version_info/version_info.h"
 #include "content/public/browser/browser_thread.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
@@ -147,6 +151,12 @@ const char* ExternalWebAppManager::kHistogramDisabledCount =
 const char* ExternalWebAppManager::kHistogramConfigErrorCount =
     "WebApp.Preinstalled.ConfigErrorCount";
 
+void ExternalWebAppManager::RegisterProfilePrefs(
+    user_prefs::PrefRegistrySyncable* registry) {
+  registry->RegisterStringPref(prefs::kWebAppsLastPreinstallSynchronizeVersion,
+                               "");
+}
+
 void ExternalWebAppManager::SkipStartupForTesting() {
   g_skip_startup_for_testing_ = true;
 }
@@ -251,9 +261,22 @@ void ExternalWebAppManager::PostProcessConfigs(ConsumeInstallOptions callback,
   // same as being disabled).
   int enabled_count = parsed_configs.options_list.size();
 
+  bool is_new_user = IsNewUser();
+
   base::EraseIf(
       parsed_configs.options_list, [&](const ExternalInstallOptions& options) {
-        // Remove if any blocked by admin policy.
+        // Remove if only for new users, user isn't new and app was not
+        // installed previously.
+        if (options.only_for_new_users && !is_new_user) {
+          bool was_previously_installed =
+              ExternallyInstalledWebAppPrefs(profile_->GetPrefs())
+                  .LookupAppId(options.install_url)
+                  .has_value();
+          if (!was_previously_installed)
+            return true;
+        }
+
+        // Remove if any apps to replace are blocked by admin policy.
         for (const AppId& app_id : options.uninstall_and_replace) {
           if (extensions::IsExtensionBlockedByPolicy(profile_, app_id)) {
             ++parsed_configs.disabled_count;
@@ -262,13 +285,13 @@ void ExternalWebAppManager::PostProcessConfigs(ConsumeInstallOptions callback,
           }
         }
 
-        // Keep if any installed.
+        // Keep if any apps to replace are installed.
         for (const AppId& app_id : options.uninstall_and_replace) {
           if (extensions::IsExtensionInstalled(profile_, app_id))
             return false;
         }
 
-        // Remove if any previously uninstalled.
+        // Remove if any apps to replace were previously uninstalled.
         for (const AppId& app_id : options.uninstall_and_replace) {
           if (extensions::IsExternalExtensionUninstalled(profile_, app_id))
             return true;
@@ -303,6 +326,12 @@ void ExternalWebAppManager::OnExternalWebAppsSynchronized(
     PendingAppManager::SynchronizeCallback callback,
     std::map<GURL, InstallResultCode> install_results,
     std::map<GURL, bool> uninstall_results) {
+  // Note that we are storing the Chrome version instead of a "has synchronised"
+  // bool in order to do version update specific logic in the future.
+  profile_->GetPrefs()->SetString(
+      prefs::kWebAppsLastPreinstallSynchronizeVersion,
+      version_info::GetMajorVersionNumber());
+
   RecordExternalAppInstallResultCode("Webapp.InstallResult.Default",
                                      install_results);
   if (callback) {
@@ -339,4 +368,18 @@ base::FilePath ExternalWebAppManager::GetConfigDir() {
   return dir;
 }
 
+bool ExternalWebAppManager::IsNewUser() {
+  PrefService* prefs = profile_->GetPrefs();
+  std::string last_version =
+      prefs->GetString(prefs::kWebAppsLastPreinstallSynchronizeVersion);
+  if (!last_version.empty())
+    return false;
+  // It's not enough to check whether the last_version string has been set
+  // because users have been around before this pref was introduced (M88). We
+  // distinguish those users via the presence of any
+  // ExternallyInstalledWebAppPrefs which would have been set by past default
+  // app installs. Remove this after a few Chrome versions have passed.
+  return ExternallyInstalledWebAppPrefs(prefs).HasNoApps();
+}
+
 }  //  namespace web_app

chrome/browser/web_applications/external_web_app_manager.h

@@ -18,6 +18,10 @@ namespace base {
 class FilePath;
 }
 
+namespace user_prefs {
+class PrefRegistrySyncable;
+}
+
 class Profile;
 
 namespace web_app {
@@ -45,6 +49,8 @@ class ExternalWebAppManager {
   static const char* kHistogramDisabledCount;
   static const char* kHistogramConfigErrorCount;
 
+  static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
+
   static void SkipStartupForTesting();
   static void SetConfigDirForTesting(const base::FilePath* config_dir);
   static void SetConfigsForTesting(const std::vector<base::Value>* configs);
@@ -84,6 +90,10 @@ class ExternalWebAppManager {
 
   base::FilePath GetConfigDir();
 
+  // Returns whether this is the first time we've deployed default apps on this
+  // profile.
+  bool IsNewUser();
+
   PendingAppManager* pending_app_manager_ = nullptr;
   Profile* const profile_;
 

chrome/browser/web_applications/external_web_app_manager_browsertest.cc

@@ -46,10 +46,29 @@ class ExternalWebAppManagerBrowserTest
     return WebAppProvider::Get(browser()->profile())->registrar();
   }
 
+  void SyncEmptyConfigs() {
+    std::vector<base::Value> app_configs;
+    ExternalWebAppManager::SetConfigsForTesting(&app_configs);
+
+    base::RunLoop run_loop;
+    WebAppProvider::Get(browser()->profile())
+        ->external_web_app_manager_for_testing()
+        .LoadAndSynchronizeForTesting(base::BindLambdaForTesting(
+            [&](std::map<GURL, InstallResultCode> install_results,
+                std::map<GURL, bool> uninstall_results) {
+              EXPECT_EQ(install_results.size(), 0u);
+              EXPECT_EQ(uninstall_results.size(), 0u);
+              run_loop.Quit();
+            }));
+    run_loop.Run();
+
+    ExternalWebAppManager::SetConfigsForTesting(nullptr);
+  }
+
   // Mocks "icon.png" as available in the config's directory.
   base::Optional<InstallResultCode> SyncDefaultAppConfig(
       const GURL& install_url,
-      std::string app_config_string) {
+      base::StringPiece app_config_string) {
     base::FilePath test_config_dir(FILE_PATH_LITERAL("test_dir"));
     ExternalWebAppManager::SetConfigDirForTesting(&test_config_dir);
 
@@ -414,6 +433,52 @@ IN_PROC_BROWSER_TEST_F(ExternalWebAppManagerBrowserTest,
             SK_ColorBLUE);
 }
 
+const char kOnlyForNewUsersInstallUrl[] = "https://example.org/";
+const char kOnlyForNewUsersConfig[] = R"({
+    "app_url": "https://example.org/",
+    "launch_container": "window",
+    "user_type": ["unmanaged"],
+    "only_for_new_users": true,
+    "only_use_offline_manifest": true,
+    "offline_manifest": {
+      "name": "Test",
+      "start_url": "https://example.org/",
+      "scope": "https://example.org/",
+      "display": "standalone",
+      "icon_any_pngs": ["icon.png"]
+    }
+  })";
+
+IN_PROC_BROWSER_TEST_F(ExternalWebAppManagerBrowserTest,
+                       PRE_OnlyForNewUsersWithNewUser) {
+  // New user should have the app installed.
+  EXPECT_EQ(SyncDefaultAppConfig(GURL(kOnlyForNewUsersInstallUrl),
+                                 kOnlyForNewUsersConfig),
+            InstallResultCode::kSuccessOfflineOnlyInstall);
+}
+
+IN_PROC_BROWSER_TEST_F(ExternalWebAppManagerBrowserTest,
+                       OnlyForNewUsersWithNewUser) {
+  // App should persist after user stops being a new user.
+  EXPECT_EQ(SyncDefaultAppConfig(GURL(kOnlyForNewUsersInstallUrl),
+                                 kOnlyForNewUsersConfig),
+            InstallResultCode::kSuccessAlreadyInstalled);
+}
+
+IN_PROC_BROWSER_TEST_F(ExternalWebAppManagerBrowserTest,
+                       PRE_OnlyForNewUsersWithOldUser) {
+  // Simulate running Chrome without the configs present.
+  SyncEmptyConfigs();
+}
+IN_PROC_BROWSER_TEST_F(ExternalWebAppManagerBrowserTest,
+                       OnlyForNewUsersWithOldUser) {
+  // This instance of Chrome should be considered not a new user after the
+  // previous PRE_ launch and sync.
+  EXPECT_EQ(SyncDefaultAppConfig(GURL(kOnlyForNewUsersInstallUrl),
+                                 kOnlyForNewUsersConfig),
+            base::nullopt);
+}
+
 #endif  // defined(OS_CHROMEOS)
 
 }  // namespace web_app

chrome/browser/web_applications/external_web_app_utils.cc

@@ -36,6 +36,10 @@ constexpr char kAppUrl[] = "app_url";
 // on Chrome OS.
 constexpr char kHideFromUser[] = "hide_from_user";
 
+// kOnlyForNewUsers is an optional boolean. If set and true we will not install
+// the app for users that have already run Chrome before.
+constexpr char kOnlyForNewUsers[] = "only_for_new_users";
+
 // kCreateShortcuts is an optional boolean which controls whether OS
 // level shortcuts are created. On Chrome OS this controls whether the app is
 // pinned to the shelf.
@@ -220,6 +224,16 @@ ExternalConfigParseResult ParseConfig(FileUtilsWrapper& file_utils,
     return Result::Error();
   }
 
+  // only_for_new_users
+  value = app_config.FindKey(kOnlyForNewUsers);
+  if (value) {
+    if (!value->is_bool()) {
+      LOG(ERROR) << file << " had an invalid " << kOnlyForNewUsers;
+      return Result::Error();
+    }
+    options.only_for_new_users = value->GetBool();
+  }
+
   // hide_from_user
   bool hide_from_user = false;
   value = app_config.FindKey(kHideFromUser);

chrome/browser/web_applications/web_app_provider.cc

@@ -319,6 +319,7 @@ void WebAppProvider::CheckIsConnected() const {
 void WebAppProvider::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
   ExternallyInstalledWebAppPrefs::RegisterProfilePrefs(registry);
+  ExternalWebAppManager::RegisterProfilePrefs(registry);
   WebAppPolicyManager::RegisterProfilePrefs(registry);
   SystemWebAppManager::RegisterProfilePrefs(registry);
   WebAppPrefsUtilsRegisterProfilePrefs(registry);

chrome/common/pref_names.cc

@@ -1928,6 +1928,11 @@ const char kWebAppsDailyMetricsDate[] = "web_apps.daily_metrics_date";
 // Dictionary that maps web app URLs to Chrome extension IDs.
 const char kWebAppsExtensionIDs[] = "web_apps.extension_ids";
 
+// A string representing the last version of Chrome preinstalled web apps were
+// synchronised for.
+const char kWebAppsLastPreinstallSynchronizeVersion[] =
+    "web_apps.last_preinstall_synchronize_version";
+
 // Dictionary that maps web app ID to a dictionary of various preferences.
 // Used only in the new web applications system to store app preferences which
 // outlive the app installation and uninstallation.

chrome/common/pref_names.h

@@ -644,6 +644,7 @@ extern const char kWebAppInstallMetrics[];
 extern const char kWebAppsDailyMetrics[];
 extern const char kWebAppsDailyMetricsDate[];
 extern const char kWebAppsExtensionIDs[];
+extern const char kWebAppsLastPreinstallSynchronizeVersion[];
 extern const char kWebAppsPreferences[];
 extern const char kWebAppsUserDisplayModeCleanedUp[];
 extern const char kSystemWebAppLastUpdateVersion[];