[page_load_metrics] Add a NavigationThrottle for richer abort metrics

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_METRICS_WEB_CONTENTS_OBSERVER_H_
 #define COMPONENTS_PAGE_LOAD_METRICS_BROWSER_METRICS_WEB_CONTENTS_OBSERVER_H_
 
 #include <map>
 #include <memory>
 #include <vector>
@@ skipping 110 matching lines @@
   // class. The PageLoadTracker must not hold on to
   // currently_committed_load_or_null or navigation_handle beyond the scope of
   // the constructor.
   PageLoadTracker(bool in_foreground,
                   PageLoadMetricsEmbedderInterface* embedder_interface,
                   const GURL& currently_committed_url,
                   content::NavigationHandle* navigation_handle,
                   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);
   void WebContentsHidden();
   void WebContentsShown();
 
   void OnInputEvent(const blink::WebInputEvent& event);
 
   void NotifyClientRedirectTo(const PageLoadTracker& destination);
 
@@ skipping 21 matching lines @@
   // 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,
                    base::TimeTicks timestamp,
                    bool is_certainly_browser_timestamp);
   void UpdateAbort(UserAbortType abort_type,
                    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);
 
   // Only valid to call post-commit.
   const GURL& committed_url() const {
     DCHECK(!commit_time_.is_null());
     return url_;
   }
 
   base::TimeTicks navigation_start() const { return navigation_start_; }
 
   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:
   // - base::TimeTicks is not comparable across processes because the clock
   // is not system wide monotonic.
   // - *event_time < navigation_start_
   void ClampBrowserTimestampIfInterProcessTimeTickSkew(
       base::TimeTicks* event_time);
 
   void UpdateAbortInternal(UserAbortType abort_type,
@@ skipping 27 matching lines @@
   // 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_;
   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.
   // This is useful because it allows histograming abort chain lengths based on
   // what the last load's transition type is. i.e. holding down F-5 to spam
   // reload will produce a long chain with the RELOAD transition.
   const int aborted_chain_size_;
 
   // This member counts consecutive provisional aborts that share a url. It will
   // always be less than or equal to |aborted_chain_size_|.
@@ skipping 34 matching lines @@
   void DidRedirectNavigation(
       content::NavigationHandle* navigation_handle) override;
   void NavigationStopped() override;
   void OnInputEvent(const blink::WebInputEvent& event) override;
   void WasShown() override;
   void WasHidden() override;
   void RenderProcessGone(base::TerminationStatus status) override;
   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();
 
  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,
                                         base::TimeTicks timestamp,
                                         bool is_certainly_browser_timestamp);
 
   // Register / Unregister input event callback to given RenderViewHost
   void RegisterInputEventObserver(content::RenderViewHost* host);
   void UnregisterInputEventObserver(content::RenderViewHost* host);
 
   // 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,
                        const PageLoadMetadata& metadata);
 
   // True if the web contents is currently in the foreground.
   bool in_foreground_;
 
   // The PageLoadTrackers must be deleted before the |embedder_interface_|,
   // because they hold a pointer to the |embedder_interface_|.
@@ skipping 17 matching lines @@
 
   // Has the MWCO observed at least one navigation?
   bool has_navigated_;
 
   DISALLOW_COPY_AND_ASSIGN(MetricsWebContentsObserver);
 };
 
 }  // namespace page_load_metrics
 
 #endif  // COMPONENTS_PAGE_LOAD_METRICS_BROWSER_METRICS_WEB_CONTENTS_OBSERVER_H_