extensions: Delete ExtensionService::BlockAllExtensions()

And UnblockAllExtensions(). The functions now live in
ExtensionRegistrar. Introduce one new delegate method so that
UnblockAllExtensions() can trigger an external install warning, like
the method in ExtensionService used to do.

Bug: 408049386
Change-Id: Id8e89c611e9d8bbaea1192e817ed8fec6e93d2fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6426657
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Auto-Submit: James Cook <jamescook@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1442757}

chrome/browser/enterprise/reporting/extension_info_unittest.cc

@@ -146,7 +146,7 @@ TEST_F(ExtensionInfoTest, ExtensionTerminated) {
 
 TEST_F(ExtensionInfoTest, ExtensionBlocked) {
   auto extension = BuildExtension();
-  service()->BlockAllExtensions();
+  registrar()->BlockAllExtensions();
 
   em::ChromeUserProfileInfo info;
   AppendExtensionInfoIntoProfileReport(profile(), &info);

chrome/browser/extensions/chrome_extension_registrar_delegate.cc

@@ -14,6 +14,7 @@
 #include "chrome/browser/extensions/extension_assets_manager.h"
 #include "chrome/browser/extensions/extension_disabled_ui.h"
 #include "chrome/browser/extensions/extension_special_storage_policy.h"
+#include "chrome/browser/extensions/external_install_manager.h"
 #include "chrome/browser/extensions/install_verifier.h"
 #include "chrome/browser/extensions/installed_loader.h"
 #include "chrome/browser/extensions/permissions/permissions_updater.h"
@@ -326,6 +327,10 @@ void ChromeExtensionRegistrarDelegate::GrantActivePermissions(
   PermissionsUpdater(profile_).GrantActivePermissions(extension);
 }
 
+void ChromeExtensionRegistrarDelegate::UpdateExternalExtensionAlert() {
+  ExternalInstallManager::Get(profile_)->UpdateExternalExtensionAlert();
+}
+
 void ChromeExtensionRegistrarDelegate::CheckPermissionsIncrease(
     const Extension* extension,
     bool is_extension_loaded) {

chrome/browser/extensions/chrome_extension_registrar_delegate.h

@@ -62,6 +62,7 @@ class ChromeExtensionRegistrarDelegate : public ExtensionRegistrar::Delegate {
   bool CanEnableExtension(const Extension* extension) override;
   bool CanDisableExtension(const Extension* extension) override;
   void GrantActivePermissions(const Extension* extension) override;
+  void UpdateExternalExtensionAlert() override;
 
  private:
   // Disables the extension if the privilege level has increased

chrome/browser/extensions/desktop_android/desktop_android_extension_system.cc

@@ -136,6 +136,7 @@ class DesktopAndroidExtensionRegistrarDelegate
   void GrantActivePermissions(const Extension* extension) override {
     PermissionsUpdater(browser_context_).GrantActivePermissions(extension);
   }
+  void UpdateExternalExtensionAlert() override {}
 
  private:
   raw_ptr<content::BrowserContext> browser_context_;  // Not owned.

chrome/browser/extensions/extension_service.cc

@@ -561,26 +561,6 @@ void ExtensionService::DisableUserExtensionsExcept(
   }
 }
 
-// Extensions that are not locked, components or forced by policy should be
-// locked. Extensions are no longer considered enabled or disabled. Blocklisted
-// extensions are now considered both blocklisted and locked.
-// TODO(crbug.com/408049386): Migrate callers to ExtensionRegistrar.
-void ExtensionService::BlockAllExtensions() {
-  extension_registrar_->BlockAllExtensions();
-}
-
-// All locked extensions should revert to being either enabled or disabled
-// as appropriate.
-// TODO(crbug.com/408049386): Migrate callers to ExtensionRegistrar and use a
-// delegate to access `external_install_manager_` (or inline the call).
-void ExtensionService::UnblockAllExtensions() {
-  extension_registrar_->UnblockAllExtensions();
-
-  // While extensions are blocked, we won't display any external install
-  // warnings. Now that they are unblocked, we should update the error.
-  external_install_manager_->UpdateExternalExtensionAlert();
-}
-
 content::BrowserContext* ExtensionService::GetBrowserContext() const {
   // Implemented in the .cc file to avoid adding a profile.h dependency to
   // extension_service.h.

chrome/browser/extensions/extension_service.h

@@ -294,18 +294,6 @@ class ExtensionService : public ExtensionServiceInterface,
   // |was_installed_by_default| flag.
   void DisableUserExtensionsExcept(const std::vector<std::string>& except_ids);
 
-  // Puts all extensions in a blocked state: Unloading every extension, and
-  // preventing them from ever loading until UnblockAllExtensions is called.
-  // This state is stored in preferences, so persists until Chrome restarts.
-  //
-  // Component, external component and allowlisted policy installed extensions
-  // are exempt from being Blocked (see CanBlockExtension in .cc file).
-  void BlockAllExtensions();
-
-  // All blocked extensions are reverted to their previous state, and are
-  // reloaded. Newly added extensions are no longer automatically blocked.
-  void UnblockAllExtensions();
-
   // Informs the service that an extension's files are in place for loading.
   //
   // |extension|                the extension

chrome/browser/extensions/extension_service_test_with_install.cc

@@ -388,7 +388,7 @@ void ExtensionServiceTestWithInstall::TerminateExtension(
 }
 
 void ExtensionServiceTestWithInstall::BlockAllExtensions() {
-  service()->BlockAllExtensions();
+  registrar()->BlockAllExtensions();
 }
 
 void ExtensionServiceTestWithInstall::ClearLoadedExtensions() {

chrome/browser/extensions/extension_service_unittest.cc

@@ -736,7 +736,7 @@ class ExtensionServiceTest : public ExtensionServiceTestWithInstall {
     EXPECT_FALSE(IsBlocked(extension_id));
 
     // Block the extensions.
-    service()->BlockAllExtensions();
+    registrar()->BlockAllExtensions();
     task_environment()->RunUntilIdle();
 
     if (should_block)
@@ -744,7 +744,7 @@ class ExtensionServiceTest : public ExtensionServiceTestWithInstall {
     else
       ASSERT_FALSE(IsBlocked(extension_id));
 
-    service()->UnblockAllExtensions();
+    registrar()->UnblockAllExtensions();
     task_environment()->RunUntilIdle();
 
     ASSERT_FALSE(IsBlocked(extension_id));
@@ -4066,7 +4066,7 @@ TEST_F(ExtensionServiceTest, BlockAndUnblockBlocklistedExtension) {
   AssertExtensionBlocksAndUnblocks(false, good0);
   AssertExtensionBlocksAndUnblocks(false, good1);
 
-  service()->BlockAllExtensions();
+  registrar()->BlockAllExtensions();
 
   // Remove an extension from the blocklist while the service is blocked.
   test_blocklist.SetBlocklistState(good0, NOT_BLOCKLISTED, true);
@@ -4119,7 +4119,7 @@ TEST_F(ExtensionServiceTest, BlockAndUnblockTheme) {
 TEST_F(ExtensionServiceTest, WillNotLoadExtensionsWhenBlocked) {
   InitializeGoodInstalledExtensionService();
 
-  service()->BlockAllExtensions();
+  registrar()->BlockAllExtensions();
 
   service()->Init();
 
@@ -4132,7 +4132,7 @@ TEST_F(ExtensionServiceTest, WillNotLoadExtensionsWhenBlocked) {
 TEST_F(ExtensionServiceTest, IsEnabledExtensionBlockedAndNotInstalled) {
   InitializeEmptyExtensionService();
 
-  service()->BlockAllExtensions();
+  registrar()->BlockAllExtensions();
 
   registrar()->IsExtensionEnabled(theme_crx);
 }
@@ -7422,7 +7422,7 @@ TEST_F(ExtensionServiceTest, BlockedExternalExtension) {
   external_install_manager()->UpdateExternalExtensionAlert();
   EXPECT_FALSE(HasExternalInstallErrors(profile()));
 
-  service()->BlockAllExtensions();
+  registrar()->BlockAllExtensions();
 
   provider->UpdateOrAddExtension(page_action, "1.0.0.0",
                                  data_dir().AppendASCII("page_action.crx"));
@@ -7430,7 +7430,7 @@ TEST_F(ExtensionServiceTest, BlockedExternalExtension) {
   WaitForInstallationAttemptToComplete(page_action);
   EXPECT_FALSE(HasExternalInstallErrors(profile()));
 
-  service()->UnblockAllExtensions();
+  registrar()->UnblockAllExtensions();
   EXPECT_TRUE(HasExternalInstallErrors(profile()));
 }
 
@@ -8495,7 +8495,7 @@ TEST_F(ExtensionServiceTest, PluginManagerCrash) {
 
   // crbug.com/708230: This will cause OnExtensionUnloaded to be called
   // redundantly for a disabled extension.
-  service()->BlockAllExtensions();
+  registrar()->BlockAllExtensions();
 }
 #endif  // BUILDFLAG(ENABLE_PLUGINS)
 
@@ -8517,7 +8517,7 @@ TEST_F(ExtensionServiceTest, BlockDisabledExtensionNotification) {
   registry()->AddObserver(&observer);
 
   // Block the extension
-  service()->BlockAllExtensions();
+  registrar()->BlockAllExtensions();
 
   // Check that we didn't get unloading notification
   EXPECT_EQ(std::string(), observer.last_extension_unloaded);

chrome/browser/extensions/updater/extension_updater_unittest.cc

@@ -221,6 +221,7 @@ class StubExtensionRegistrarDelegate : public ExtensionRegistrar::Delegate {
   bool CanEnableExtension(const Extension* extension) override { return true; }
   bool CanDisableExtension(const Extension* extension) override { return true; }
   void GrantActivePermissions(const Extension* extension) override {}
+  void UpdateExternalExtensionAlert() override {}
 };
 
 class MockUpdateService : public UpdateService {

chrome/browser/profiles/profile_manager.cc

@@ -116,8 +116,8 @@
 #endif
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
-#include "chrome/browser/extensions/extension_service.h"
 #include "extensions/browser/api/management/management_api.h"
+#include "extensions/browser/extension_registrar.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/manifest.h"
@@ -1461,15 +1461,13 @@ void ProfileManager::DoFinalInitForServices(Profile* profile,
       extensions_enabled);
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
-  // Set the block extensions bit on the ExtensionService. There likely are no
+  // Set the block extensions bit on the ExtensionRegistrar. There likely are no
   // blockable extensions to block.
   ProfileAttributesEntry* entry =
       GetProfileAttributesStorage().GetProfileAttributesWithPath(
           profile->GetPath());
   if (entry && entry->IsSigninRequired()) {
-    extensions::ExtensionSystem::Get(profile)
-        ->extension_service()
-        ->BlockAllExtensions();
+    extensions::ExtensionRegistrar::Get(profile)->BlockAllExtensions();
   }
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS)
 

chrome/browser/profiles/profile_window.cc

@@ -45,8 +45,8 @@
 #include "third_party/abseil-cpp/absl/cleanup/cleanup.h"
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
-#include "chrome/browser/extensions/extension_service.h"
 #include "extensions/browser/extension_prefs.h"
+#include "extensions/browser/extension_registrar.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_registry_factory.h"
 #include "extensions/browser/extension_system.h"
@@ -71,9 +71,7 @@ namespace {
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
 void UnblockExtensions(Profile* profile) {
-  extensions::ExtensionService* extension_service =
-      extensions::ExtensionSystem::Get(profile)->extension_service();
-  extension_service->UnblockAllExtensions();
+  extensions::ExtensionRegistrar::Get(profile)->UnblockAllExtensions();
 }
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS)
 

extensions/browser/extension_registrar.cc

@@ -757,6 +757,8 @@ void ExtensionRegistrar::BlockAllExtensions() {
   }
 }
 
+// All locked extensions should revert to being either enabled or disabled
+// as appropriate.
 void ExtensionRegistrar::UnblockAllExtensions() {
   block_extensions_ = false;
 
@@ -767,6 +769,10 @@ void ExtensionRegistrar::UnblockAllExtensions() {
     registry_->RemoveBlocked(extension->id());
     AddExtension(extension.get());
   }
+
+  // While extensions are blocked, we won't display any external install
+  // warnings. Now that they are unblocked, we should update the error.
+  delegate_->UpdateExternalExtensionAlert();
 }
 
 void ExtensionRegistrar::OnBlocklistStateRemoved(

extensions/browser/extension_registrar.h

@@ -121,6 +121,9 @@ class ExtensionRegistrar : public KeyedService, public ProcessManagerObserver {
     // Updates the `extension`s granted permissions lists to include all
     // permissions in the `extensions`s manifest.
     virtual void GrantActivePermissions(const Extension* extension) = 0;
+
+    // Checks if there are any new external extensions to notify the user about.
+    virtual void UpdateExternalExtensionAlert() = 0;
   };
 
   explicit ExtensionRegistrar(content::BrowserContext* browser_context);

extensions/browser/extension_registrar_unittest.cc

@@ -99,6 +99,7 @@ class TestExtensionRegistrarDelegate : public ExtensionRegistrar::Delegate {
   MOCK_METHOD1(CanDisableExtension, bool(const Extension* extension));
   MOCK_METHOD1(ShouldBlockExtension, bool(const Extension* extension));
   MOCK_METHOD1(GrantActivePermissions, void(const Extension* extension));
+  MOCK_METHOD0(UpdateExternalExtensionAlert, void());
 };
 
 }  // namespace

extensions/shell/browser/shell_extension_loader.cc

@@ -10,6 +10,7 @@
 #include "base/files/file_util.h"
 #include "base/functional/bind.h"
 #include "base/logging.h"
+#include "base/notimplemented.h"
 #include "base/task/sequenced_task_runner.h"
 #include "content/public/browser/browser_context.h"
 #include "extensions/browser/extension_file_task_runner.h"
@@ -189,4 +190,8 @@ void ShellExtensionLoader::GrantActivePermissions(const Extension* extension) {
   NOTIMPLEMENTED();
 }
 
+void ShellExtensionLoader::UpdateExternalExtensionAlert() {
+  NOTIMPLEMENTED();
+}
+
 }  // namespace extensions

extensions/shell/browser/shell_extension_loader.h

@@ -73,6 +73,7 @@ class ShellExtensionLoader : public ExtensionRegistrar::Delegate {
   bool CanEnableExtension(const Extension* extension) override;
   bool CanDisableExtension(const Extension* extension) override;
   void GrantActivePermissions(const Extension* extension) override;
+  void UpdateExternalExtensionAlert() override;
 
   raw_ptr<content::BrowserContext> browser_context_;  // Not owned.