[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 d9e06e865eca3d776939d9d019216c5e96a31b33..9c8ebc4851a70c17550382d245cbd9a94904aaa8 100644
--- a/components/page_load_metrics/browser/metrics_web_contents_observer.h
+++ b/components/page_load_metrics/browser/metrics_web_contents_observer.h
@@ -128,6 +128,7 @@ class PageLoadTracker {
                   int aborted_chain_size,
                   int aborted_chain_size_same_url);
   ~PageLoadTracker();
+  void WillStartNavigationRequest(content::NavigationHandle* navigation_handle);
   void Redirect(content::NavigationHandle* navigation_handle);
   void Commit(content::NavigationHandle* navigation_handle);
   void FailedProvisionalLoad(content::NavigationHandle* navigation_handle);
@@ -169,12 +170,13 @@ class PageLoadTracker {
                    base::TimeTicks timestamp,
                    bool is_certainly_browser_timestamp);
 
-  // This method returns true if this page load has been aborted with type of
-  // ABORT_OTHER, and the |abort_cause_time| is within a sufficiently close
-  // 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);
+  // This method returns true if this page load has been aborted with the
+  // |abort_cause_time| within a sufficiently close delta to when it was
+  // aborted. This is used to update aborts with less precise abort causes like
+  // ABORT_OTHER and ABORT_UNKNOWN_NAVIGATION. While this heuristic is coarse,
+  // it works better and is simpler than other feasible methods. See
+  // https://goo.gl/WKRG98.
+  bool WasAbortedRecently(base::TimeTicks abort_cause_time);
 
   bool MatchesOriginalNavigation(content::NavigationHandle* navigation_handle);
 
@@ -188,6 +190,8 @@ class PageLoadTracker {
 
   PageLoadExtraInfo ComputePageLoadExtraInfo();
 
+  ui::PageTransition page_transition() { return page_transition_; }

[Bryan McQuade, 2016/07/12 16:45:29]

nit: can we make this method const?

[Charlie Harrison, 2016/07/12 19:20:22]

Done.

+
  private:
   // This function converts a TimeTicks value taken in the browser process
   // to navigation_start_ if:
@@ -235,6 +239,12 @@ class PageLoadTracker {
   PageLoadTiming timing_;
   PageLoadMetadata metadata_;
 
+  // The page transition type of this page load. This will be
+  // PAGE_TRANSITION_LAST_CORE until PageLoadTracker::WillStartRequest.
+  // TODO(csharrison): Some transitions are only known at commit time. Consider
+  // updating this member then.
+  ui::PageTransition page_transition_;

[Bryan McQuade, 2016/07/12 16:45:29]

rather than defaulting to PAGE_TRANSITION_LAST_CORE, which feels a bit
unintuitive, why don't we use base::Optional<ui::PageTransition> here instead?
and page_transition() can return a const Optional<>&, or it can be documented to
only be valid to invoke after some certain time, and we can DCHECK() that the
optional is set when it gets invoked.

[Charlie Harrison, 2016/07/12 19:20:22]

Done.

+
   // 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.
@@ -289,6 +299,9 @@ class MetricsWebContentsObserver
   void RenderViewHostChanged(content::RenderViewHost* old_host,
                              content::RenderViewHost* new_host) override;
 
+  // Forwarded call from a navigation throttle.
+  void WillStartNavigationRequest(content::NavigationHandle* navigation_handle);
+
   // This getter function is required for testing.
   const PageLoadExtraInfo GetPageLoadExtraInfoForCommittedLoad();
 
@@ -308,10 +321,9 @@ class MetricsWebContentsObserver
 
   // Notify aborted provisional loads that a new navigation occurred. This is
   // used for more consistent attribution tracking for aborted provisional
-  // loads. This method returns the provisional load that was likely aborted
-  // by this navigation, to help instantiate the new PageLoadTracker.
-  std::unique_ptr<PageLoadTracker> NotifyAbortedProvisionalLoadsNewNavigation(
-      content::NavigationHandle* new_navigation);
+  // loads.
+  void NotifyAbortedProvisionalLoadsNewNavigation(
+      PageLoadTracker* new_navigation);
 
   void OnTimingUpdated(content::RenderFrameHost*,
                        const PageLoadTiming& timing,