Generalize abort tracking to page end state tracking

chrome/browser/page_load_metrics/page_load_tracker.h

Index: chrome/browser/page_load_metrics/page_load_tracker.h
diff --git a/chrome/browser/page_load_metrics/page_load_tracker.h b/chrome/browser/page_load_metrics/page_load_tracker.h
index ef782b2a6406e91f60814b8ad272c4a4eae62d77..c08628d0d7236c1f13edb318241a19fee8fc8e6f 100644
--- a/chrome/browser/page_load_metrics/page_load_tracker.h
+++ b/chrome/browser/page_load_metrics/page_load_tracker.h
@@ -77,11 +77,11 @@ enum InternalErrorLoadEvent {
   // occur if the browser filters loads less aggressively than the renderer.
   ERR_NO_IPCS_RECEIVED,
 
-  // Tracks frequency with which we record an abort time that occurred before
+  // Tracks frequency with which we record an end time that occurred before
   // navigation start. This is expected to happen in some cases (see comments in
   // cc file for details). We use this error counter to understand how often it
   // happens.
-  ERR_ABORT_BEFORE_NAVIGATION_START,
+  ERR_END_BEFORE_NAVIGATION_START,

[Charlie Harrison, 2017/02/21 19:49:31]

please update the histograms xml file for this.

[Bryan McQuade, 2017/02/21 21:51:59]

Done, thanks!

 
   // A new navigation triggers abort updates in multiple trackers in
   // |aborted_provisional_loads_|, when usually there should only be one (the
@@ -104,6 +104,9 @@ enum InternalErrorLoadEvent {
   // commit nor a failed provisional load.
   ERR_NO_COMMIT_OR_FAILED_PROVISIONAL_LOAD,
 
+  // No page load end time was recorded for this page load.
+  ERR_NO_PAGE_LOAD_END_TIME,
+
   // Add values before this final count.
   ERR_LAST_ENTRY,
 };
@@ -112,7 +115,7 @@ enum InternalErrorLoadEvent {
 // metrics_web_contents_observer.cc. They are declared here to allow both files
 // to access them.
 void RecordInternalError(InternalErrorLoadEvent event);
-UserAbortType AbortTypeForPageTransition(ui::PageTransition transition);
+PageEndReason EndReasonForPageTransition(ui::PageTransition transition);
 void LogAbortChainSameURLHistogram(int aborted_chain_size_same_url);
 bool IsNavigationUserInitiated(content::NavigationHandle* handle);
 
@@ -138,7 +141,8 @@ class PageLoadTracker {
   void WillProcessNavigationResponse(
       content::NavigationHandle* navigation_handle);
   void Commit(content::NavigationHandle* navigation_handle);
-  void FailedProvisionalLoad(content::NavigationHandle* navigation_handle);
+  void FailedProvisionalLoad(content::NavigationHandle* navigation_handle,
+                             base::TimeTicks failed_load_time);
   void WebContentsHidden();
   void WebContentsShown();
 
@@ -168,8 +172,8 @@ class PageLoadTracker {
     return aborted_chain_size_same_url_;
   }
 
-  UserAbortType abort_type() const { return abort_type_; }
-  base::TimeTicks abort_time() const { return abort_time_; }
+  PageEndReason page_end_reason() const { return page_end_reason_; }
+  base::TimeTicks page_end_time() const { return page_end_time_; }
 
   void AddObserver(std::unique_ptr<PageLoadMetricsObserver> observer);
 
@@ -180,19 +184,19 @@ class PageLoadTracker {
   // in the
   // browser process or not. We need this to possibly clamp browser timestamp on
   // a machine with inter process time tick skew.
-  void NotifyAbort(UserAbortType abort_type,
-                   UserInitiatedInfo user_initiated_info,
-                   base::TimeTicks timestamp,
-                   bool is_certainly_browser_timestamp);
-  void UpdateAbort(UserAbortType abort_type,
-                   UserInitiatedInfo user_initiated_info,
-                   base::TimeTicks timestamp,
-                   bool is_certainly_browser_timestamp);
+  void NotifyPageEnd(PageEndReason page_end_reason,
+                     UserInitiatedInfo user_initiated_info,
+                     base::TimeTicks timestamp,
+                     bool is_certainly_browser_timestamp);
+  void UpdatePageEnd(PageEndReason page_end_reason,
+                     UserInitiatedInfo user_initiated_info,
+                     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
+  // END_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
+  // aborted with END_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) const;
 
@@ -228,10 +232,10 @@ class PageLoadTracker {
   void ClampBrowserTimestampIfInterProcessTimeTickSkew(
       base::TimeTicks* event_time);
 
-  void UpdateAbortInternal(UserAbortType abort_type,
-                           UserInitiatedInfo user_initiated_info,
-                           base::TimeTicks timestamp,
-                           bool is_certainly_browser_timestamp);
+  void UpdatePageEndInternal(PageEndReason page_end_reason,
+                             UserInitiatedInfo user_initiated_info,
+                             base::TimeTicks timestamp,
+                             bool is_certainly_browser_timestamp);
 
   // If |final_navigation| is null, then this is an "unparented" abort chain,
   // and represents a sequence of provisional aborts that never ends with a
@@ -266,21 +270,21 @@ class PageLoadTracker {
 
   std::unique_ptr<FailedProvisionalLoadInfo> failed_provisional_load_info_;
 
-  // Will be ABORT_NONE if we have not aborted this load yet. Otherwise will
-  // be the first abort action the user performed.
-  UserAbortType abort_type_;
-
-  // Whether the abort for this page load was user initiated. For example, if
-  // this page load was aborted by a new navigation, this field tracks whether
-  // that new navigation was user-initiated. This field is only useful if this
-  // page load's abort type is a value other than ABORT_NONE. Note that this
-  // value is currently experimental, and is subject to change. In particular,
-  // this field is never set to true for some abort types, such as stop and
-  // close, since we don't yet have sufficient instrumentation to know if a stop
-  // or close was caused by a user action.
-  UserInitiatedInfo abort_user_initiated_info_;
-
-  base::TimeTicks abort_time_;
+  // Will be END_NONE if we have not ended this load yet. Otherwise will
+  // be the first page end reason encountered.
+  PageEndReason page_end_reason_;
+
+  // Whether the page end cause for this page load was user initiated. For
+  // example, if this page load was ended by a new navigation, this field tracks
+  // whether that new navigation was user-initiated. This field is only useful
+  // if this page load's end reason is a value other than END_NONE. Note that
+  // this value is currently experimental, and is subject to change. In
+  // particular, this field is never set to true for some page end reasons, such
+  // as stop and close, since we don't yet have sufficient instrumentation to
+  // know if a stop or close was caused by a user action.
+  UserInitiatedInfo page_end_user_initiated_info_;
+
+  base::TimeTicks page_end_time_;
 
   // We record separate metrics for events that occur after a background,
   // because metrics like layout/paint are delayed artificially