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 fc876c671daea63b084e640a2cb2fdc474ce576d..0d65213325eff1b08107937422e5ad69fa913053 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -465,6 +465,67 @@ class SessionStorageHolder : public base::SupportsUserData::Data {
 const void* const kDefaultSubframeProcessHostHolderKey =
     &kDefaultSubframeProcessHostHolderKey;
 
+// This class manages spare renderers.
+class SpareRenderProcessHostManager {
+ public:
+  SpareRenderProcessHostManager() {}
+
+  void StartSpareRenderProcessHost(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.
+    }
+
+    matching_browser_context_ = browser_context;
+    matching_storage_partition_ = current_partition;
+    spare_render_process_host_ = RenderProcessHostImpl::CreateRenderProcessHost(

[Benoit L, 2017/06/09 14:35:22]

Wouldn't this leak the previous one in the case where spare_render_process_host_
was not null above?

Actually, shouldn't it be a unique_ptr?

[mattcary, 2017/06/09 14:40:53]

I don't think so. RPH seem to handle their own deletion; see the destructor:
https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_pro...

There appears to be a global host list. Probably it can't be deleted arbitrarily
as it needs to wait for cleanup IPCs from the renderer process?

And indeed, in browsertests that assertion does fire (browsertests are
complicated by other reasons and not included here).

There may be something I'm not understanding, though, so any insight you have
would be appreciated...

+        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 delete
+      // the non-matching one so as not to waste resources.
+      // TODO(mattcary): is deleting the RPH enough?
+      CleanupSpareRenderProcessHost();
+      return nullptr;
+    }
+    RenderProcessHost* rph = spare_render_process_host_;
+    spare_render_process_host_ = nullptr;
+    return rph;
+  }
+
+  void CleanupSpareRenderProcessHost() {
+    if (spare_render_process_host_) {
+      spare_render_process_host_->Cleanup();
+      spare_render_process_host_ = nullptr;
+    }
+  }
+
+ private:
+  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);
+};
+
+SpareRenderProcessHostManager* g_spare_render_process_host_manager = nullptr;
+
 class DefaultSubframeProcessHostHolder : public base::SupportsUserData::Data,
                                          public RenderProcessHostObserver {
  public:
@@ -483,7 +544,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 +556,13 @@ class DefaultSubframeProcessHostHolder : public base::SupportsUserData::Data,
     if (host_)
       return host_;
 
-    host_ = new RenderProcessHostImpl(
+    if (g_spare_render_process_host_manager) {
+      host_ =
+          g_spare_render_process_host_manager->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 +583,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 +974,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,
@@ -2914,6 +2995,16 @@ bool RenderProcessHostImpl::IsSuitableHost(RenderProcessHost* host,
   return GetContentClient()->browser()->IsSuitableHost(host, site_url);
 }
 
+// static
+void RenderProcessHost::StartSpareRenderProcessHost(
+    content::BrowserContext* browser_context) {
+  if (!g_spare_render_process_host_manager) {

[Benoit L, 2017/06/09 14:35:22]

since this is a leaky singleton, what about a LazyInstance?
We don't need it to be threadsafe though...

[mattcary, 2017/06/09 14:40:53]

Done

+    g_spare_render_process_host_manager = new SpareRenderProcessHostManager();
+  }
+  g_spare_render_process_host_manager->StartSpareRenderProcessHost(
+      browser_context);
+}
+
 // static
 bool RenderProcessHost::run_renderer_in_process() {
   return g_run_renderer_in_process_;
@@ -3110,18 +3201,19 @@ 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));
+  if (!render_process_host && g_spare_render_process_host_manager) {
+    render_process_host =
+        g_spare_render_process_host_manager->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, site_instance);
-    } 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);

[Benoit L, 2017/06/09 14:35:22]

Doesn't this bypass the factory?

[mattcary, 2017/06/09 14:40:53]

Nope, that's exactly why I added CreateRenderProcessHost.

   }
   return render_process_host;
 }