[code health] extensions: Remove some NotificationService usage.

Use RenderProcessHostObserver and RenderProcessHostCreationObserver
APIs, to replace the usage of NotificationService. More details in
the bug.

Bug: 357627
Change-Id: I57d75492ac13b7da014833c24be52f8c37fe531b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1842611Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703357}

extensions/browser/api/automation_internal/automation_event_router.cc

@@ -39,10 +39,6 @@ AutomationEventRouter::AutomationEventRouter()
     : active_context_(ExtensionsAPIClient::Get()
                           ->GetAutomationInternalApiDelegate()
                           ->GetActiveUserContext()) {
-  registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
-                 content::NotificationService::AllBrowserContextsAndSources());
-  registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
-                 content::NotificationService::AllBrowserContextsAndSources());
 #if defined(USE_AURA)
   // Not reset because |this| is leaked.
   ExtensionsAPIClient::Get()
@@ -201,6 +197,7 @@ void AutomationEventRouter::Register(const ExtensionId& extension_id,
     if (!desktop)
       listener.tree_ids.insert(ax_tree_id);
     listeners_.push_back(listener);
+    rph_observers_.Add(content::RenderProcessHost::FromID(listener_process_id));
     UpdateActiveProfile();
     return;
   }
@@ -227,22 +224,19 @@ void AutomationEventRouter::DispatchAccessibilityEvents(
   DispatchAccessibilityEvents(event_bundle);
 }
 
-void AutomationEventRouter::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  if (type != content::NOTIFICATION_RENDERER_PROCESS_TERMINATED &&
-      type != content::NOTIFICATION_RENDERER_PROCESS_CLOSED) {
-    NOTREACHED();
-    return;
-  }
+void AutomationEventRouter::RenderProcessExited(
+    content::RenderProcessHost* host,
+    const content::ChildProcessTerminationInfo& info) {
+  RenderProcessHostDestroyed(host);
+}
 
-  content::RenderProcessHost* rph =
-      content::Source<content::RenderProcessHost>(source).ptr();
-  int process_id = rph->GetID();
+void AutomationEventRouter::RenderProcessHostDestroyed(
+    content::RenderProcessHost* host) {
+  int process_id = host->GetID();
   base::EraseIf(listeners_, [process_id](const AutomationListener& item) {
     return item.process_id == process_id;
   });
+  rph_observers_.Remove(host);
   UpdateActiveProfile();
 }
 

extensions/browser/api/automation_internal/automation_event_router.h

@@ -10,9 +10,9 @@
 
 #include "base/macros.h"
 #include "base/memory/singleton.h"
+#include "base/scoped_observer.h"
 #include "content/public/browser/ax_event_notification_details.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/render_process_host_observer.h"
 #include "extensions/browser/api/automation_internal/automation_event_router_interface.h"
 #include "extensions/common/api/automation_internal.h"
 #include "extensions/common/extension_id.h"
@@ -35,7 +35,7 @@ namespace extensions {
 struct AutomationListener;
 
 class AutomationEventRouter : public ui::AXEventBundleSink,
-                              public content::NotificationObserver,
+                              public content::RenderProcessHostObserver,
                               public AutomationEventRouterInterface {
  public:
   static AutomationEventRouter* GetInstance();
@@ -104,10 +104,11 @@ class AutomationEventRouter : public ui::AXEventBundleSink,
                                    const gfx::Point& mouse_location,
                                    std::vector<ui::AXEvent> events) override;
 
-  // content::NotificationObserver interface.
-  void Observe(int type,
-               const content::NotificationSource& source,
-               const content::NotificationDetails& details) override;
+  // RenderProcessHostObserver:
+  void RenderProcessExited(
+      content::RenderProcessHost* host,
+      const content::ChildProcessTerminationInfo& info) override;
+  void RenderProcessHostDestroyed(content::RenderProcessHost* host) override;
 
   // Called when the user switches profiles or when a listener is added
   // or removed. The purpose is to ensure that multiple instances of the
@@ -126,6 +127,9 @@ class AutomationEventRouter : public ui::AXEventBundleSink,
 
   content::BrowserContext* active_context_;
 
+  ScopedObserver<content::RenderProcessHost, content::RenderProcessHostObserver>
+      rph_observers_{this};
+
   friend struct base::DefaultSingletonTraits<AutomationEventRouter>;
 
   DISALLOW_COPY_AND_ASSIGN(AutomationEventRouter);

extensions/browser/renderer_startup_helper.cc

@@ -61,35 +61,27 @@ bool IsExtensionVisibleToContext(const Extension& extension,
 RendererStartupHelper::RendererStartupHelper(BrowserContext* browser_context)
     : browser_context_(browser_context) {
   DCHECK(browser_context);
-  registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CREATED,
-                 content::NotificationService::AllBrowserContextsAndSources());
-  registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
-                 content::NotificationService::AllBrowserContextsAndSources());
-  registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
-                 content::NotificationService::AllBrowserContextsAndSources());
 }
 
-RendererStartupHelper::~RendererStartupHelper() {}
-
-void RendererStartupHelper::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  switch (type) {
-    case content::NOTIFICATION_RENDERER_PROCESS_CREATED:
-      InitializeProcess(
-          content::Source<content::RenderProcessHost>(source).ptr());
-      break;
-    case content::NOTIFICATION_RENDERER_PROCESS_TERMINATED:
-    // Fall through.
-    case content::NOTIFICATION_RENDERER_PROCESS_CLOSED:
-      // This is needed to take care of the case when a RenderProcessHost is
-      // reused for a different renderer process.
-      UntrackProcess(content::Source<content::RenderProcessHost>(source).ptr());
-      break;
-    default:
-      NOTREACHED() << "Unexpected notification: " << type;
-  }
+RendererStartupHelper::~RendererStartupHelper() {
+  for (auto* process : initialized_processes_)
+    process->RemoveObserver(this);
+}
+
+void RendererStartupHelper::OnRenderProcessHostCreated(
+    content::RenderProcessHost* host) {
+  InitializeProcess(host);
+}
+
+void RendererStartupHelper::RenderProcessExited(
+    content::RenderProcessHost* host,
+    const content::ChildProcessTerminationInfo& info) {
+  UntrackProcess(host);
+}
+
+void RendererStartupHelper::RenderProcessHostDestroyed(
+    content::RenderProcessHost* host) {
+  UntrackProcess(host);
 }
 
 void RendererStartupHelper::InitializeProcess(
@@ -181,6 +173,7 @@ void RendererStartupHelper::InitializeProcess(
 
   initialized_processes_.insert(process);
   pending_active_extensions_.erase(process);
+  process->AddObserver(this);
 }
 
 void RendererStartupHelper::UntrackProcess(
@@ -190,6 +183,7 @@ void RendererStartupHelper::UntrackProcess(
     return;
   }
 
+  process->RemoveObserver(this);
   initialized_processes_.erase(process);
   pending_active_extensions_.erase(process);
   for (auto& extension_process_pair : extension_process_map_)

extensions/browser/renderer_startup_helper.h

@@ -13,8 +13,8 @@
 #include "base/memory/singleton.h"
 #include "components/keyed_service/content/browser_context_keyed_service_factory.h"
 #include "components/keyed_service/core/keyed_service.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/render_process_host_creation_observer.h"
+#include "content/public/browser/render_process_host_observer.h"
 #include "extensions/common/extension_id.h"
 
 namespace content {
@@ -35,16 +35,22 @@ class Extension;
 // TODO(devlin): "StartupHelper" is no longer sufficient to describe the entire
 // behavior of this class.
 class RendererStartupHelper : public KeyedService,
-                              public content::NotificationObserver {
+                              public content::RenderProcessHostCreationObserver,
+                              public content::RenderProcessHostObserver {
  public:
   // This class sends messages to all renderers started for |browser_context|.
   explicit RendererStartupHelper(content::BrowserContext* browser_context);
   ~RendererStartupHelper() override;
 
-  // content::NotificationObserver overrides:
-  void Observe(int type,
-               const content::NotificationSource& source,
-               const content::NotificationDetails& details) override;
+  // content::RenderProcessHostCreationObserver:
+  void OnRenderProcessHostCreated(
+      content::RenderProcessHost* process_host) override;
+
+  // content::RenderProcessHostObserver:
+  void RenderProcessExited(
+      content::RenderProcessHost* host,
+      const content::ChildProcessTerminationInfo& info) override;
+  void RenderProcessHostDestroyed(content::RenderProcessHost* host) override;
 
   // Sends a message to the specified |process| activating the given extension
   // once the process is initialized. OnExtensionLoaded should have already been
@@ -87,8 +93,6 @@ class RendererStartupHelper : public KeyedService,
   std::map<content::RenderProcessHost*, std::set<ExtensionId>>
       pending_active_extensions_;
 
-  content::NotificationRegistrar registrar_;
-
   DISALLOW_COPY_AND_ASSIGN(RendererStartupHelper);
 };
 

extensions/browser/renderer_startup_helper_unittest.cc

@@ -6,8 +6,6 @@
 
 #include "base/stl_util.h"
 #include "components/crx_file/id_util.h"
-#include "content/public/browser/notification_service.h"
-#include "content/public/browser/notification_types.h"
 #include "content/public/test/mock_render_process_host.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
@@ -46,17 +44,11 @@ class RendererStartupHelperTest : public ExtensionsTest {
 
  protected:
   void SimulateRenderProcessCreated(content::RenderProcessHost* rph) {
-    content::NotificationService::current()->Notify(
-        content::NOTIFICATION_RENDERER_PROCESS_CREATED,
-        content::Source<content::RenderProcessHost>(rph),
-        content::NotificationService::NoDetails());
+    helper_->OnRenderProcessHostCreated(rph);
   }
 
   void SimulateRenderProcessTerminated(content::RenderProcessHost* rph) {
-    content::NotificationService::current()->Notify(
-        content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
-        content::Source<content::RenderProcessHost>(rph),
-        content::NotificationService::NoDetails());
+    helper_->RenderProcessHostDestroyed(rph);
   }
 
   scoped_refptr<const Extension> CreateExtension(const std::string& id_input) {

extensions/browser/user_script_loader.cc

@@ -165,9 +165,6 @@ UserScriptLoader::UserScriptLoader(BrowserContext* browser_context,
       queued_load_(false),
       browser_context_(browser_context),
       host_id_(host_id) {
-  registrar_.Add(this,
-                 content::NOTIFICATION_RENDERER_PROCESS_CREATED,
-                 content::NotificationService::AllBrowserContextsAndSources());
 }
 
 UserScriptLoader::~UserScriptLoader() {
@@ -216,17 +213,13 @@ void UserScriptLoader::ClearScripts() {
   AttemptLoad();
 }
 
-void UserScriptLoader::Observe(int type,
-                               const content::NotificationSource& source,
-                               const content::NotificationDetails& details) {
-  DCHECK_EQ(type, content::NOTIFICATION_RENDERER_PROCESS_CREATED);
-  content::RenderProcessHost* process =
-      content::Source<content::RenderProcessHost>(source).ptr();
+void UserScriptLoader::OnRenderProcessHostCreated(
+    content::RenderProcessHost* process_host) {
   if (!ExtensionsBrowserClient::Get()->IsSameContext(
-          browser_context_, process->GetBrowserContext()))
+          browser_context_, process_host->GetBrowserContext()))
     return;
   if (initial_load_complete()) {
-    SendUpdate(process, shared_memory_,
+    SendUpdate(process_host, shared_memory_,
                std::set<HostID>());  // Include all hosts.
   }
 }

extensions/browser/user_script_loader.h

@@ -16,8 +16,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/scoped_observer.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/render_process_host_creation_observer.h"
 #include "extensions/common/host_id.h"
 #include "extensions/common/user_script.h"
 
@@ -39,7 +38,7 @@ namespace extensions {
 // of this class are embedded within classes with names ending in
 // UserScriptMaster. These "master" classes implement the strategy for which
 // scripts to load/unload on this logical unit of scripts.
-class UserScriptLoader : public content::NotificationObserver {
+class UserScriptLoader : public content::RenderProcessHostCreationObserver {
  public:
   using LoadScriptsCallback =
       base::OnceCallback<void(std::unique_ptr<UserScriptList>,
@@ -113,10 +112,9 @@ class UserScriptLoader : public content::NotificationObserver {
   const HostID& host_id() const { return host_id_; }
 
  private:
-  // content::NotificationObserver implementation.
-  void Observe(int type,
-               const content::NotificationSource& source,
-               const content::NotificationDetails& details) override;
+  // content::RenderProcessHostCreationObserver:
+  void OnRenderProcessHostCreated(
+      content::RenderProcessHost* process_host) override;
 
   // Returns whether or not it is possible that calls to AddScripts(),
   // RemoveScripts(), and/or ClearScripts() have caused any real change in the
@@ -143,9 +141,6 @@ class UserScriptLoader : public content::NotificationObserver {
     return loaded_scripts_.get() == nullptr;
   }
 
-  // Manages our notification registrations.
-  content::NotificationRegistrar registrar_;
-
   // Contains the scripts that were found the last time scripts were updated.
   base::ReadOnlySharedMemoryRegion shared_memory_;