Disable preinstalled web apps if the Chrome app they're replacing is blocked by policy

This CL updates our preinstalled web app replacement flow to disable
the web app if the Chrome app it's meant to replace has been disabled
by admin policy.

This is to prevent default apps appearing on enterprise devices during
web app migration when they were originally being blocked as Chrome apps.

Bug: 1131338
Change-Id: I44a5d0c3c9b547c28a5c3b664e53844335ab3da3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425549
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813060}

chrome/browser/web_applications/BUILD.gn

@@ -18,6 +18,7 @@ source_set("common") {
   sources = [
     "daily_metrics_helper.cc",
     "daily_metrics_helper.h",
+    "extension_status_utils.h",
     "manifest_update_manager.cc",
     "manifest_update_manager.h",
     "manifest_update_task.cc",

chrome/browser/web_applications/extension_status_utils.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_WEB_APPLICATIONS_EXTENSION_STATUS_UTILS_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_EXTENSION_STATUS_UTILS_H_
+
+#include <string>
+
+namespace content {
+class BrowserContext;
+}
+
+namespace extensions {
+
+bool IsExtensionBlockedByPolicy(content::BrowserContext* context,
+                                const std::string& extension_id);
+
+// Returns whether the extension with |extension_id| is installed regardless of
+// disabled/blocked/terminated status.
+bool IsExtensionInstalled(content::BrowserContext* context,
+                          const std::string& extension_id);
+
+// Returns whether the user has uninstalled an externally installed extension
+// with |extension_id|.
+bool IsExternalExtensionUninstalled(content::BrowserContext* context,
+                                    const std::string& extension_id);
+
+}  // namespace extensions
+
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_EXTENSION_STATUS_UTILS_H_

chrome/browser/web_applications/extensions/BUILD.gn

@@ -31,6 +31,7 @@ source_set("extensions") {
     "bookmark_app_shortcut_manager.h",
     "bookmark_app_util.cc",
     "bookmark_app_util.h",
+    "extension_status_utils.cc",
     "web_app_extension_shortcut.cc",
     "web_app_extension_shortcut.h",
     "web_app_extension_shortcut_mac.h",

chrome/browser/web_applications/extensions/extension_status_utils.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/web_applications/extension_status_utils.h"
+#include "chrome/browser/extensions/extension_management.h"
+#include "extensions/browser/extension_prefs.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/browser/extension_system.h"
+
+namespace extensions {
+
+bool IsExtensionBlockedByPolicy(content::BrowserContext* context,
+                                const std::string& extension_id) {
+  auto* registry = ExtensionRegistry::Get(context);
+  // May be nullptr in unit tests.
+  if (!registry)
+    return false;
+
+  const Extension* extension = registry->GetInstalledExtension(extension_id);
+  ExtensionManagement* management =
+      ExtensionManagementFactory::GetForBrowserContext(context);
+  ExtensionManagement::InstallationMode mode =
+      extension ? management->GetInstallationMode(extension)
+                : management->GetInstallationMode(extension_id,
+                                                  /*update_url=*/std::string());
+  return mode == ExtensionManagement::INSTALLATION_BLOCKED ||
+         mode == ExtensionManagement::INSTALLATION_REMOVED;
+}
+
+bool IsExtensionInstalled(content::BrowserContext* context,
+                          const std::string& extension_id) {
+  auto* registry = ExtensionRegistry::Get(context);
+  // May be nullptr in unit tests.
+  return registry && registry->GetInstalledExtension(extension_id);
+}
+
+bool IsExternalExtensionUninstalled(content::BrowserContext* context,
+                                    const std::string& extension_id) {
+  auto* prefs = ExtensionPrefs::Get(context);
+  // May be nullptr in unit tests.
+  return prefs && prefs->IsExternalExtensionUninstalled(extension_id);
+}
+
+}  // namespace extensions

chrome/browser/web_applications/external_web_app_manager.cc

@@ -31,6 +31,7 @@
 #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"
+#include "chrome/browser/web_applications/extension_status_utils.h"
 #include "chrome/browser/web_applications/external_web_app_utils.h"
 #include "chrome/browser/web_applications/preinstalled_web_apps.h"
 #include "chrome/common/chrome_features.h"
@@ -246,31 +247,35 @@ void ExternalWebAppManager::PostProcessConfigs(ConsumeInstallOptions callback,
     parsed_configs.options_list.push_back(std::move(options));
   parsed_configs.disabled_count += preinstalled_web_apps.disabled_count;
 
-  // Save this as we may remove apps due to user uninstall (not the same as
-  // being disabled).
+  // Track this separately as we may remove apps due to user uninstall (not the
+  // same as being disabled).
   int enabled_count = parsed_configs.options_list.size();
 
-  // Remove web apps whose replace target was uninstalled.
-  if (extensions::ExtensionSystem::Get(profile_)) {
-    auto* extension_prefs = extensions::ExtensionPrefs::Get(profile_);
-    auto* extension_registry = extensions::ExtensionRegistry::Get(profile_);
-
   base::EraseIf(
-        parsed_configs.options_list,
-        [&](const ExternalInstallOptions& options) {
+      parsed_configs.options_list, [&](const ExternalInstallOptions& options) {
+        // Remove if any blocked by admin policy.
         for (const AppId& app_id : options.uninstall_and_replace) {
-            if (extension_registry->GetInstalledExtension(app_id))
+          if (extensions::IsExtensionBlockedByPolicy(profile_, app_id)) {
+            ++parsed_configs.disabled_count;
+            --enabled_count;
+            return true;
+          }
+        }
+
+        // Keep if any installed.
+        for (const AppId& app_id : options.uninstall_and_replace) {
+          if (extensions::IsExtensionInstalled(profile_, app_id))
             return false;
         }
 
+        // Remove if any previously uninstalled.
         for (const AppId& app_id : options.uninstall_and_replace) {
-            if (extension_prefs->IsExternalExtensionUninstalled(app_id))
+          if (extensions::IsExternalExtensionUninstalled(profile_, app_id))
             return true;
         }
 
         return false;
       });
-  }
 
   base::UmaHistogramCounts100(ExternalWebAppManager::kHistogramEnabledCount,
                               enabled_count);

chrome/browser/web_applications/external_web_app_manager_unittest.cc

@@ -19,13 +19,16 @@
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/scoped_path_override.h"
+#include "chrome/browser/extensions/extension_management_test_util.h"
 #include "chrome/browser/supervised_user/supervised_user_constants.h"
 #include "chrome/browser/web_applications/components/external_app_install_features.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
+#include "chrome/browser/web_applications/preinstalled_web_apps.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/account_id/account_id.h"
+#include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
@@ -186,6 +189,53 @@ class ExternalWebAppManagerTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(ExternalWebAppManagerTest);
 };
 
+TEST_F(ExternalWebAppManagerTest, ReplacementExtensionBlockedByPolicy) {
+  using PolicyUpdater = extensions::ExtensionManagementPrefUpdater<
+      sync_preferences::TestingPrefServiceSyncable>;
+  auto test_profile = std::make_unique<TestingProfile>();
+  sync_preferences::TestingPrefServiceSyncable* prefs =
+      test_profile->GetTestingPrefService();
+
+  ScopedTestingPreinstalledAppData scoped_preinstalled_apps;
+  GURL install_url("https://test.app");
+  constexpr char kExtensionId[] = "abcdefghijklmnopabcdefghijklmnop";
+  scoped_preinstalled_apps.apps.push_back({
+      .install_url = install_url,
+      .feature_name = nullptr,
+      .app_id_to_replace = kExtensionId,
+  });
+
+  auto expect_present = [&]() {
+    std::vector<ExternalInstallOptions> options_list =
+        LoadApps(/*test_dir=*/"", test_profile.get());
+    ASSERT_EQ(options_list.size(), 1u);
+    EXPECT_EQ(options_list[0].install_url, install_url);
+  };
+
+  auto expect_not_present = [&]() {
+    std::vector<ExternalInstallOptions> options_list =
+        LoadApps(/*test_dir=*/"", test_profile.get());
+    ASSERT_EQ(options_list.size(), 0u);
+  };
+
+  expect_present();
+
+  PolicyUpdater(prefs).SetBlocklistedByDefault(false);
+  expect_present();
+
+  PolicyUpdater(prefs).SetBlocklistedByDefault(true);
+  expect_not_present();
+
+  PolicyUpdater(prefs).SetIndividualExtensionInstallationAllowed(kExtensionId,
+                                                                 true);
+  expect_present();
+
+  PolicyUpdater(prefs).SetBlocklistedByDefault(false);
+  PolicyUpdater(prefs).SetIndividualExtensionInstallationAllowed(kExtensionId,
+                                                                 false);
+  expect_not_present();
+}
+
 // Only Chrome OS parses config files.
 #if defined(OS_CHROMEOS)
 TEST_F(ExternalWebAppManagerTest, GoodJson) {

chrome/browser/web_applications/preinstalled_web_apps.cc

@@ -46,7 +46,8 @@ PreinstalledWebApps GetPreinstalledWebApps() {
   PreinstalledWebApps result;
 
   for (const PreinstalledAppData& app_data : GetPreinstalledAppData()) {
-    if (!IsExternalAppInstallFeatureEnabled(app_data.feature_name)) {
+    if (app_data.feature_name &&
+        !IsExternalAppInstallFeatureEnabled(app_data.feature_name)) {
       ++result.disabled_count;
       continue;
     }