extensions: remove legacy notification system

This change removes the last vestiges of the deprecated //content notification system from //extensions. Specifically, this change: 1. Reworks the SigninProfileExtensionsPolicyTest suite to depend on InstallTracker and InstallObserver rather than content notifications 2. Removes the logic in CrxInstaller to emit NOTIFICATION_EXTENSION_INSTALL_ERROR 3. Removes all uses of the extension-specific notification types 4. Removes the extension notification types themselves Fixed: 1174734 Change-Id: Ib835fba75674a63be9df4def5aafcefd54aec6d8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4771627 Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Elly FJ <ellyjones@chromium.org> Cr-Commit-Position: refs/heads/main@{#1184332}
chromium · Aug 16, 2023 · 0a7315b · 0a7315b
1 parent 5ff6532
commit 0a7315b
Show file tree

Hide file tree

Showing 24 changed files with 26 additions and 124 deletions.
diff --git a/chrome/browser/ash/extensions/external_cache_impl.cc b/chrome/browser/ash/extensions/external_cache_impl.cc
@@ -34,7 +34,6 @@
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/browser/updater/extension_downloader.h"
 #include "extensions/browser/updater/extension_downloader_types.h"
 #include "extensions/common/extension.h"

diff --git a/chrome/browser/ash/extensions/install_limiter.cc b/chrome/browser/ash/extensions/install_limiter.cc
@@ -11,7 +11,6 @@
 #include "base/task/thread_pool.h"
 #include "chrome/browser/ash/extensions/install_limiter_factory.h"
 #include "extensions/browser/extensions_browser_client.h"
-#include "extensions/browser/notification_types.h"
 
 namespace {
 

diff --git a/chrome/browser/ash/policy/login/signin_profile_extensions_policy_browsertest.cc b/chrome/browser/ash/policy/login/signin_profile_extensions_policy_browsertest.cc
@@ -13,27 +13,27 @@
 #include "base/memory/raw_ptr.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
+#include "base/scoped_observation.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/version.h"
 #include "chrome/browser/ash/policy/login/signin_profile_extensions_policy_test_base.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/install_observer.h"
+#include "chrome/browser/extensions/install_tracker.h"
 #include "chrome/browser/policy/extension_force_install_mixin.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_paths.h"
 #include "components/version_info/version_info.h"
 #include "content/public/browser/browser_context.h"
-#include "content/public/browser/notification_details.h"
-#include "content/public/browser/notification_source.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/test_launcher.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/browser/extension_host_test_helper.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/extension_util.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/browser/test_extension_registry_observer.h"
 #include "extensions/browser/update_observer.h"
 #include "extensions/common/extension.h"
@@ -99,41 +99,37 @@ base::FilePath GetNoImmediateUpdateExtensionPath(const std::string& version) {
 
 // Observer that allows waiting for an installation failure of a specific
 // extension/app.
-// TODO(emaxx): Extract this into a more generic helper class for using in other
-// tests.
-class ExtensionInstallErrorObserver final {
+class ExtensionInstallErrorObserver : public extensions::InstallObserver {
  public:
-  ExtensionInstallErrorObserver(const Profile* profile,
+  ExtensionInstallErrorObserver(Profile* profile,
                                 const std::string& extension_id)
-      : profile_(profile),
-        extension_id_(extension_id),
-        notification_observer_(
-            extensions::NOTIFICATION_EXTENSION_INSTALL_ERROR,
-            base::BindRepeating(
-                &ExtensionInstallErrorObserver::IsNotificationRelevant,
-                base::Unretained(this))) {}
+      : extension_id_(extension_id) {
+    auto* tracker = extensions::InstallTracker::Get(profile);
+    CHECK(tracker);
+    observation_.Observe(tracker);
+  }
 
   ExtensionInstallErrorObserver(const ExtensionInstallErrorObserver&) = delete;
   ExtensionInstallErrorObserver& operator=(
       const ExtensionInstallErrorObserver&) = delete;
 
-  void Wait() { notification_observer_.Wait(); }
+  void Wait() { run_loop_.Run(); }
 
- private:
-  // Callback which is used for |WindowedNotificationObserver| for checking
-  // whether the condition being awaited is met.
-  bool IsNotificationRelevant(
-      const content::NotificationSource& source,
-      const content::NotificationDetails& details) const {
-    extensions::CrxInstaller* const crx_installer =
-        content::Source<extensions::CrxInstaller>(source).ptr();
-    return crx_installer->profile() == profile_ &&
-           crx_installer->extension()->id() == extension_id_;
+  // extensions::InstallObserver:
+  void OnFinishCrxInstall(content::BrowserContext* context,
+                          const extensions::CrxInstaller& installer,
+                          const std::string& extension_id,
+                          bool success) override {
+    if (extension_id == extension_id_) {
+      run_loop_.Quit();
+    }
   }
 
-  const raw_ptr<const Profile, ExperimentalAsh> profile_;
+ private:
+  base::RunLoop run_loop_;
   const extensions::ExtensionId extension_id_;
-  content::WindowedNotificationObserver notification_observer_;
+  base::ScopedObservation<extensions::InstallTracker, InstallObserver>
+      observation_{this};
 };
 
 // Observer that allows waiting until the specified version of the given

diff --git a/chrome/browser/ash/power/renderer_freezer.cc b/chrome/browser/ash/power/renderer_freezer.cc
@@ -20,7 +20,6 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui.h"
 #include "extensions/browser/extension_registry.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/browser/process_map.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/permissions/api_permission.h"

diff --git a/chrome/browser/ash/power/renderer_freezer_unittest.cc b/chrome/browser/ash/power/renderer_freezer_unittest.cc
@@ -28,7 +28,6 @@
 #include "content/public/browser/site_instance.h"
 #include "content/public/test/browser_task_environment.h"
 #include "content/public/test/mock_render_process_host.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/browser/process_map.h"
 #include "extensions/common/extension_builder.h"

diff --git a/chrome/browser/certificate_provider/test_certificate_provider_extension.cc b/chrome/browser/certificate_provider/test_certificate_provider_extension.cc
@@ -29,7 +29,6 @@
 #include "crypto/rsa_private_key.h"
 #include "extensions/browser/api/test/test_api.h"
 #include "extensions/browser/event_router.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/common/api/test.h"
 #include "net/cert/asn1_util.h"
 #include "net/cert/x509_certificate.h"

diff --git a/chrome/browser/chrome_notification_types.h b/chrome/browser/chrome_notification_types.h
@@ -7,19 +7,8 @@
 
 #include "build/build_config.h"
 #include "build/chromeos_buildflags.h"
-#include "extensions/buildflags/buildflags.h"
-
-#if BUILDFLAG(ENABLE_EXTENSIONS)
-#include "extensions/browser/notification_types.h"
-#else
 #include "content/public/browser/notification_types.h"
-#endif
-
-#if BUILDFLAG(ENABLE_EXTENSIONS)
-#define PREVIOUS_END extensions::NOTIFICATION_EXTENSIONS_END
-#else
-#define PREVIOUS_END content::NOTIFICATION_CONTENT_END
-#endif
+#include "extensions/buildflags/buildflags.h"
 
 // **
 // ** NOTICE
@@ -35,7 +24,7 @@
 namespace chrome {
 
 enum NotificationType {
-  NOTIFICATION_CHROME_START = PREVIOUS_END,
+  NOTIFICATION_CHROME_START = content::NOTIFICATION_CONTENT_END,
 
   // Authentication ----------------------------------------------------------
 

diff --git a/chrome/browser/extensions/api/commands/command_service.cc b/chrome/browser/extensions/api/commands/command_service.cc
@@ -29,7 +29,6 @@
 #include "extensions/browser/extension_function_registry.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_system.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/common/api/commands/commands_handler.h"
 #include "extensions/common/command.h"
 #include "extensions/common/feature_switch.h"

diff --git a/chrome/browser/extensions/api/developer_private/developer_private_api.cc b/chrome/browser/extensions/api/developer_private/developer_private_api.cc
@@ -90,7 +90,6 @@
 #include "extensions/browser/extension_util.h"
 #include "extensions/browser/file_highlighter.h"
 #include "extensions/browser/management_policy.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/browser/path_util.h"
 #include "extensions/browser/permissions_manager.h"
 #include "extensions/browser/process_manager_factory.h"

diff --git a/chrome/browser/extensions/api/image_writer_private/operation_manager.cc b/chrome/browser/extensions/api/image_writer_private/operation_manager.cc
@@ -25,7 +25,6 @@
 #include "extensions/browser/api/extensions_api_client.h"
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_host.h"
-#include "extensions/browser/notification_types.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 
 #if BUILDFLAG(IS_CHROMEOS_ASH)

diff --git a/chrome/browser/extensions/api/management/management_api_browsertest.cc b/chrome/browser/extensions/api/management/management_api_browsertest.cc
@@ -32,7 +32,6 @@
 #include "extensions/browser/extension_host_test_helper.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/common/extension_builder.h"
 #include "extensions/common/extension_id.h"
 #include "extensions/test/extension_test_message_listener.h"

diff --git a/chrome/browser/extensions/browsertest_util.cc b/chrome/browser/extensions/browsertest_util.cc
@@ -28,7 +28,6 @@
 #include "content/public/test/test_utils.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/common/extension.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"

diff --git a/chrome/browser/extensions/chrome_extension_test_notification_observer.cc b/chrome/browser/extensions/chrome_extension_test_notification_observer.cc
@@ -12,9 +12,9 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "content/public/browser/browser_context.h"
+#include "content/public/browser/notification_types.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/test_utils.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/common/extension.h"
 

diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
@@ -42,7 +42,6 @@
 #include "components/crx_file/crx_verifier.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/notification_service.h"
 #include "extensions/browser/content_verifier.h"
 #include "extensions/browser/extension_file_task_runner.h"
 #include "extensions/browser/extension_prefs.h"
@@ -52,7 +51,6 @@
 #include "extensions/browser/install/extension_install_ui.h"
 #include "extensions/browser/install_flag.h"
 #include "extensions/browser/install_stage.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/browser/policy_check.h"
 #include "extensions/browser/preload_check_group.h"
 #include "extensions/browser/requirements_checker.h"
@@ -979,12 +977,6 @@ void CrxInstaller::ReportFailureFromUIThread(const CrxInstallError& error) {
   if (!service_weak_.get() || service_weak_->browser_terminating())
     return;
 
-  content::NotificationService* service =
-      content::NotificationService::current();
-  service->Notify(NOTIFICATION_EXTENSION_INSTALL_ERROR,
-                  content::Source<CrxInstaller>(this),
-                  content::Details<const CrxInstallError>(&error));
-
   // This isn't really necessary, it is only used because unit tests expect to
   // see errors get reported via this interface.
   //

diff --git a/chrome/browser/extensions/crx_installer_browsertest.cc b/chrome/browser/extensions/crx_installer_browsertest.cc
@@ -58,7 +58,6 @@
 #include "extensions/browser/install/crx_install_error.h"
 #include "extensions/browser/install/sandboxed_unpacker_failure_reason.h"
 #include "extensions/browser/management_policy.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/browser/permissions_manager.h"
 #include "extensions/browser/renderer_startup_helper.h"
 #include "extensions/browser/test_extension_registry_observer.h"

diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc
@@ -72,7 +72,6 @@
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/browser/service_worker/service_worker_test_utils.h"
 #include "extensions/browser/test_extension_registry_observer.h"
 #include "extensions/browser/uninstall_reason.h"

diff --git a/chrome/browser/extensions/extension_service_test_with_install.cc b/chrome/browser/extensions/extension_service_test_with_install.cc
@@ -18,7 +18,6 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/test/browser_task_environment.h"
 #include "extensions/browser/extension_creator.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/common/verifier_formats.h"
 #include "testing/gtest/include/gtest/gtest.h"
 

diff --git a/chrome/browser/extensions/load_error_reporter.cc b/chrome/browser/extensions/load_error_reporter.cc
@@ -18,7 +18,6 @@
 #include "chrome/browser/ui/simple_message_box.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/notification_service.h"
-#include "extensions/browser/notification_types.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace extensions {

diff --git a/chrome/browser/policy/extension_policy_browsertest.cc b/chrome/browser/policy/extension_policy_browsertest.cc
@@ -72,7 +72,6 @@
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/extensions_browser_client.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/browser/scoped_ignore_content_verifier_for_test.h"
 #include "extensions/browser/test_extension_registry_observer.h"
 #include "extensions/browser/updater/extension_cache_fake.h"

diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
@@ -129,7 +129,6 @@
 // Delayed warnings feature checks if the Suspicious Site Reporter extension
 // is installed. These includes are to fake-install this extension.
 #include "chrome/browser/extensions/crx_installer.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/browser/test_extension_registry_observer.h"
 #include "extensions/common/extension.h"
 #endif

diff --git a/chrome/browser/ui/views/extensions/extensions_toolbar_unittest.cc b/chrome/browser/ui/views/extensions/extensions_toolbar_unittest.cc
@@ -17,7 +17,6 @@
 #include "components/crx_file/id_util.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/test/test_utils.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_builder.h"
 #include "extensions/common/mojom/manifest.mojom-shared.h"

diff --git a/extensions/browser/BUILD.gn b/extensions/browser/BUILD.gn
@@ -484,7 +484,6 @@ source_set("browser_sources") {
     "mojo/keep_alive_impl.h",
     "network_permissions_updater.cc",
     "network_permissions_updater.h",
-    "notification_types.h",
     "null_app_sorting.cc",
     "null_app_sorting.h",
     "offscreen_document_host.cc",

diff --git a/extensions/browser/app_window/app_window.cc b/extensions/browser/app_window/app_window.cc
@@ -41,7 +41,6 @@
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/extension_web_contents_observer.h"
 #include "extensions/browser/extensions_browser_client.h"
-#include "extensions/browser/notification_types.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/browser/suggest_permission_util.h"
 #include "extensions/browser/view_type_utils.h"

diff --git a/extensions/browser/notification_types.h b/extensions/browser/notification_types.h