Enable spare RenderProcessHost to be preinitialized.

content/browser/renderer_host/render_process_host_impl.cc

Index: content/browser/renderer_host/render_process_host_impl.cc
diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
index 89b1560761207ba8f6815ecad7ae689790343172..af150e6e2f1b871277a106f29ba3169f74d63bc4 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -465,6 +465,84 @@ class SessionStorageHolder : public base::SupportsUserData::Data {
 const void* const kDefaultSubframeProcessHostHolderKey =
     &kDefaultSubframeProcessHostHolderKey;
 
+// This class manages spare renderers.
+class SpareRenderProcessHostManager {
+ public:
+  SpareRenderProcessHostManager() {}
+
+  void WarmupSpareRenderProcessHost(BrowserContext* browser_context) {
+    StoragePartitionImpl* current_partition =
+        static_cast<StoragePartitionImpl*>(
+            BrowserContext::GetStoragePartition(browser_context, nullptr));
+
+    if (spare_render_process_host_ &&
+        matching_browser_context_ == browser_context &&
+        matching_storage_partition_ == current_partition) {
+      return;  // Nothing to warm up.
+    }
+
+    CleanupSpareRenderProcessHost();
+
+    matching_browser_context_ = browser_context;
+    matching_storage_partition_ = current_partition;
+    spare_render_process_host_ = RenderProcessHostImpl::CreateRenderProcessHost(
+        browser_context, current_partition, false /* is_for_guests_only */);
+
+    spare_render_process_host_->Init();
+  }
+
+  RenderProcessHost* MaybeTakeSpareRenderProcessHost(
+      const BrowserContext* browser_context,
+      const StoragePartition* partition,
+      bool is_for_guests_only) {
+    if (!spare_render_process_host_ ||
+        browser_context != matching_browser_context_ ||
+        partition != matching_storage_partition_ || is_for_guests_only) {
+      // As a new RenderProcessHost will almost certainly be created, we cleanup
+      // the non-matching one so as not to waste resources.
+      CleanupSpareRenderProcessHost();
+      return nullptr;
+    }
+
+    if (!spare_render_process_host_->HasConnection()) {

[falken, 2017/06/15 06:46:49]

I'm curious why this is needed. (I'm not really a render process host expert.)
Does this signify the RPH crashed?

Could the RPH just not be ready yet? Do we also want to check something like
FastShutdownStarted()?

[mattcary, 2017/06/15 08:30:39]

I'm not sure it is needed.

The case I'm worried about is if the spare renderer process is killed the warmup
and this function. Any advice on the best way to handle that would be great, as
I'm not personally familiar with the best way to hook into the RPH lifecycle (in
particular, it seems that the RPHObserver isn't sufficient, although it may be
necessary).

 I'll look into FastShutdownStarted.

+      CleanupSpareRenderProcessHost();
+      return nullptr;
+    }
+
+    RenderProcessHost* rph = spare_render_process_host_;
+    spare_render_process_host_ = nullptr;
+    return rph;
+  }
+
+  void OnStorageParitionShutdown(StoragePartition* partition) {

[falken, 2017/06/15 06:46:49]

typo: Partition

[mattcary, 2017/06/15 08:30:39]

Done.

+    if (partition == matching_storage_partition_) {
+      CleanupSpareRenderProcessHost();
+    }
+  }
+
+ private:
+  void CleanupSpareRenderProcessHost() {
+    if (spare_render_process_host_) {
+      spare_render_process_host_->Cleanup();
+      spare_render_process_host_ = nullptr;
+    }
+  }
+
+  // This is a bare pointer, because RenderProcessHost manages the lifetime of
+  // all its instances; see g_all_hosts, above.
+  RenderProcessHost* spare_render_process_host_;
+
+  // Used only to check if a creation request matches the
+  // spare, and not accessed.
+  const BrowserContext* matching_browser_context_ = nullptr;
+  const StoragePartition* matching_storage_partition_ = nullptr;
+
+  DISALLOW_COPY_AND_ASSIGN(SpareRenderProcessHostManager);
+};
+
+base::LazyInstance<SpareRenderProcessHostManager>::Leaky
+    g_spare_render_process_host_manager = LAZY_INSTANCE_INITIALIZER;
+
 class DefaultSubframeProcessHostHolder : public base::SupportsUserData::Data,
                                          public RenderProcessHostObserver {
  public:
@@ -483,7 +561,7 @@ class DefaultSubframeProcessHostHolder : public base::SupportsUserData::Data,
     // Is this the default storage partition? If it isn't, then just give it its
     // own non-shared process.
     if (partition != default_partition || is_for_guests_only) {
-      RenderProcessHostImpl* host = new RenderProcessHostImpl(
+      RenderProcessHost* host = RenderProcessHostImpl::CreateRenderProcessHost(
           browser_context_, static_cast<StoragePartitionImpl*>(partition),
           is_for_guests_only);
       host->SetIsNeverSuitableForReuse();
@@ -495,7 +573,12 @@ class DefaultSubframeProcessHostHolder : public base::SupportsUserData::Data,
     if (host_)
       return host_;
 
-    host_ = new RenderProcessHostImpl(
+    host_ =
+        g_spare_render_process_host_manager.Get()
+            .MaybeTakeSpareRenderProcessHost(browser_context_, partition,
+                                             false /* is for guests only */);
+
+    host_ = RenderProcessHostImpl::CreateRenderProcessHost(
         browser_context_, static_cast<StoragePartitionImpl*>(partition),
         false /* for guests only */);
     host_->SetIsNeverSuitableForReuse();
@@ -516,7 +599,7 @@ class DefaultSubframeProcessHostHolder : public base::SupportsUserData::Data,
 
   // The default subframe render process used for the default storage partition
   // of this BrowserContext.
-  RenderProcessHostImpl* host_ = nullptr;
+  RenderProcessHost* host_ = nullptr;
 };
 
 void CreateMemoryCoordinatorHandle(
@@ -907,6 +990,20 @@ void RenderProcessHost::SetMaxRendererProcessCount(size_t count) {
   g_max_renderer_count_override = count;
 }
 
+// static
+RenderProcessHost* RenderProcessHostImpl::CreateRenderProcessHost(
+    BrowserContext* browser_context,
+    StoragePartitionImpl* storage_partition_impl,
+    bool is_for_guests_only) {
+  if (g_render_process_host_factory_) {
+    return g_render_process_host_factory_->CreateRenderProcessHost(
+        browser_context);
+  }
+
+  return new RenderProcessHostImpl(browser_context, storage_partition_impl,
+                                   is_for_guests_only);
+}
+
 RenderProcessHostImpl::RenderProcessHostImpl(
     BrowserContext* browser_context,
     StoragePartitionImpl* storage_partition_impl,
@@ -1978,6 +2075,13 @@ void RenderProcessHostImpl::RemoveExpectedNavigationToSite(
   tracker->DecrementSiteProcessCount(site_url, render_process_host->GetID());
 }
 
+// static
+void RenderProcessHostImpl::OnStorageParitionShutdown(

[falken, 2017/06/15 06:46:49]

typo: Partition

[mattcary, 2017/06/15 08:30:39]

Done (and in all the other instances too).

+    StoragePartition* partition) {
+  g_spare_render_process_host_manager.Get().OnStorageParitionShutdown(
+      partition);
+}
+
 bool RenderProcessHostImpl::IsForGuestsOnly() const {
   return is_for_guests_only_;
 }
@@ -2917,6 +3021,13 @@ bool RenderProcessHostImpl::IsSuitableHost(RenderProcessHost* host,
   return GetContentClient()->browser()->IsSuitableHost(host, site_url);
 }
 
+// static
+void RenderProcessHost::WarmupSpareRenderProcessHost(
+    content::BrowserContext* browser_context) {
+  g_spare_render_process_host_manager.Get().WarmupSpareRenderProcessHost(
+      browser_context);
+}
+
 // static
 bool RenderProcessHost::run_renderer_in_process() {
   return g_run_renderer_in_process_;
@@ -3113,18 +3224,20 @@ RenderProcessHost* RenderProcessHostImpl::GetProcessHostForSiteInstance(
     render_process_host = GetExistingProcessHost(browser_context, site_url);
   }
 
+  // Try to use a spare process if one is found.
+  StoragePartitionImpl* partition = static_cast<StoragePartitionImpl*>(
+      BrowserContext::GetStoragePartition(browser_context, site_instance));

[falken, 2017/06/15 06:46:49]

really naive question: I wonder if we have to somehow stop the render process
from being discovered and used by some other thing (so it would no longer be a
spare, unused render process). E,g, anyone could probably discover it via
AllHostsIterator()?

I guess everything that "claims" a render process comes through
GetProcessHostForSiteInstance().

[mattcary, 2017/06/15 08:30:39]

That's a good point. I just searched through the code, all uses of
AllHostsIterator appear to be for updating hosts when events come though. So
that seems to be okay (although I guess this might expose some bugs where the
renderer is expected to be fully initialized with a web contents and such).
Yes, that was what we understood.

+  if (!render_process_host) {
+    render_process_host =
+        g_spare_render_process_host_manager.Get()
+            .MaybeTakeSpareRenderProcessHost(browser_context, partition,
+                                             is_for_guests_only);
+  }
+
   // Otherwise (or if that fails), create a new one.
   if (!render_process_host) {
-    if (g_render_process_host_factory_) {
-      render_process_host =
-          g_render_process_host_factory_->CreateRenderProcessHost(
-              browser_context);
-    } else {
-      StoragePartitionImpl* partition = static_cast<StoragePartitionImpl*>(
-          BrowserContext::GetStoragePartition(browser_context, site_instance));
-      render_process_host = new RenderProcessHostImpl(
-          browser_context, partition, is_for_guests_only);
-    }
+    render_process_host =
+        CreateRenderProcessHost(browser_context, partition, is_for_guests_only);
   }
   return render_process_host;
 }