Add UMA for recording what happens when attempting to take a spare RPH.

The new UMA should help evaluate how often the spare is missing (e.g.
because of low process limit on low-memory devices), mismatched (e.g.
when simultaneously navigating in 2 profiles) or taken.  The spare can
only help improve performance of cross-site navigations if it is
actually taken.

Bug: 808114
Change-Id: Ic37cacf014de73f9adefc0407e60b4cfa246028b
Reviewed-on: https://chromium-review.googlesource.com/1012510
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551026}

content/browser/renderer_host/render_process_host_impl.cc

@@ -601,6 +601,25 @@ class SpareRenderProcessHostManager : public RenderProcessHostObserver {
     StoragePartition* site_storage =
         BrowserContext::GetStoragePartition(browser_context, site_instance);
 
+    // Log UMA metrics.
+    using SpareProcessMaybeTakeAction =
+        RenderProcessHostImpl::SpareProcessMaybeTakeAction;
+    SpareProcessMaybeTakeAction action =
+        SpareProcessMaybeTakeAction::kNoSparePresent;
+    if (!spare_render_process_host_)
+      action = SpareProcessMaybeTakeAction::kNoSparePresent;
+    else if (browser_context != spare_render_process_host_->GetBrowserContext())
+      action = SpareProcessMaybeTakeAction::kMismatchedBrowserContext;
+    else if (site_storage != spare_render_process_host_->GetStoragePartition())
+      action = SpareProcessMaybeTakeAction::kMismatchedStoragePartition;
+    else if (!should_use_spare)
+      action = SpareProcessMaybeTakeAction::kRefusedByEmbedder;
+    else
+      action = SpareProcessMaybeTakeAction::kSpareTaken;
+    UMA_HISTOGRAM_ENUMERATION(
+        "BrowserRenderProcessHost.SpareProcessMaybeTakeAction", action);
+
+    // Decide whether to take or drop the spare process.
     RenderProcessHost* returned_process = nullptr;
     if (spare_render_process_host_ &&
         browser_context == spare_render_process_host_->GetBrowserContext() &&
@@ -612,6 +631,7 @@ class SpareRenderProcessHostManager : public RenderProcessHostObserver {
       // discard the spare RPH, so if one exists, it should always be live here.
       CHECK(spare_render_process_host_->HasConnection());
 
+      DCHECK_EQ(SpareProcessMaybeTakeAction::kSpareTaken, action);
       returned_process = spare_render_process_host_;
       ReleaseSpareRenderProcessHost(spare_render_process_host_);
     } else if (!RenderProcessHostImpl::IsSpareProcessKeptAtAllTimes()) {

content/browser/renderer_host/render_process_host_impl.h

@@ -303,6 +303,17 @@ class CONTENT_EXPORT RenderProcessHostImpl
   static void NotifySpareManagerAboutRecentlyUsedBrowserContext(
       BrowserContext* browser_context);
 
+  // This enum backs a histogram, so do not change the order of entries or
+  // remove entries and update enums.xml if adding new entries.
+  enum class SpareProcessMaybeTakeAction {
+    kNoSparePresent = 0,
+    kMismatchedBrowserContext = 1,
+    kMismatchedStoragePartition = 2,
+    kRefusedByEmbedder = 3,
+    kSpareTaken = 4,
+    kMaxValue = kSpareTaken
+  };
+
   static base::MessageLoop* GetInProcessRendererThreadForTesting();
 
   // This forces a renderer that is running "in process" to shut down.

content/browser/renderer_host/render_process_host_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
+#include "base/test/histogram_tester.h"
 #include "build/build_config.h"
 #include "content/common/frame_messages.h"
 #include "content/common/frame_owner_properties.h"
@@ -26,6 +27,7 @@
 #include "content/test/test_render_frame_host.h"
 #include "content/test/test_render_view_host.h"
 #include "content/test/test_web_contents.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "third_party/blink/public/common/frame/frame_policy.h"
 
 namespace content {
@@ -907,6 +909,17 @@ class SpareRenderProcessHostUnitTest : public RenderViewHostImplTestHarness {
   DISALLOW_COPY_AND_ASSIGN(SpareRenderProcessHostUnitTest);
 };
 
+using SpareProcessMaybeTakeAction =
+    RenderProcessHostImpl::SpareProcessMaybeTakeAction;
+void ExpectSpareProcessMaybeTakeActionBucket(
+    const base::HistogramTester& histograms,
+    SpareProcessMaybeTakeAction expected_action) {
+  EXPECT_THAT(
+      histograms.GetAllSamples(
+          "BrowserRenderProcessHost.SpareProcessMaybeTakeAction"),
+      testing::ElementsAre(base::Bucket(static_cast<int>(expected_action), 1)));
+}
+
 TEST_F(SpareRenderProcessHostUnitTest, TestRendererTaken) {
   RenderProcessHost::WarmupSpareRenderProcessHost(browser_context());
   ASSERT_EQ(1U, rph_factory_.GetProcesses()->size());
@@ -915,9 +928,12 @@ TEST_F(SpareRenderProcessHostUnitTest, TestRendererTaken) {
   EXPECT_EQ(spare_rph, rph_factory_.GetProcesses()->at(0).get());
 
   const GURL kUrl1("http://foo.com");
+  base::HistogramTester histograms;
   SetContents(CreateTestWebContents());
   NavigateAndCommit(kUrl1);
   EXPECT_EQ(spare_rph, main_test_rfh()->GetProcess());
+  ExpectSpareProcessMaybeTakeActionBucket(
+      histograms, SpareProcessMaybeTakeAction::kSpareTaken);
 
   EXPECT_NE(spare_rph,
             RenderProcessHostImpl::GetSpareRenderProcessHostForTesting());
@@ -942,9 +958,12 @@ TEST_F(SpareRenderProcessHostUnitTest, TestRendererNotTaken) {
   EXPECT_EQ(old_spare, rph_factory_.GetProcesses()->at(0).get());
 
   const GURL kUrl1("http://foo.com");
+  base::HistogramTester histograms;
   SetContents(CreateTestWebContents());
   NavigateAndCommit(kUrl1);
   EXPECT_NE(old_spare, main_test_rfh()->GetProcess());
+  ExpectSpareProcessMaybeTakeActionBucket(
+      histograms, SpareProcessMaybeTakeAction::kMismatchedBrowserContext);
 
   // Pumping the message loop here accounts for the delay between calling
   // RPH::Cleanup on the spare and the time when the posted delete actually
@@ -965,6 +984,31 @@ TEST_F(SpareRenderProcessHostUnitTest, TestRendererNotTaken) {
   }
 }
 
+TEST_F(SpareRenderProcessHostUnitTest, SpareMissing) {
+  RenderProcessHostImpl::DiscardSpareRenderProcessHostForTesting();
+  ASSERT_EQ(0U, rph_factory_.GetProcesses()->size());
+  RenderProcessHost* spare_rph =
+      RenderProcessHostImpl::GetSpareRenderProcessHostForTesting();
+  EXPECT_FALSE(spare_rph);
+
+  const GURL kUrl1("http://foo.com");
+  base::HistogramTester histograms;
+  SetContents(CreateTestWebContents());
+  NavigateAndCommit(kUrl1);
+  EXPECT_TRUE(main_test_rfh()->GetProcess());
+  ExpectSpareProcessMaybeTakeActionBucket(
+      histograms, SpareProcessMaybeTakeAction::kNoSparePresent);
+
+  spare_rph = RenderProcessHostImpl::GetSpareRenderProcessHostForTesting();
+  if (RenderProcessHostImpl::IsSpareProcessKeptAtAllTimes()) {
+    EXPECT_TRUE(spare_rph);
+    EXPECT_EQ(2U, rph_factory_.GetProcesses()->size());
+  } else {
+    EXPECT_FALSE(spare_rph);
+    EXPECT_EQ(1U, rph_factory_.GetProcesses()->size());
+  }
+}
+
 TEST_F(SpareRenderProcessHostUnitTest,
        SpareShouldNotLaunchInParallelWithOtherProcess) {
   std::unique_ptr<BrowserContext> alternate_context(new TestBrowserContext());

tools/metrics/histograms/enums.xml

@@ -41501,6 +41501,14 @@ Called by update_net_trust_anchors.py.-->
   <int value="3" label="User-initiated with logs enabled"/>
 </enum>
 
+<enum name="SpareProcessMaybeTakeAction">
+  <int value="0" label="NoSparePresent"/>
+  <int value="1" label="MismatchedBrowserContext"/>
+  <int value="2" label="MismatchedStoragePartition"/>
+  <int value="3" label="RefusedByEmbedder"/>
+  <int value="4" label="SpareTaken"/>
+</enum>
+
 <enum name="SpareWebContentsStatus">
   <int value="0" label="Created"/>
   <int value="1" label="Used"/>

tools/metrics/histograms/histograms.xml

@@ -9285,6 +9285,16 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="BrowserRenderProcessHost.SpareProcessMaybeTakeAction"
+    enum="SpareProcessMaybeTakeAction">
+  <owner>alexmos@chromium.org</owner>
+  <owner>lukasza@chromium.org</owner>
+  <summary>
+    Records what happens when an attempt is made to use a spare
+    RenderProcessHost - logs that either the attempt succeeded or why it failed.
+  </summary>
+</histogram>
+
 <histogram name="BrowserWindow.Resize.Duration" units="ms">
   <owner>sadrul@chromium.org</owner>
   <summary>