Add start/finish navigation to startup metrics.

chrome/browser/metrics/first_web_contents_profiler.cc

Index: chrome/browser/metrics/first_web_contents_profiler.cc
diff --git a/chrome/browser/metrics/first_web_contents_profiler.cc b/chrome/browser/metrics/first_web_contents_profiler.cc
index a95ff37d65df883ea517cd442d3dd8f612f1e33f..c6c25d14eb306a5b1efa24fcd2ce6c3f5ab76f26 100644
--- a/chrome/browser/metrics/first_web_contents_profiler.cc
+++ b/chrome/browser/metrics/first_web_contents_profiler.cc
@@ -22,6 +22,7 @@
 #include "components/startup_metric_utils/startup_metric_utils.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_details.h"
+#include "content/public/browser/navigation_handle.h"
 
 namespace {
 
@@ -116,6 +117,9 @@ FirstWebContentsProfiler::FirstWebContentsProfiler(
       initial_entry_committed_(false),
       collected_paint_metric_(false),
       collected_load_metric_(false),
+      collected_main_navigation_start_metric_(false),
+      collected_main_navigation_finished_metric_(false),
+      collected_main_navigation_finished_all_metric_(false),
       responsiveness_histogram_(NULL),
       responsiveness_1sec_histogram_(NULL),
       responsiveness_10sec_histogram_(NULL),
@@ -125,6 +129,9 @@ FirstWebContentsProfiler::FirstWebContentsProfiler(
   InitHistograms();
 }
 
+FirstWebContentsProfiler::~FirstWebContentsProfiler() {
+}
+
 void FirstWebContentsProfiler::DidFirstVisuallyNonEmptyPaint() {
   if (collected_paint_metric_)
     return;
@@ -178,6 +185,62 @@ void FirstWebContentsProfiler::DocumentOnLoadCompletedInMainFrame() {
     FinishedCollectingMetrics(FinishReason::DONE);
 }
 
+void FirstWebContentsProfiler::DidStartNavigation(
+    content::NavigationHandle* navigation_handle) {
+  if (startup_metric_utils::WasNonBrowserUIDisplayed()) {
+    FinishedCollectingMetrics(FinishReason::ABANDON_BLOCKING_UI);
+    return;
+  }
+
+  if (!navigation_handle->IsInMainFrame())
+    return;
+
+  // Record that the navigation identified by |navigation_handle| has started.
+  // DidFinishNavigation() assumes that every |navigation_handle| seen here is a
+  // new navigation, assert that this is true.
+  bool new_navigation =
+      main_navigation_handles_.insert(navigation_handle).second;
+  DCHECK(new_navigation);
+
+  if (!collected_main_navigation_start_metric_) {
+    collected_main_navigation_start_metric_ = true;
+    startup_metric_utils::RecordFirstWebContentsMainNavigationStart(
+        base::Time::Now());
+  }
+}
+
+void FirstWebContentsProfiler::DidFinishNavigation(
+    content::NavigationHandle* navigation_handle) {
+  if (collected_main_navigation_finished_all_metric_)
+    return;
+
+  if (startup_metric_utils::WasNonBrowserUIDisplayed()) {
+    FinishedCollectingMetrics(FinishReason::ABANDON_BLOCKING_UI);
+    return;
+  }
+
+  if (!navigation_handle->IsInMainFrame())
+    return;

[clamy, 2015/10/22 17:21:07]

You may also need to check whether the navigation was aborted or not.
NavigationHandle::HasCommitted will tell you if the navigation committed. If you
need to distinguish between a successful commit and an error page commit, use
NavigationHandle::IsErrorPage.

[gab, 2015/10/22 20:37:39]

Good point, even added it as an explicit early exit criteria like some others we
already have in this file.

+
+  // Remove |navigation_handle| from the tracked |main_navigation_handles_|.
+  size_t handles_tracked = main_navigation_handles_.erase(navigation_handle);
+  DCHECK_EQ(handles_tracked, 1U);
+
+  // First navigation in main frame is complete.
+  if (!collected_main_navigation_finished_metric_) {
+    collected_main_navigation_finished_metric_ = true;
+    startup_metric_utils::RecordFirstWebContentsMainNavigationFinishedFirst(
+        base::Time::Now());
+  }
+
+  // All known navigations in main frame are complete.
+  if (main_navigation_handles_.empty()) {
+    collected_main_navigation_finished_all_metric_ = true;
+    startup_metric_utils::RecordFirstWebContentsMainNavigationFinishedAll(

[clamy, 2015/10/22 17:21:07]

I don't think this is really needed.

While it's true that there can be at most 2 ongoing navigations in the same
frame, the first one two commit will cancel the other one, exceptin one special
case. The special case is a non user-initiated same-site navigation (a
renderer-initiated one) will not cancel an ongoing cross-site navigation (which
is browser-initiated).

In the case of startup, you don't have a renderer with a page loaded in it so
the special case cannot happen. This means that all navigations will always
finish by the time the first of them commit (there are some subtleties if it's
the cross-site one that finishes first due to having to do a round-trip with the
renderer for the execution of the unload event, but we don't allow the same-site
navigation to commit during that time anyway, so the time difference you could
see between the two DidFinishNavigation is not meaningful).

Since you most likely don't need to check that all navigations are finished, you
don't need to store the NavigationHandles either, making this patch simpler :).

[gab, 2015/10/22 20:37:39]

Ah ok, maybe this should be made clearer in WebContentsObserver? i.e. from
someone that doesn't know anything about networking I was worried that this
could matter a lot (i.e.  I thought a "navigation" was to a single host and that
there were many per frame to gather all resources -- though now that I think
about it I guess the navigation brings back the main resource and then the
renderers figure out what other resources are needed from there?).

+        base::Time::Now());
+  }
+}
+
 void FirstWebContentsProfiler::NavigationEntryCommitted(
     const content::LoadCommittedDetails& load_details) {
   // Abandon profiling on any navigation to a different page as it: