Break Navigation.TimeToReadyToCommit into main frame vs subframe.

The goal of this CL is to understand whether TimeToReadyToCommit
regressions with --site-per-process are affected by whether the
navigation is for a main frame or subframe.

Bug: 808114
Change-Id: Ib55b51f2b777defc508443788cc036866e638075
Reviewed-on: https://chromium-review.googlesource.com/1058606
Commit-Queue: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558837}

content/browser/frame_host/navigation_handle_impl.cc

@@ -835,6 +835,17 @@ void NavigationHandleImpl::ReadyToCommitNavigation(
     base::TimeDelta delta = ready_to_commit_time_ - navigation_start_;
     LOG_NAVIGATION_TIMING_HISTOGRAM("TimeToReadyToCommit", transition_, delta,
                                     base::TimeDelta::FromSeconds(10));
+
+    if (IsInMainFrame()) {
+      LOG_NAVIGATION_TIMING_HISTOGRAM("TimeToReadyToCommit.MainFrame",
+                                      transition_, delta,
+                                      base::TimeDelta::FromSeconds(10));
+    } else {
+      LOG_NAVIGATION_TIMING_HISTOGRAM("TimeToReadyToCommit.Subframe",
+                                      transition_, delta,
+                                      base::TimeDelta::FromSeconds(10));
+    }
+
     if (is_same_process_) {
       LOG_NAVIGATION_TIMING_HISTOGRAM("TimeToReadyToCommit.SameProcess",
                                       transition_, delta,

content/browser/frame_host/navigation_handle_impl_browsertest.cc

@@ -2197,8 +2197,8 @@ IN_PROC_BROWSER_TEST_F(NavigationHandleImplBrowserTest, StartToCommitMetrics) {
   }
 }
 
-// Verify that the SameProcess vs CrossProcess version of the
-// TimeToReadyToCommit metric is correctly logged.
+// Verify that the TimeToReadyToCommit metrics are correctly logged for
+// SameProcess vs CrossProcess as well as MainFrame vs Subframe cases.
 IN_PROC_BROWSER_TEST_F(NavigationHandleImplBrowserTest,
                        TimeToReadyToCommitMetrics) {
   EXPECT_TRUE(
@@ -2211,6 +2211,8 @@ IN_PROC_BROWSER_TEST_F(NavigationHandleImplBrowserTest,
     EXPECT_TRUE(NavigateToURL(shell(), url));
 
     base::HistogramTester::CountsMap expected_counts = {
+        {"Navigation.TimeToReadyToCommit.MainFrame", 1},
+        {"Navigation.TimeToReadyToCommit.MainFrame.NewNavigation", 1},
         {"Navigation.TimeToReadyToCommit.NewNavigation", 1},
         {"Navigation.TimeToReadyToCommit.SameProcess", 1},
         {"Navigation.TimeToReadyToCommit.SameProcess.NewNavigation", 1}};
@@ -2226,6 +2228,8 @@ IN_PROC_BROWSER_TEST_F(NavigationHandleImplBrowserTest,
     EXPECT_TRUE(NavigateToURL(shell(), url));
 
     base::HistogramTester::CountsMap expected_counts = {
+        {"Navigation.TimeToReadyToCommit.MainFrame", 1},
+        {"Navigation.TimeToReadyToCommit.MainFrame.NewNavigation", 1},
         {"Navigation.TimeToReadyToCommit.NewNavigation", 1},
         {"Navigation.TimeToReadyToCommit.CrossProcess", 1},
         {"Navigation.TimeToReadyToCommit.CrossProcess.NewNavigation", 1}};
@@ -2233,6 +2237,36 @@ IN_PROC_BROWSER_TEST_F(NavigationHandleImplBrowserTest,
         histograms.GetTotalCountsForPrefix("Navigation.TimeToReadyToCommit."),
         testing::ContainerEq(expected_counts));
   }
+
+  // Add a new subframe.
+  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
+                            ->GetFrameTree()
+                            ->root();
+  EXPECT_TRUE(ExecuteScript(
+      root, "document.body.appendChild(document.createElement('iframe'));"));
+
+  // Navigate subframe cross-site and ensure Subframe metrics are logged.
+  {
+    base::HistogramTester histograms;
+    GURL url(embedded_test_server()->GetURL("b.com", "/title3.html"));
+    NavigateFrameToURL(root->child_at(0), url);
+
+    std::string navigation_type =
+        AreAllSitesIsolatedForTesting() ? "CrossProcess" : "SameProcess";
+    base::HistogramTester::CountsMap expected_counts = {
+        {"Navigation.TimeToReadyToCommit.Subframe", 1},
+        {"Navigation.TimeToReadyToCommit.Subframe.NewNavigation", 1},
+        {"Navigation.TimeToReadyToCommit.NewNavigation", 1},
+        {base::StringPrintf("Navigation.TimeToReadyToCommit.%s",
+                            navigation_type.c_str()),
+         1},
+        {base::StringPrintf("Navigation.TimeToReadyToCommit.%s.NewNavigation",
+                            navigation_type.c_str()),
+         1}};
+    EXPECT_THAT(
+        histograms.GetTotalCountsForPrefix("Navigation.TimeToReadyToCommit."),
+        testing::ContainerEq(expected_counts));
+  }
 }
 
 IN_PROC_BROWSER_TEST_F(NavigationHandleImplDownloadBrowserTest, IsDownload) {

tools/metrics/histograms/histograms.xml

@@ -112754,6 +112754,7 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   <affected-histogram name="Navigation.StartToCommit"/>
   <affected-histogram name="Navigation.StartToCommit.CrossProcess"/>
   <affected-histogram name="Navigation.StartToCommit.SameProcess"/>
+  <affected-histogram name="Navigation.TimeToReadyToCommit"/>
 </histogram_suffixes>
 
 <histogram_suffixes name="NavigationPreloadEnabled" separator="_">
@@ -112841,7 +112842,9 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   <affected-histogram name="Navigation.StartToCommit.Subframe"/>
   <affected-histogram name="Navigation.TimeToReadyToCommit"/>
   <affected-histogram name="Navigation.TimeToReadyToCommit.CrossProcess"/>
+  <affected-histogram name="Navigation.TimeToReadyToCommit.MainFrame"/>
   <affected-histogram name="Navigation.TimeToReadyToCommit.SameProcess"/>
+  <affected-histogram name="Navigation.TimeToReadyToCommit.Subframe"/>
 </histogram_suffixes>
 
 <histogram_suffixes name="Net.BidirectionalStreamExperiment" separator=".">