[page_load_metrics] Add a NavigationThrottle for richer abort metrics

chrome/browser/page_load_metrics/metrics_navigation_throttle.h

Index: chrome/browser/page_load_metrics/metrics_navigation_throttle.h
diff --git a/chrome/browser/page_load_metrics/metrics_navigation_throttle.h b/chrome/browser/page_load_metrics/metrics_navigation_throttle.h
new file mode 100644
index 0000000000000000000000000000000000000000..64a3ebfa4fe03b698cec14d76dfb40e21f3ecef4
--- /dev/null
+++ b/chrome/browser/page_load_metrics/metrics_navigation_throttle.h
@@ -0,0 +1,36 @@
+// Copyright 2016 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 CHROME_BROWSER_PAGE_LOAD_METRICS_METRICS_NAVIGATION_THROTTLE_H_
+#define CHROME_BROWSER_PAGE_LOAD_METRICS_METRICS_NAVIGATION_THROTTLE_H_
+
+#include <memory>
+
+#include "base/macros.h"
+#include "content/public/browser/navigation_throttle.h"
+
+namespace page_load_metrics {
+
+// This class is used to forward calls to the MetricsWebContentsObserver.
+// Namely, WillStartRequest() is called on NavigationThrottles, but not on
+// WebContentsObservers. Data from the NavigationHandle accessed at this point
+// is used to obtain more reliable abort metrics (like page transition type).

[nasko, 2016/08/01 15:29:22]

Based on some poking I've done on separate CLs I've been working on, I'm not
sure that the page transition type is correct that early in renderer-initiated
navigations. Have you confirmed you are getting what you expect?

[Charlie Harrison, 2016/08/01 17:42:19]

It is generally correct. There are some gotchas around AUTO_SUBFRAME but we
aren't really concerned about that case. I tested:
link clicks
form submits
window.location = <new url>
window.location.reload
history.back

and they all have identical transition at WillStart and DidFinish navigation.
Let me know if you remember specific cases though. In general I would say we are
okay with being wrong for some subset of navigations, as long as our coverage
increases from the 0 it is at today :)

I'll add a thank you for making me do the work right now to verify, because I've
found that JS initiated history navs do not show up as RELOAD / FORWARD_BACK but
do show up as LINK | CLIENT_SIDE_REDIRECT (both before and after commit). I'll
include a histogram for that case in a later CL, as I suspect that will actually
get rid of a LOT of noise in the abort histograms!

I've added a TODO in MetricsWebContentsObserver.

+class MetricsNavigationThrottle : public content::NavigationThrottle {
+ public:
+  static std::unique_ptr<content::NavigationThrottle> Create(
+      content::NavigationHandle* handle);
+  ~MetricsNavigationThrottle() override;
+
+  // content::NavigationThrottle:
+  content::NavigationThrottle::ThrottleCheckResult WillStartRequest() override;
+
+ private:
+  explicit MetricsNavigationThrottle(content::NavigationHandle* handle);
+
+  DISALLOW_COPY_AND_ASSIGN(MetricsNavigationThrottle);
+};
+
+}  // namespace page_load_metrics
+
+#endif  // CHROME_BROWSER_PAGE_LOAD_METRICS_METRICS_NAVIGATION_THROTTLE_H_