[code health] components/crash: Remove use of NotificationService

Use RenderProcessHostCreationObserver and RenderProcessHostObserver
APIs, instead of using NotificationService.

BUG=357627

Change-Id: Ia3755cd64056484c9b703b31d5a79489785f0e56
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1842556Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708488}

chrome/browser/metrics/oom/out_of_memory_reporter_unittest.cc

@@ -122,6 +122,11 @@ class OutOfMemoryReporterTest : public ChromeRenderViewHostTestHarness,
   }
 
   void SimulateRendererCreated() {
+#if defined(OS_ANDROID)
+    content::RenderProcessHostCreationObserver* creation_observer =
+        crash_reporter::ChildExitObserver::GetInstance();
+    creation_observer->OnRenderProcessHostCreated(process());
+#endif
     content::NotificationService::current()->Notify(
         content::NOTIFICATION_RENDERER_PROCESS_CREATED,
         content::Source<content::RenderProcessHost>(process()),

components/crash/content/browser/child_exit_observer_android.cc

@@ -8,13 +8,12 @@
 
 #include "base/bind.h"
 #include "base/logging.h"
+#include "base/no_destructor.h"
 #include "base/stl_util.h"
 #include "components/crash/content/browser/crash_memory_metrics_collector_android.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/child_process_data.h"
 #include "content/public/browser/child_process_termination_info.h"
-#include "content/public/browser/notification_service.h"
-#include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
 
 using content::BrowserThread;
@@ -23,8 +22,27 @@ namespace crash_reporter {
 
 namespace {
 
-base::LazyInstance<ChildExitObserver>::DestructorAtExit g_instance =
-    LAZY_INSTANCE_INITIALIZER;
+void PopulateTerminationInfoForRenderProcessHost(
+    content::RenderProcessHost* rph,
+    ChildExitObserver::TerminationInfo* info) {
+  info->process_host_id = rph->GetID();
+  info->pid = rph->GetProcess().Handle();
+  info->process_type = content::PROCESS_TYPE_RENDERER;
+  info->app_state = base::android::APPLICATION_STATE_UNKNOWN;
+  info->renderer_has_visible_clients = rph->VisibleClientCount() > 0;
+  info->renderer_was_subframe = rph->GetFrameDepth() > 0u;
+  CrashMemoryMetricsCollector* collector =
+      CrashMemoryMetricsCollector::GetFromRenderProcessHost(rph);
+
+  // CrashMemoryMetircsCollector is created in chrome_content_browser_client,
+  // and does not exist in non-chrome platforms such as android webview /
+  // chromecast.
+  if (collector) {
+    // SharedMemory creation / Map() might fail.
+    DCHECK(collector->MemoryMetrics());
+    info->blink_oom_metrics = *collector->MemoryMetrics();
+  }
+}
 
 void PopulateTerminationInfo(
     const content::ChildProcessTerminationInfo& content_info,
@@ -46,6 +64,13 @@ void PopulateTerminationInfo(
   info->renderer_was_subframe = content_info.renderer_was_subframe;
 }
 
+ChildExitObserver* CreateSingletonInstance() {
+  static base::NoDestructor<ChildExitObserver> s_instance;
+  return s_instance.get();
+}
+
+ChildExitObserver* g_instance = nullptr;
+
 }  // namespace
 
 ChildExitObserver::TerminationInfo::TerminationInfo() = default;
@@ -60,34 +85,24 @@ void ChildExitObserver::Create() {
   // If this DCHECK fails in a unit test then a previously executing
   // test that makes use of ChildExitObserver forgot to create a
   // ShadowingAtExitManager.
-  DCHECK(!g_instance.IsCreated());
-  g_instance.Get();
+  DCHECK(!g_instance);
+  g_instance = CreateSingletonInstance();
 }
 
 // static
 ChildExitObserver* ChildExitObserver::GetInstance() {
-  DCHECK(g_instance.IsCreated());
-  return g_instance.Pointer();
+  DCHECK(g_instance);
+  return g_instance;
 }
 
 ChildExitObserver::ChildExitObserver()
-    : notification_registrar_(),
-      registered_clients_lock_(),
+    : registered_clients_lock_(),
       registered_clients_(),
       process_host_id_to_pid_(),
       browser_child_process_info_(),
       crash_signals_lock_(),
       child_pid_to_crash_signal_(),
       scoped_observer_(this) {
-  notification_registrar_.Add(this,
-                              content::NOTIFICATION_RENDERER_PROCESS_CREATED,
-                              content::NotificationService::AllSources());
-  notification_registrar_.Add(this,
-                              content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
-                              content::NotificationService::AllSources());
-  notification_registrar_.Add(this,
-                              content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
-                              content::NotificationService::AllSources());
   BrowserChildProcessObserver::Add(this);
   scoped_observer_.Add(crashpad::CrashHandlerHost::Get());
 }
@@ -168,70 +183,48 @@ void ChildExitObserver::BrowserChildProcessKilled(
   // Subsequent BrowserChildProcessHostDisconnected will call OnChildExit.
 }
 
-void ChildExitObserver::Observe(int type,
-                                const content::NotificationSource& source,
-                                const content::NotificationDetails& details) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  content::RenderProcessHost* rph =
-      content::Source<content::RenderProcessHost>(source).ptr();
-  TerminationInfo info;
-  info.process_host_id = rph->GetID();
-  info.pid = rph->GetProcess().Handle();
-  info.process_type = content::PROCESS_TYPE_RENDERER;
-  info.app_state = base::android::APPLICATION_STATE_UNKNOWN;
-  info.renderer_has_visible_clients = rph->VisibleClientCount() > 0;
-  info.renderer_was_subframe = rph->GetFrameDepth() > 0u;
-  CrashMemoryMetricsCollector* collector =
-      CrashMemoryMetricsCollector::GetFromRenderProcessHost(rph);
-
-  // CrashMemoryMetircsCollector is created in chrome_content_browser_client,
-  // and does not exist in non-chrome platforms such as android webview /
-  // chromecast.
-  if (collector) {
-    // SharedMemory creation / Map() might fail.
-    DCHECK(collector->MemoryMetrics());
-    info.blink_oom_metrics = *collector->MemoryMetrics();
-  }
-  switch (type) {
-    case content::NOTIFICATION_RENDERER_PROCESS_TERMINATED: {
-      // NOTIFICATION_RENDERER_PROCESS_TERMINATED is sent when the renderer
-      // process is cleanly shutdown.
-      info.normal_termination = true;
-      break;
-    }
-    case content::NOTIFICATION_RENDERER_PROCESS_CLOSED: {
-      // We do not care about android fast shutdowns as it is a known case where
-      // the renderer is intentionally killed when we are done with it.
-      info.normal_termination = rph->FastShutdownStarted();
-      info.app_state = base::android::ApplicationStatusListener::GetState();
-      const auto& content_info =
-          *content::Details<content::ChildProcessTerminationInfo>(details)
-               .ptr();
-      PopulateTerminationInfo(content_info, &info);
-      break;
-    }
-    case content::NOTIFICATION_RENDERER_PROCESS_CREATED: {
-      // The child process pid isn't available when process is gone, keep a
-      // mapping between process_host_id and pid, so we can find it later.
+void ChildExitObserver::OnRenderProcessHostCreated(
+    content::RenderProcessHost* rph) {
   process_host_id_to_pid_[rph->GetID()] = rph->GetProcess().Handle();
-      return;
-    }
-    default:
-      NOTREACHED();
-      return;
-  }
-  const auto& iter = process_host_id_to_pid_.find(rph->GetID());
-  // NOTIFICATION_RENDERER_PROCESS_CLOSED corresponds to death of an underlying
-  // RenderProcess. NOTIFICATION_RENDERER_PROCESS_TERMINATED corresponds to when
-  // the RenderProcessHost's lifetime is ending. Ideally, we'd only listen to
-  // the former, but if the RenderProcessHost is destroyed before the
-  // RenderProcess, then the former is never sent.
+  rph_observers_.Add(rph);
+}
+
+void ChildExitObserver::RenderProcessExited(
+    content::RenderProcessHost* host,
+    const content::ChildProcessTerminationInfo& termination_info) {
+  OnRenderProcessHostGone(host, termination_info);
+}
+
+void ChildExitObserver::RenderProcessHostDestroyed(
+    content::RenderProcessHost* host) {
+  OnRenderProcessHostGone(host, base::nullopt);
+}
+
+void ChildExitObserver::OnRenderProcessHostGone(
+    content::RenderProcessHost* host,
+    base::Optional<content::ChildProcessTerminationInfo> termination_info) {
+  const auto& iter = process_host_id_to_pid_.find(host->GetID());
   if (iter == process_host_id_to_pid_.end()) {
     return;
   }
+
+  rph_observers_.Remove(host);
+  TerminationInfo info;
+  PopulateTerminationInfoForRenderProcessHost(host, &info);
   if (info.pid == base::kNullProcessHandle) {
     info.pid = iter->second;
   }
+
+  if (termination_info.has_value()) {
+    // We do not care about android fast shutdowns as it is a known case where
+    // the renderer is intentionally killed when we are done with it.
+    info.normal_termination = host->FastShutdownStarted();
+    info.app_state = base::android::ApplicationStatusListener::GetState();
+    PopulateTerminationInfo(*termination_info, &info);
+  } else {
+    info.normal_termination = true;
+  }
+
   process_host_id_to_pid_.erase(iter);
   OnChildExit(&info);
 }

components/crash/content/browser/child_exit_observer_android.h

@@ -11,14 +11,16 @@
 
 #include "base/android/application_status_listener.h"
 #include "base/android/child_process_binding_types.h"
-#include "base/lazy_instance.h"
+#include "base/no_destructor.h"
+#include "base/optional.h"
 #include "base/process/process.h"
 #include "base/scoped_observer.h"
 #include "base/synchronization/lock.h"
 #include "components/crash/content/browser/crash_handler_host_linux.h"
 #include "content/public/browser/browser_child_process_observer.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/child_process_termination_info.h"
+#include "content/public/browser/render_process_host_creation_observer.h"
+#include "content/public/browser/render_process_host_observer.h"
 #include "content/public/common/child_process_host.h"
 #include "content/public/common/process_type.h"
 #include "third_party/blink/public/common/oom_intervention/oom_intervention_types.h"
@@ -33,7 +35,8 @@ namespace crash_reporter {
 // purpose of reacting to child process crashes.
 // The ChildExitObserver instance exists on the browser main thread.
 class ChildExitObserver : public content::BrowserChildProcessObserver,
-                          public content::NotificationObserver,
+                          public content::RenderProcessHostCreationObserver,
+                          public content::RenderProcessHostObserver,
                           public crashpad::CrashHandlerHost::Observer {
  public:
   struct TerminationInfo {
@@ -131,7 +134,7 @@ class ChildExitObserver : public content::BrowserChildProcessObserver,
   void ChildReceivedCrashSignal(base::ProcessId pid, int signo) override;
 
  private:
-  friend struct base::LazyInstanceTraitsBase<ChildExitObserver>;
+  friend class base::NoDestructor<ChildExitObserver>;
 
   ChildExitObserver();
   ~ChildExitObserver() override;
@@ -143,15 +146,23 @@ class ChildExitObserver : public content::BrowserChildProcessObserver,
       const content::ChildProcessData& data,
       const content::ChildProcessTerminationInfo& info) override;
 
-  // NotificationObserver implementation:
-  void Observe(int type,
-               const content::NotificationSource& source,
-               const content::NotificationDetails& details) override;
+  // RenderProcessHostCreationObserver:
+  void OnRenderProcessHostCreated(
+      content::RenderProcessHost* process_host) override;
+
+  // RenderProcessHostObserver:
+  void RenderProcessExited(
+      content::RenderProcessHost* host,
+      const content::ChildProcessTerminationInfo& info) override;
+  void RenderProcessHostDestroyed(content::RenderProcessHost* host) override;
 
   // Called on child process exit (including crash).
   void OnChildExit(TerminationInfo* info);
 
-  content::NotificationRegistrar notification_registrar_;
+  // Called on RenderProcessHost removal.
+  void OnRenderProcessHostGone(
+      content::RenderProcessHost* host,
+      base::Optional<content::ChildProcessTerminationInfo> termination_info);
 
   base::Lock registered_clients_lock_;
   std::vector<std::unique_ptr<Client>> registered_clients_;
@@ -169,6 +180,9 @@ class ChildExitObserver : public content::BrowserChildProcessObserver,
                  crashpad::CrashHandlerHost::Observer>
       scoped_observer_;
 
+  ScopedObserver<content::RenderProcessHost, content::RenderProcessHostObserver>
+      rph_observers_{this};
+
   DISALLOW_COPY_AND_ASSIGN(ChildExitObserver);
 };