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

components/page_load_metrics/browser/metrics_web_contents_observer.h

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_PAGE_LOAD_METRICS_BROWSER_PAGE_LOAD_METRICS_WEB_CONTENTS_OBSERVER_H_
 #define COMPONENTS_PAGE_LOAD_METRICS_BROWSER_PAGE_LOAD_METRICS_WEB_CONTENTS_OBSERVER_H_
 
 #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"
 #include "components/page_load_metrics/common/page_load_timing.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
 #include "net/base/net_errors.h"
 
 class PageLoadMetricsObserverTest;
@@ skipping 47 matching lines @@
 
 const char kProvisionalEvents[] = "PageLoad.Events.Provisional";
 const char kCommittedEvents[] = "PageLoad.Events.Committed";
 const char kBackgroundProvisionalEvents[] =
     "PageLoad.Events.Provisional.Background";
 const char kBackgroundCommittedEvents[] =
     "PageLoad.Events.Committed.Background";
 
 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";
 
 // NOTE: Some of these histograms are separated into a separate histogram
 // specified by the ".Background" suffix. For these events, we put them into the
 // background histogram if the web contents was ever in the background from
 // navigation start to the event in question.
 
 // ProvisionalLoadEvents count all main frame navigations before they commit.
 // The events in this enum are all disjoint, and summing them yields the total
@@ skipping 81 matching lines @@
   ERR_IPC_FROM_BAD_URL_SCHEME,
 
   // If we track a navigation, but the renderer sends us no IPCs. This could
   // occur if the browser filters loads less aggressively than the renderer.
   ERR_NO_IPCS_RECEIVED,
 
   // Add values before this final count.
   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 {
  public:
   virtual ~PageLoadMetricsEmbedderInterface() {}
   virtual rappor::RapporService* GetRapporService() = 0;
   virtual bool IsPrerendering(content::WebContents* web_contents) = 0;
 };
 
 // This class tracks a given page load, starting from navigation start /
@@ skipping 13 matching lines @@
   void Commit(content::NavigationHandle* navigation_handle);
   void WebContentsHidden();
   void WebContentsShown();
 
   // Returns true if the timing was successfully updated.
   bool UpdateTiming(const PageLoadTiming& timing);
   void RecordProvisionalEvent(ProvisionalLoadEvent event);
   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.
   const GURL& GetCommittedURL();
 
   base::TimeDelta GetBackgroundDelta();
   void RecordTimingHistograms();
+  void RecordAbortTimingHistograms();
   void RecordRappor();
 
   bool has_commit_;
 
   // 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.
   base::TimeTicks background_time_;
   base::TimeTicks foreground_time_;
   bool started_in_foreground_;
 
   PageLoadTiming timing_;
   GURL url_;
 
@@ skipping 27 matching lines @@
   void AddObserver(PageLoadMetricsObserver* observer) override;
   void RemoveObserver(PageLoadMetricsObserver* observer) override;
 
   // 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 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.
   bool in_foreground_;
 
   // This map tracks all of the navigations ongoing that are not committed
   // yet. Once a navigation is committed, it moves from the map to
   // committed_load_. Note that a PageLoadTrackers NavigationHandle is only
   // 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_;
   base::ObserverList<PageLoadMetricsObserver, true> observers_;
 
   DISALLOW_COPY_AND_ASSIGN(MetricsWebContentsObserver);
 };
 
 }  // namespace page_load_metrics
 
 #endif  // COMPONENTS_PAGE_LOAD_METRICS_BROWSER_PAGE_LOAD_METRICS_WEB_CONTENTS_OBSERVER_H_