[page_load_metrics] Add a NavigationThrottle for richer abort metrics

components/page_load_metrics/browser/metrics_web_contents_observer.h

Index: components/page_load_metrics/browser/metrics_web_contents_observer.h
diff --git a/components/page_load_metrics/browser/metrics_web_contents_observer.h b/components/page_load_metrics/browser/metrics_web_contents_observer.h
index fd06800b54fe3396ec79a05bfebbdd185fc067e9..a2ec0abb0a48e38c764cdfc7817a7d76dcf950b4 100644
--- a/components/page_load_metrics/browser/metrics_web_contents_observer.h
+++ b/components/page_load_metrics/browser/metrics_web_contents_observer.h
@@ -179,7 +179,7 @@ class PageLoadTracker {
   // delta to when it was aborted. Note that only provisional loads can be
   // aborted with ABORT_OTHER. While this heuristic is coarse, it works better
   // and is simpler than other feasible methods. See https://goo.gl/WKRG98.
-  bool IsLikelyProvisionalAbort(base::TimeTicks abort_cause_time);
+  bool IsLikelyProvisionalAbort(base::TimeTicks abort_cause_time) const;
 
   bool MatchesOriginalNavigation(content::NavigationHandle* navigation_handle);
 
@@ -193,6 +193,8 @@ class PageLoadTracker {
 
   PageLoadExtraInfo ComputePageLoadExtraInfo();
 
+  ui::PageTransition page_transition() const { return page_transition_; }
+
  private:
   // This function converts a TimeTicks value taken in the browser process
   // to navigation_start_ if:
@@ -244,6 +246,8 @@ class PageLoadTracker {
   PageLoadTiming timing_;
   PageLoadMetadata metadata_;
 
+  ui::PageTransition page_transition_;
+
   // This is a subtle member. If a provisional load A gets aborted by
   // provisional load B, which gets aborted by C that eventually commits, then
   // there exists an abort chain of length 2, starting at A's navigation_start.
@@ -284,8 +288,6 @@ class MetricsWebContentsObserver
   // content::WebContentsObserver implementation:
   bool OnMessageReceived(const IPC::Message& message,
                          content::RenderFrameHost* render_frame_host) override;
-  void DidStartNavigation(
-      content::NavigationHandle* navigation_handle) override;
   void DidFinishNavigation(
       content::NavigationHandle* navigation_handle) override;
   void DidRedirectNavigation(
@@ -298,6 +300,9 @@ class MetricsWebContentsObserver
   void RenderViewHostChanged(content::RenderViewHost* old_host,
                              content::RenderViewHost* new_host) override;
 
+  // This method is forwarded from the MetricsNavigationThrottle.
+  void WillStartNavigationRequest(content::NavigationHandle* navigation_handle);

[Bryan McQuade, 2016/07/20 21:51:11]

this is great - a really nice clean change. my only question/concern is whether
there are cases where we would have gotten a DidStartNavigation call that we
won't get a WillStartNavigationRequestCall. Is that a possibility? Could that
cause us to fail to track certain pages that we'd otherwise want to track? My
guess is that it's not possible, but I wanted to check to be sure.

[Charlie Harrison, 2016/07/21 14:16:16]

I'm fairly sure this will always be called. It corresponds to
ResourceHandler::OnWillStart which is "Called before the net::URLRequest (whose
url is |url|) is to be started."

+clamy@ in case I'm wrong.

+
   // This getter function is required for testing.
   const PageLoadExtraInfo GetPageLoadExtraInfoForCommittedLoad();