Introduce RenderProcessHostObserver, use it in its first client.

BUG=170921
TEST=everything still works

Review URL: https://codereview.chromium.org/72203003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@235877 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 153 insertions, 38 deletions

diff --git a/content/browser/browser_main_loop.cc b/content/browser/browser_main_loop.cc
index 3addee3..7b7a529 100644
--- a/content/browser/browser_main_loop.cc
+++ b/content/browser/browser_main_loop.cc
@@ -38,6 +38,7 @@
 #include "content/browser/plugin_service_impl.h"
 #include "content/browser/renderer_host/media/audio_mirroring_manager.h"
 #include "content/browser/renderer_host/media/media_stream_manager.h"
+#include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/speech/speech_recognition_manager_impl.h"
 #include "content/browser/startup_task_runner.h"
 #include "content/browser/tracing/trace_controller_impl.h"
@@ -740,10 +741,8 @@ void BrowserMainLoop::ShutdownThreadsAndCleanUp() {
       base::Bind(base::IgnoreResult(&base::ThreadRestrictions::SetIOAllowed),
                  true));
 
-  if (RenderProcessHost::run_renderer_in_process() &&
-      !RenderProcessHost::AllHostsIterator().IsAtEnd()) {
-    delete RenderProcessHost::AllHostsIterator().GetCurrentValue();
-  }
+  if (RenderProcessHost::run_renderer_in_process())
+    RenderProcessHostImpl::ShutDownInProcessRenderer();
 
   if (parts_) {
     TRACE_EVENT0("shutdown",
diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
index 6070b22..ff5966d 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -117,6 +117,7 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host_factory.h"
+#include "content/public/browser/render_process_host_observer.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/resource_context.h"
@@ -359,6 +360,9 @@ RenderProcessHostImpl::RenderProcessHostImpl(
     bool is_guest)
         : fast_shutdown_started_(false),
           deleting_soon_(false),
+#ifndef NDEBUG
+          is_self_deleted_(false),
+#endif
           pending_views_(0),
           visible_widgets_(0),
           backgrounded_(true),
@@ -399,7 +403,37 @@ RenderProcessHostImpl::RenderProcessHostImpl(
   //       creation.
 }
 
+// static
+void RenderProcessHostImpl::ShutDownInProcessRenderer() {
+  DCHECK(g_run_renderer_in_process_);
+
+  switch (g_all_hosts.Pointer()->size()) {
+    case 0:
+      return;
+    case 1: {
+      RenderProcessHostImpl* host = static_cast<RenderProcessHostImpl*>(
+          AllHostsIterator().GetCurrentValue());
+      FOR_EACH_OBSERVER(RenderProcessHostObserver,
+                        host->observers_,
+                        RenderProcessHostDestroyed(host));
+#ifndef NDEBUG
+      host->is_self_deleted_ = true;
+#endif
+      delete host;
+      return;
+    }
+    default:
+      NOTREACHED() << "There should be only one RenderProcessHost when running "
+                   << "in-process.";
+  }
+}
+
 RenderProcessHostImpl::~RenderProcessHostImpl() {
+#ifndef NDEBUG
+  DCHECK(is_self_deleted_)
+      << "RenderProcessHostImpl is destroyed by something other than itself";
+#endif
+
   ChildProcessSecurityPolicyImpl::GetInstance()->Remove(GetID());
 
   if (gpu_observer_registered_) {
@@ -771,6 +805,15 @@ void RenderProcessHostImpl::RemoveRoute(int32 routing_id) {
     Cleanup();
 }
 
+void RenderProcessHostImpl::AddObserver(RenderProcessHostObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void RenderProcessHostImpl::RemoveObserver(
+    RenderProcessHostObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 bool RenderProcessHostImpl::WaitForBackingStoreMsg(
     int render_widget_id,
     const base::TimeDelta& max_delay,
@@ -1358,14 +1401,20 @@ bool RenderProcessHostImpl::IgnoreInputEvents() const {
 }
 
 void RenderProcessHostImpl::Cleanup() {
-  // When no other owners of this object, we can delete ourselves
+  // When there are no other owners of this object, we can delete ourselves.
   if (listeners_.IsEmpty()) {
     DCHECK_EQ(0, pending_views_);
+    FOR_EACH_OBSERVER(RenderProcessHostObserver,
+                      observers_,
+                      RenderProcessHostDestroyed(this));
     NotificationService::current()->Notify(
         NOTIFICATION_RENDERER_PROCESS_TERMINATED,
         Source<RenderProcessHost>(this),
         NotificationService::NoDetails());
 
+#ifndef NDEBUG
+    is_self_deleted_ = true;
+#endif
     base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
     deleting_soon_ = true;
     // It's important not to wait for the DeleteTask to delete the channel
@@ -1514,6 +1563,7 @@ void RenderProcessHost::SetRunRendererInProcess(bool value) {
   }
 }
 
+// static
 RenderProcessHost::iterator RenderProcessHost::AllHostsIterator() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   return iterator(g_all_hosts.Pointer());
diff --git a/content/browser/renderer_host/render_process_host_impl.h b/content/browser/renderer_host/render_process_host_impl.h
index 681d329..2f1040a 100644
--- a/content/browser/renderer_host/render_process_host_impl.h
+++ b/content/browser/renderer_host/render_process_host_impl.h
@@ -10,6 +10,7 @@
 #include <string>
 
 #include "base/memory/scoped_ptr.h"
+#include "base/observer_list.h"
 #include "base/process/process.h"
 #include "base/timer/timer.h"
 #include "content/browser/child_process_launcher.h"
@@ -82,6 +83,8 @@ class CONTENT_EXPORT RenderProcessHostImpl
   virtual int GetNextRoutingID() OVERRIDE;
   virtual void AddRoute(int32 routing_id, IPC::Listener* listener) OVERRIDE;
   virtual void RemoveRoute(int32 routing_id) OVERRIDE;
+  virtual void AddObserver(RenderProcessHostObserver* observer) OVERRIDE;
+  virtual void RemoveObserver(RenderProcessHostObserver* observer) OVERRIDE;
   virtual bool WaitForBackingStoreMsg(int render_widget_id,
                                       const base::TimeDelta& max_delay,
                                       IPC::Message* msg) OVERRIDE;
@@ -183,6 +186,9 @@ class CONTENT_EXPORT RenderProcessHostImpl
 
   static base::MessageLoop* GetInProcessRendererThreadForTesting();
 
+  // This forces a renderer that is running "in process" to shut down.
+  static void ShutDownInProcessRenderer();
+
 #if defined(OS_ANDROID)
   const scoped_refptr<BrowserDemuxerAndroid>& browser_demuxer_android() {
     return browser_demuxer_android_;
@@ -204,6 +210,11 @@ class CONTENT_EXPORT RenderProcessHostImpl
   // True if we've posted a DeleteTask and will be deleted soon.
   bool deleting_soon_;
 
+#ifndef NDEBUG
+  // True if this object has deleted itself.
+  bool is_self_deleted_;
+#endif
+
   // The count of currently swapped out but pending RenderViews.  We have
   // started to swap these in, so the renderer process should not exit if
   // this count is non-zero.
@@ -307,6 +318,9 @@ class CONTENT_EXPORT RenderProcessHostImpl
   // Owned by |browser_context_|.
   StoragePartitionImpl* storage_partition_impl_;
 
+  // The observers watching our lifetime.
+  ObserverList<RenderProcessHostObserver> observers_;
+
   // True if the process can be shut down suddenly.  If this is true, then we're
   // sure that all the RenderViews in the process can be shutdown suddenly.  If
   // it's false, then specific RenderViews might still be allowed to be shutdown
diff --git a/content/browser/site_instance_impl.cc b/content/browser/site_instance_impl.cc
index 3682e40..29f5a78 100644
--- a/content/browser/site_instance_impl.cc
+++ b/content/browser/site_instance_impl.cc
@@ -10,8 +10,6 @@
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/storage_partition_impl.h"
 #include "content/public/browser/content_browser_client.h"
-#include "content/public/browser/notification_service.h"
-#include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host_factory.h"
 #include "content/public/browser/web_ui_controller_factory.h"
 #include "content/public/common/content_switches.h"
@@ -46,14 +44,14 @@ SiteInstanceImpl::SiteInstanceImpl(BrowsingInstance* browsing_instance)
       process_(NULL),
       has_site_(false) {
   DCHECK(browsing_instance);
-
-  registrar_.Add(this, NOTIFICATION_RENDERER_PROCESS_TERMINATED,
-                 NotificationService::AllBrowserContextsAndSources());
 }
 
 SiteInstanceImpl::~SiteInstanceImpl() {
   GetContentClient()->browser()->SiteInstanceDeleting(this);
 
+  if (process_)
+    process_->RemoveObserver(this);
+
   // Now that no one is referencing us, we can safely remove ourselves from
   // the BrowsingInstance.  Any future visits to a page from this site
   // (within the same BrowsingInstance) can safely create a new SiteInstance.
@@ -129,6 +127,7 @@ RenderProcessHost* SiteInstanceImpl::GetProcess() {
       }
     }
     CHECK(process_);
+    process_->AddObserver(this);
 
     // If we are using process-per-site, we need to register this process
     // for the current site so that we can find it again.  (If no site is set
@@ -326,13 +325,10 @@ GURL SiteInstanceImpl::GetEffectiveURL(BrowserContext* browser_context,
       GetEffectiveURL(browser_context, url);
 }
 
-void SiteInstanceImpl::Observe(int type,
-                               const NotificationSource& source,
-                               const NotificationDetails& details) {
-  DCHECK(type == NOTIFICATION_RENDERER_PROCESS_TERMINATED);
-  RenderProcessHost* rph = Source<RenderProcessHost>(source).ptr();
-  if (rph == process_)
-    process_ = NULL;
+void SiteInstanceImpl::RenderProcessHostDestroyed(RenderProcessHost* host) {
+  DCHECK_EQ(process_, host);
+  process_->RemoveObserver(this);
+  process_ = NULL;
 }
 
 void SiteInstanceImpl::LockToOrigin() {
diff --git a/content/browser/site_instance_impl.h b/content/browser/site_instance_impl.h
index 013e658..09249ee 100644
--- a/content/browser/site_instance_impl.h
+++ b/content/browser/site_instance_impl.h
@@ -7,8 +7,7 @@
 
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/common/content_export.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 "content/public/browser/site_instance.h"
 #include "url/gurl.h"
 
@@ -16,12 +15,12 @@ namespace content {
 class RenderProcessHostFactory;
 
 class CONTENT_EXPORT SiteInstanceImpl : public SiteInstance,
-                                        public NotificationObserver {
+                                        public RenderProcessHostObserver {
  public:
   // SiteInstance interface overrides.
   virtual int32 GetId() OVERRIDE;
   virtual bool HasProcess() const OVERRIDE;
-  virtual  RenderProcessHost* GetProcess() OVERRIDE;
+  virtual RenderProcessHost* GetProcess() OVERRIDE;
   virtual const GURL& GetSiteURL() const OVERRIDE;
   virtual SiteInstance* GetRelatedSiteInstance(const GURL& url) OVERRIDE;
   virtual bool IsRelatedSiteInstance(const SiteInstance* instance) OVERRIDE;
@@ -84,10 +83,8 @@ class CONTENT_EXPORT SiteInstanceImpl : public SiteInstance,
   static GURL GetEffectiveURL(BrowserContext* browser_context,
                               const GURL& url);
 
-  // NotificationObserver implementation.
-  virtual void Observe(int type,
-                       const NotificationSource& source,
-                       const NotificationDetails& details) OVERRIDE;
+  // RenderProcessHostObserver implementation.
+  virtual void RenderProcessHostDestroyed(RenderProcessHost* host) OVERRIDE;
 
   // Used to restrict a process' origin access rights.
   void LockToOrigin();
@@ -104,8 +101,6 @@ class CONTENT_EXPORT SiteInstanceImpl : public SiteInstance,
   // The number of active views under this SiteInstance.
   size_t active_view_count_;
 
-  NotificationRegistrar registrar_;
-
   // BrowsingInstance to which this SiteInstance belongs.
   scoped_refptr<BrowsingInstance> browsing_instance_;
 
diff --git a/content/content_browser.gypi b/content/content_browser.gypi
index f89c226..d5edf9f 100644
--- a/content/content_browser.gypi
+++ b/content/content_browser.gypi
@@ -148,6 +148,7 @@
     'public/browser/render_frame_host.h',
     'public/browser/render_process_host.h',
     'public/browser/render_process_host_factory.h',
+    'public/browser/render_process_host_observer.h',
     'public/browser/render_view_host.h',
     'public/browser/render_widget_host.h',
     'public/browser/render_widget_host_view.h',
diff --git a/content/public/browser/render_process_host.h b/content/public/browser/render_process_host.h
index 3aedc59..76558254 100644
--- a/content/public/browser/render_process_host.h
+++ b/content/public/browser/render_process_host.h
@@ -26,6 +26,7 @@ class TimeDelta;
 namespace content {
 class BrowserContext;
 class BrowserMessageFilter;
+class RenderProcessHostObserver;
 class RenderWidgetHost;
 class StoragePartition;
 
@@ -55,6 +56,8 @@ class CONTENT_EXPORT RenderProcessHost : public IPC::Sender,
     int exit_code;
   };
 
+  // General functions ---------------------------------------------------------
+
   virtual ~RenderProcessHost() {}
 
   // Initialize the new renderer process, returning true on success. This must
@@ -73,6 +76,12 @@ class CONTENT_EXPORT RenderProcessHost : public IPC::Sender,
   virtual void AddRoute(int32 routing_id, IPC::Listener* listener) = 0;
   virtual void RemoveRoute(int32 routing_id) = 0;
 
+  // Add and remove observers for lifecycle events. The order in which
+  // notifications are sent to observers is undefined. Observers must be sure to
+  // remove the observer before they go away.
+  virtual void AddObserver(RenderProcessHostObserver* observer) = 0;
+  virtual void RemoveObserver(RenderProcessHostObserver* observer) = 0;
+
   // Called to wait for the next UpdateRect message for the specified render
   // widget.  Returns true if successful, and the msg out-param will contain a
   // copy of the received UpdateRect message.
diff --git a/content/public/browser/render_process_host_observer.h b/content/public/browser/render_process_host_observer.h
new file mode 100644
index 0000000..2216737
--- /dev/null
+++ b/content/public/browser/render_process_host_observer.h
@@ -0,0 +1,27 @@
+// Copyright 2013 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 CONTENT_PUBLIC_BROWSER_RENDER_PROCESS_HOST_OBSERVER_H_
+#define CONTENT_PUBLIC_BROWSER_RENDER_PROCESS_HOST_OBSERVER_H_
+
+#include "content/common/content_export.h"
+
+namespace content {
+
+class RenderProcessHost;
+
+// An observer API implemented by classes which are interested
+// in RenderProcessHost lifecycle events.
+class CONTENT_EXPORT RenderProcessHostObserver {
+ public:
+  // Called when the observed RenderProcessHost itself is destroyed.
+  virtual void RenderProcessHostDestroyed(RenderProcessHost* host) {}
+
+ protected:
+  virtual ~RenderProcessHostObserver() {}
+};
+
+}  // namespace content
+
+#endif  // CONTENT_PUBLIC_BROWSER_RENDER_PROCESS_HOST_OBSERVER_H_
diff --git a/content/public/test/mock_render_process_host.cc b/content/public/test/mock_render_process_host.cc
index 20f44cc..21bcaf2 100644
--- a/content/public/test/mock_render_process_host.cc
+++ b/content/public/test/mock_render_process_host.cc
@@ -19,15 +19,15 @@
 
 namespace content {
 
-MockRenderProcessHost::MockRenderProcessHost(
-    BrowserContext* browser_context)
-        : transport_dib_(NULL),
-          bad_msg_count_(0),
-          factory_(NULL),
-          id_(ChildProcessHostImpl::GenerateChildProcessUniqueId()),
-          browser_context_(browser_context),
-          prev_routing_id_(0),
-          fast_shutdown_started_(false) {
+MockRenderProcessHost::MockRenderProcessHost(BrowserContext* browser_context)
+    : transport_dib_(NULL),
+      bad_msg_count_(0),
+      factory_(NULL),
+      id_(ChildProcessHostImpl::GenerateChildProcessUniqueId()),
+      browser_context_(browser_context),
+      prev_routing_id_(0),
+      fast_shutdown_started_(false),
+      deletion_callback_called_(false) {
   // Child process security operations can't be unit tested unless we add
   // ourselves as an existing child process.
   ChildProcessSecurityPolicyImpl::GetInstance()->Add(GetID());
@@ -40,8 +40,14 @@ MockRenderProcessHost::~MockRenderProcessHost() {
   delete transport_dib_;
   if (factory_)
     factory_->Remove(this);
-  // In unit tests, Release() might not have been called.
-  RenderProcessHostImpl::UnregisterHost(GetID());
+
+  // In unit tests, Cleanup() might not have been called.
+  if (!deletion_callback_called_) {
+    FOR_EACH_OBSERVER(RenderProcessHostObserver,
+                      observers_,
+                      RenderProcessHostDestroyed(this));
+    RenderProcessHostImpl::UnregisterHost(GetID());
+  }
 }
 
 void MockRenderProcessHost::EnableSendQueue() {
@@ -67,6 +73,15 @@ void MockRenderProcessHost::RemoveRoute(int32 routing_id) {
   Cleanup();
 }
 
+void MockRenderProcessHost::AddObserver(RenderProcessHostObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void MockRenderProcessHost::RemoveObserver(
+    RenderProcessHostObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 bool MockRenderProcessHost::WaitForBackingStoreMsg(
     int render_widget_id,
     const base::TimeDelta& max_delay,
@@ -169,12 +184,16 @@ bool MockRenderProcessHost::IgnoreInputEvents() const {
 
 void MockRenderProcessHost::Cleanup() {
   if (listeners_.IsEmpty()) {
+    FOR_EACH_OBSERVER(RenderProcessHostObserver,
+                      observers_,
+                      RenderProcessHostDestroyed(this));
     NotificationService::current()->Notify(
         NOTIFICATION_RENDERER_PROCESS_TERMINATED,
         Source<RenderProcessHost>(this),
         NotificationService::NoDetails());
     base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
     RenderProcessHostImpl::UnregisterHost(GetID());
+    deletion_callback_called_ = true;
   }
 }
 
diff --git a/content/public/test/mock_render_process_host.h b/content/public/test/mock_render_process_host.h
index a11473a..3836ff5 100644
--- a/content/public/test/mock_render_process_host.h
+++ b/content/public/test/mock_render_process_host.h
@@ -7,6 +7,7 @@
 
 #include "base/basictypes.h"
 #include "base/memory/scoped_vector.h"
+#include "base/observer_list.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_process_host_factory.h"
 #include "ipc/ipc_test_sink.h"
@@ -38,6 +39,8 @@ class MockRenderProcessHost : public RenderProcessHost {
   virtual int GetNextRoutingID() OVERRIDE;
   virtual void AddRoute(int32 routing_id, IPC::Listener* listener) OVERRIDE;
   virtual void RemoveRoute(int32 routing_id) OVERRIDE;
+  virtual void AddObserver(RenderProcessHostObserver* observer) OVERRIDE;
+  virtual void RemoveObserver(RenderProcessHostObserver* observer) OVERRIDE;
   virtual bool WaitForBackingStoreMsg(int render_widget_id,
                                       const base::TimeDelta& max_delay,
                                       IPC::Message* msg) OVERRIDE;
@@ -96,11 +99,13 @@ class MockRenderProcessHost : public RenderProcessHost {
   const MockRenderProcessHostFactory* factory_;
   int id_;
   BrowserContext* browser_context_;
+  ObserverList<RenderProcessHostObserver> observers_;
 
   IDMap<RenderWidgetHost> render_widget_hosts_;
   int prev_routing_id_;
   IDMap<IPC::Listener> listeners_;
   bool fast_shutdown_started_;
+  bool deletion_callback_called_;
 
   DISALLOW_COPY_AND_ASSIGN(MockRenderProcessHost);
 };