[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..e520df93f1a78cbbb67253aea7c4faec654a52f1 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);

[Bryan McQuade, 2016/07/13 18:23:36]

let's make this a const method

[Charlie Harrison, 2016/07/13 18:58:19]

Done.

 
   bool MatchesOriginalNavigation(content::NavigationHandle* navigation_handle);
 
@@ -188,6 +190,10 @@ class PageLoadTracker {
 
   PageLoadExtraInfo ComputePageLoadExtraInfo();
 
+  const base::Optional<ui::PageTransition> page_transition() const {

[Bryan McQuade, 2016/07/13 18:23:36]

do you want to return a const reference here, to avoid the copy?

[Charlie Harrison, 2016/07/13 18:58:19]

Ah yes, this was a typo :)

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

[Bryan McQuade, 2016/07/13 18:23:36]

is the value that NavigationHandle::GetPageTransition() reports at commit ever
less accurate than what we would have received from the throttle? If not, seems
like we might as well just update the PageLoadTracker::Commit() function to use
the updated page transition as part of this change.

[Charlie Harrison, 2016/07/13 18:58:19]

It will only be more accurate. Done.

+  // updating this member then.
+  base::Optional<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.
@@ -289,6 +301,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 +323,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,