[do not review][page_load_metrics] User Initiated Abort Tracking

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 f0a6bc02730e6897e571a3fe4080cd29aaf7e595..5567cf2dfa45347875389e3c25cf9ff21cdc33bf 100644
--- a/components/page_load_metrics/browser/metrics_web_contents_observer.h
+++ b/components/page_load_metrics/browser/metrics_web_contents_observer.h
@@ -7,6 +7,7 @@
 
 #include "base/containers/scoped_ptr_map.h"
 #include "base/macros.h"
+#include "base/memory/scoped_vector.h"
 #include "base/observer_list.h"
 #include "base/time/time.h"
 #include "components/page_load_metrics/browser/page_load_metrics_observer.h"
@@ -74,6 +75,35 @@ const char kBackgroundCommittedEvents[] =
 
 const char kErrorEvents[] = "PageLoad.Events.InternalError";
 
+const char kHistogramProvisionalAbortForwardBack[] =
+    "PageLoad.Timing2.Aborts.Provisional.ForwardBackBeforeFirstPaint";

[Bryan McQuade, 2015/11/17 13:33:08]

do you think the provisional vs committed distinction is important here? from a
user standpoint whether we aborted before vs after commit doesn't seem all that
significant to me. i could see it making sense to track a general time-to-commit
stat to track how quickly we get to commit (for e.g. plznav) but i'm not sure we
also need to track time-to-abort before vs after commit.

[Bryan McQuade, 2015/11/17 13:33:08]

can we use 'BeforePaint' rather than 'BeforeFirstPaint' to shorten these names a
bit?

[Bryan McQuade, 2015/11/17 13:33:08]

nit: let's stick to singular rather than pluran in these names, so 'Abort'
rather than 'Aborts'.

[Charlie Harrison, 2015/11/17 16:11:54]

Done. (Aborts => Abort)

[Charlie Harrison, 2015/11/17 16:11:55]

Done. (BeforePaint)

[Charlie Harrison, 2015/11/17 16:11:55]

Yeah the distinction is not great, but there are two main reasons to do this:
- We don't know if a provisional load is relevant
- We have much coarser heuristics for provisional loads

+const char kHistogramProvisionalAbortReload[] =
+    "PageLoad.Timing2.Aborts.Provisional.ReloadBeforeFirstPaint";
+const char kHistogramProvisionalAbortNewNavigation[] =
+    "PageLoad.Timing2.Aborts.Provisional.NewNavigationBeforeFirstPaint";
+const char kHistogramProvisionalAbortStop[] =
+    "PageLoad.Timing2.Aborts.Provisional.StopBeforeFirstPaint";
+const char kHistogramProvisionalAbortClose[] =
+    "PageLoad.Timing2.Aborts.Provisional.CloseBeforeFirstPaint";
+const char kHistogramProvisionalAbortOther[] =
+    "PageLoad.Timing2.Aborts.Provisional.OtherAbortBeforeFirstPaint";
+const char kHistogramProvisionalAbortBackground[] =
+    "PageLoad.Timing2.Aborts.Provisional.BackgroundBeforeFirstPaint";
+const char kHistogramCommittedAbortForwardBack[] =
+    "PageLoad.Timing2.Aborts.Committed.ForwardBackBeforeFirstPaint";
+const char kHistogramCommittedAbortReload[] =
+    "PageLoad.Timing2.Aborts.Committed.ReloadBeforeFirstPaint";
+const char kHistogramCommittedAbortNewNavigation[] =
+    "PageLoad.Timing2.Aborts.Committed.NewNavigationBeforeFirstPaint";
+const char kHistogramCommittedAbortStop[] =
+    "PageLoad.Timing2.Aborts.Committed.StopBeforeFirstPaint";
+const char kHistogramCommittedAbortClose[] =
+    "PageLoad.Timing2.Aborts.Committed.CloseBeforeFirstPaint";
+const char kHistogramCommittedAbortOther[] =
+    "PageLoad.Timing2.Aborts.Committed.OtherAbortBeforeFirstPaint";
+const char kHistogramCommittedAbortBackground[] =
+    "PageLoad.Timing2.Aborts.Committed.BackgroundBeforeFirstPaint";
+
 const char kRapporMetricsNameCoarseTiming[] =
     "PageLoad.CoarseTiming.NavigationToFirstContentfulPaint";
 
@@ -175,6 +205,38 @@ enum InternalErrorLoadEvent {
   ERR_LAST_ENTRY
 };
 
+// This enum represents how a page load ends. If the action occurs before the
+// page load finishes (or reaches some point like first paint), then we consider
+// the load to be aborted.
+enum UserAbortType {
+  // Represents no abort.
+  ABORT_NULL,

[Bryan McQuade, 2015/11/17 13:33:08]

most enums I see use a _NONE suffix for this, rather than _NULL.

[Charlie Harrison, 2015/11/17 16:11:54]

Done.

+
+  // If the user presses reload or shift-reload.
+  ABORT_RELOAD,
+
+  // The user presses the back/forward button.
+  ABORT_FORWARD_BACK,
+
+  // If the navigation is replaced by a new navigation. This includes link
+  // clicks, typing in the omnibox (not a reload), and form submissions.
+  ABORT_NEW_NAVIGATION,
+
+  // If the user presses the stop X button.
+  ABORT_STOP,
+
+  // If the navigation is aborted by closing the tab or browser.
+  ABORT_CLOSE,
+
+  // We don't know why the navigation aborted. This is the value we assign to an
+  // aborted load if the only signal we get is a provisional load finishing
+  // without committing, either without error or with net::ERR_ABORTED.
+  ABORT_OTHER,
+
+  // Add values before this final count.
+  ABORT_LAST_ENTRY
+};
+
 // This class serves as a functional interface to various chrome// features.
 // Impl version is defined in chrome/browser/page_load_metrics.
 class PageLoadMetricsEmbedderInterface {
@@ -208,6 +270,11 @@ class PageLoadTracker {
   void RecordCommittedEvent(CommittedLoadEvent event, bool backgrounded);
   bool HasBackgrounded();
 
+  // If the user performs some abort-like action while we are tracking this page
+  // load, notify the tracker. Note that we may not classify this as an abort if
+  // we've already performed a first paint.
+  void NotifyAbort(UserAbortType abort_type, const base::TimeTicks& timestamp);
+
  private:
   PageLoadExtraInfo GetPageLoadMetricsInfo();
   // Only valid to call post-commit.
@@ -215,6 +282,7 @@ class PageLoadTracker {
 
   base::TimeDelta GetBackgroundDelta();
   void RecordTimingHistograms();
+  void RecordAbortTimingHistograms();
   void RecordRappor();
 
   bool has_commit_;
@@ -222,6 +290,11 @@ class PageLoadTracker {
   // The navigation start in TimeTicks, not the wall time reported by Blink.
   const base::TimeTicks navigation_start_;
 
+  // Will be ABORT_NULL if we have not aborted this load yet. Otherwise will
+  // be the first abort action the user performed.
+  UserAbortType abort_type_;
+  base::TimeTicks abort_time_;
+
   // We record separate metrics for events that occur after a background,
   // because metrics like layout/paint are delayed artificially
   // when they occur in the background.
@@ -269,15 +342,20 @@ class MetricsWebContentsObserver
       content::NavigationHandle* navigation_handle) override;
   void DidFinishNavigation(
       content::NavigationHandle* navigation_handle) override;
-
+  void NavigationStopped() override;
   void WasShown() override;
   void WasHidden() override;
-
   void RenderProcessGone(base::TerminationStatus status) override;
 
  private:
   friend class content::WebContentsUserData<MetricsWebContentsObserver>;
 
+  // Notify all loads, provisional and committed, that we performed an action
+  // that might abort them.
+  void NotifyAbortAllLoads(UserAbortType abort_type);
+  void NotifyAbortAllLoadsWithTimestamp(UserAbortType abort_type,
+                                        const base::TimeTicks& timestamp);
+
   void OnTimingUpdated(content::RenderFrameHost*, const PageLoadTiming& timing);
 
   // True if the web contents is currently in the foreground.
@@ -289,6 +367,12 @@ class MetricsWebContentsObserver
   // valid until commit time, when we remove it from the map.
   base::ScopedPtrMap<content::NavigationHandle*, scoped_ptr<PageLoadTracker>>
       provisional_loads_;
+  // Tracks aborted provisional loads for a little bit longer than usual (one
+  // more navigation commit at the max), in order to better understand how the
+  // navigation failed. This is because most provisional loads are destroyed and
+  // vanish before we get signal about what caused the abort (new navigation,
+  // stop button, etc.).
+  ScopedVector<PageLoadTracker> aborted_provisional_loads_;
   scoped_ptr<PageLoadTracker> committed_load_;
 
   scoped_ptr<PageLoadMetricsEmbedderInterface> embedder_interface_;