Stop having renderers use both pids and a monotonically increasing "host_id". This allows ResourceDispatcher to be used by child processes other than renderers. I've done minor related cleanup on the way to make the code simpler.

Review URL: http://codereview.chromium.org/42054

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

6 files changed, 54 insertions, 42 deletions

diff --git a/chrome/browser/tab_contents/interstitial_page.cc b/chrome/browser/tab_contents/interstitial_page.cc
index 4d56420..a4e4f5b 100644
--- a/chrome/browser/tab_contents/interstitial_page.cc
+++ b/chrome/browser/tab_contents/interstitial_page.cc
@@ -110,7 +110,7 @@ InterstitialPage::InterstitialPage(WebContents* tab,
       enabled_(true),
       action_taken_(false),
       render_view_host_(NULL),
-      original_rvh_process_id_(tab->render_view_host()->process()->host_id()),
+      original_rvh_process_id_(tab->render_view_host()->process()->pid()),
       original_rvh_id_(tab->render_view_host()->routing_id()),
       should_revert_tab_title_(false),
       resource_dispatcher_host_notified_(false),
@@ -217,7 +217,7 @@ void InterstitialPage::Observe(NotificationType type,
         // The RenderViewHost is being destroyed (as part of the tab being
         // closed), make sure we clear the blocked requests.
         RenderViewHost* rvh = Source<RenderViewHost>(source).ptr();
-        DCHECK(rvh->process()->host_id() == original_rvh_process_id_ &&
+        DCHECK(rvh->process()->pid() == original_rvh_process_id_ &&
                rvh->routing_id() == original_rvh_id_);
         TakeActionOnResourceDispatcher(CANCEL);
       }
diff --git a/chrome/browser/tab_contents/render_view_host_manager.cc b/chrome/browser/tab_contents/render_view_host_manager.cc
index 162ecb9..715f2bf 100644
--- a/chrome/browser/tab_contents/render_view_host_manager.cc
+++ b/chrome/browser/tab_contents/render_view_host_manager.cc
@@ -148,8 +148,7 @@ bool RenderViewHostManager::ShouldCloseTabOnUnresponsiveRenderer() {
     // handler later finishes, this call will be ignored because the state in
     // CrossSiteResourceHandler will already be cleaned up.)
     current_host()->process()->CrossSiteClosePageACK(
-        pending_render_view_host_->site_instance()->process_host_id(),
-        pending_request_id);
+        pending_render_view_host_->process()->pid(), pending_request_id);
   }
   return false;
 }
diff --git a/chrome/browser/tab_contents/site_instance.cc b/chrome/browser/tab_contents/site_instance.cc
index 6582be6..05b3fbd 100644
--- a/chrome/browser/tab_contents/site_instance.cc
+++ b/chrome/browser/tab_contents/site_instance.cc
@@ -6,48 +6,58 @@
 
 #include "chrome/browser/renderer_host/browser_render_process_host.h"
 #include "chrome/common/url_constants.h"
+#include "chrome/common/notification_service.h"
 #include "net/base/registry_controlled_domain.h"
 
+SiteInstance::SiteInstance(BrowsingInstance* browsing_instance)
+    : browsing_instance_(browsing_instance),
+      render_process_host_factory_(NULL),
+      process_(NULL),
+      max_page_id_(-1),
+      has_site_(false) {
+  DCHECK(browsing_instance);
+
+  NotificationService::current()->AddObserver(this,
+      NotificationType::RENDERER_PROCESS_TERMINATED,
+      NotificationService::AllSources());
+}
+
 SiteInstance::~SiteInstance() {
   // 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.
   if (has_site_)
     browsing_instance_->UnregisterSiteInstance(this);
+
+  NotificationService::current()->RemoveObserver(this,
+      NotificationType::RENDERER_PROCESS_TERMINATED,
+      NotificationService::AllSources());
 }
 
 RenderProcessHost* SiteInstance::GetProcess() {
-  RenderProcessHost* process = NULL;
-  if (process_host_id_ != -1)
-    process = RenderProcessHost::FromID(process_host_id_);
-
   // Create a new process if ours went away or was reused.
-  if (!process) {
+  if (!process_) {
     // See if we should reuse an old process
     if (RenderProcessHost::ShouldTryToUseExistingProcessHost())
-      process = RenderProcessHost::GetExistingProcessHost(
+      process_ = RenderProcessHost::GetExistingProcessHost(
           browsing_instance_->profile());
 
     // Otherwise (or if that fails), create a new one.
-    if (!process) {
+    if (!process_) {
       if (render_process_host_factory_) {
-        process = render_process_host_factory_->CreateRenderProcessHost(
+        process_ = render_process_host_factory_->CreateRenderProcessHost(
             browsing_instance_->profile());
       } else {
-        process = new BrowserRenderProcessHost(browsing_instance_->profile());
+        process_ = new BrowserRenderProcessHost(browsing_instance_->profile());
       }
     }
 
-    // Update our host ID, so all pages in this SiteInstance will use
-    // the correct process.
-    process_host_id_ = process->host_id();
-
     // Make sure the process starts at the right max_page_id
-    process->UpdateMaxPageID(max_page_id_);
+    process_->UpdateMaxPageID(max_page_id_);
   }
-  DCHECK(process);
+  DCHECK(process_);
 
-  return process;
+  return process_;
 }
 
 void SiteInstance::SetSite(const GURL& url) {
@@ -151,3 +161,12 @@ bool SiteInstance::IsSameWebSite(const GURL& url1, const GURL& url2) {
 
   return net::RegistryControlledDomainService::SameDomainOrHost(url1, url2);
 }
+
+void SiteInstance::Observe(NotificationType type,
+                           const NotificationSource& source,
+                           const NotificationDetails& details) {
+  DCHECK(type == NotificationType::RENDERER_PROCESS_TERMINATED);
+  RenderProcessHost* rph = Source<RenderProcessHost>(source).ptr();
+  if (rph == process_)
+    process_ = NULL;
+}
diff --git a/chrome/browser/tab_contents/site_instance.h b/chrome/browser/tab_contents/site_instance.h
index 6e25a2b..34cef93 100644
--- a/chrome/browser/tab_contents/site_instance.h
+++ b/chrome/browser/tab_contents/site_instance.h
@@ -7,6 +7,7 @@
 
 #include "chrome/browser/browsing_instance.h"
 #include "chrome/browser/renderer_host/render_process_host.h"
+#include "chrome/common/notification_observer.h"
 #include "googleurl/src/gurl.h"
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -43,7 +44,8 @@
 // tabs with no NavigationEntries or in NavigationEntries in the history.
 //
 ///////////////////////////////////////////////////////////////////////////////
-class SiteInstance : public base::RefCounted<SiteInstance> {
+class SiteInstance : public base::RefCounted<SiteInstance>,
+                     public NotificationObserver {
  public:
   // Virtual to allow tests to extend it.
   virtual ~SiteInstance();
@@ -61,12 +63,6 @@ class SiteInstance : public base::RefCounted<SiteInstance> {
     render_process_host_factory_ = rph_factory;
   }
 
-  // Set / Get the host ID for this SiteInstance's current RenderProcessHost.
-  void set_process_host_id(int process_host_id) {
-    process_host_id_ = process_host_id;
-  }
-  int process_host_id() const { return process_host_id_; }
-
   // Update / Get the max page ID for this SiteInstance.
   void UpdateMaxPageID(int32 page_id) {
     if (page_id > max_page_id_)
@@ -133,16 +129,14 @@ class SiteInstance : public base::RefCounted<SiteInstance> {
   // Create a new SiteInstance.  Protected to give access to BrowsingInstance
   // and tests; most callers should use CreateSiteInstance or
   // GetRelatedSiteInstance instead.
-  SiteInstance(BrowsingInstance* browsing_instance)
-      : browsing_instance_(browsing_instance),
-        render_process_host_factory_(NULL),
-        process_host_id_(-1),
-        max_page_id_(-1),
-        has_site_(false) {
-    DCHECK(browsing_instance);
-  }
+  SiteInstance(BrowsingInstance* browsing_instance);
 
  private:
+  // NotificationObserver implementation.
+  void Observe(NotificationType type,
+               const NotificationSource& source,
+               const NotificationDetails& details);
+
   // BrowsingInstance to which this SiteInstance belongs.
   scoped_refptr<BrowsingInstance> browsing_instance_;
 
@@ -150,11 +144,8 @@ class SiteInstance : public base::RefCounted<SiteInstance> {
   // that the default BrowserRenderProcessHost should be created.
   const RenderProcessHostFactory* render_process_host_factory_;
 
-  // Current host ID for the RenderProcessHost that is rendering pages for this
-  // SiteInstance.  If the rendering process dies, this host ID can be
-  // replaced when a new process is created, without losing the association
-  // between all pages in this SiteInstance.
-  int process_host_id_;
+  // Current RenderProcessHost that is rendering pages for this SiteInstance.
+  RenderProcessHost* process_;
 
   // The current max_page_id in the SiteInstance's RenderProcessHost.  If the
   // rendering process dies, its replacement should start issuing page IDs that
diff --git a/chrome/browser/tab_contents/web_contents.cc b/chrome/browser/tab_contents/web_contents.cc
index 8f1a5ea..5b935b5 100644
--- a/chrome/browser/tab_contents/web_contents.cc
+++ b/chrome/browser/tab_contents/web_contents.cc
@@ -438,7 +438,10 @@ void WebContents::DidBecomeSelected() {
   if (render_widget_host_view())
     render_widget_host_view()->DidBecomeSelected();
 
-  CacheManagerHost::GetInstance()->ObserveActivity(process()->host_id());
+  // If pid() is -1, that means the RenderProcessHost still hasn't been
+  // initialized.  It'll register with CacheManagerHost when it is.
+  if (process()->pid() != -1)
+    CacheManagerHost::GetInstance()->ObserveActivity(process()->pid());
 }
 
 void WebContents::WasHidden() {
diff --git a/chrome/browser/tab_contents/web_contents_view_win.cc b/chrome/browser/tab_contents/web_contents_view_win.cc
index 51a9ffb..c1842cd 100644
--- a/chrome/browser/tab_contents/web_contents_view_win.cc
+++ b/chrome/browser/tab_contents/web_contents_view_win.cc
@@ -232,7 +232,7 @@ void WebContentsViewWin::OpenDeveloperTools() {
   if (!host)
     return;
 
-  dev_tools_window_->Show(host->process()->host_id(), host->routing_id());
+  dev_tools_window_->Show(host->process()->pid(), host->routing_id());
 }
 
 void WebContentsViewWin::ForwardMessageToDevToolsClient(