Add common page filtering logic for page load metrics.

chrome/common/page_load_metrics/page_track_decider.h

+// 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_COMMON_PAGE_LOAD_METRICS_PAGE_TRACK_DECIDER_H_
+#define CHROME_COMMON_PAGE_LOAD_METRICS_PAGE_TRACK_DECIDER_H_
+
+#include "base/macros.h"
+
+namespace page_load_metrics {
+
+// PageTrackDecider is an interface used to determine whether metrics should be
+// tracked for a given page. This class is used by the core page load metrics
+// tracking infrastructure in both the browser and render processes so policy
+// decisions about which page loads to track are consistent across processes.
+class PageTrackDecider {
+ public:
+  PageTrackDecider();
+  virtual ~PageTrackDecider();
+
+  // Whether metrics should be tracked for the current page. This method invokes
+  // the pure virtual methods below to decide whether metrics should be tracked.
+  bool ShouldTrack();
+
+  // Whether the navigation for the current page has committed.
+  virtual bool HasCommitted() = 0;
+
+  // Whether the current page's URL is HTTP or HTTPS. Note that the page does
+  // not need to have committed, nor does the URL or hostname of the URL need to
+  // point at a valid web page or host, for this method to return true.
+  virtual bool IsHTTPOrHTTPSUrl() = 0;

[Charlie Harrison, 2016/09/14 13:20:05]

I think this should this be "HttpOrHttps".

[Bryan McQuade, 2016/09/14 15:10:58]

Yeah, I prefer the style you note as well, however existing Chromium APIs use
all caps. In particular, GURL's API uses all caps, so I did this to be
consistent.

[Charlie Harrison, 2016/09/14 16:27:32]

The new style guide recommends this in new code:
http://google.github.io/styleguide/cppguide.html#Function_Names

Here is the associated chromium-dev thread:
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/k8-ztIrWaUY
Consensus on the thread seems to stick to style guide for new code.

[Bryan McQuade, 2016/09/15 22:28:10]

Ah, great, thanks! I updated the code to use Http rather than HTTP.

+
+  // Whether the current page's URL is for a Chrome new tab page. Note that in
+  // some cases the new tab page is served over the network via https.
+  virtual bool IsNewTabPageUrl() = 0;
+
+  // The methods below can only be called if HasCommitted() returns true.
+
+  // Whether the current page is a Chrome-generated error page. Example error
+  // pages include pages displayed after a network error that did not result in
+  // receiving an HTTP/HTTPS response, such as the 'There is no Internet
+  // connection' page, which is displayed when the user attempts to navigate
+  // without an Internet connection. This method returns false for non-Chrome
+  // generated error pages, such as pages with 4xx/5xx response codes. To
+  // check whether the current page resulted in a 4xx/5xx error, use
+  // IsHTTPErrorPage() instead.
+  virtual bool IsChromeErrorPage() = 0;
+
+  // Whether the current page is an HTTP(S) error page received over the
+  // network, such as a page with a 4xx/5xx response code.
+  virtual bool IsHTTPErrorPage() = 0;

[Charlie Harrison, 2016/09/14 13:20:05]

Is there any reason you didn't want to go with "IsNotHTTP2XXOr3XX" or something
similar? That's more clear to me.

[Bryan McQuade, 2016/09/14 15:10:58]

My concern with that is that it may be overly descriptive - to be honest I'm not
100% certain that we want to allow all 2xx and 3xx responses. For instance I've
wondered about 204 (we should exclude them), as well as 3xx that aren't 304 (all
other 3xx I'm aware of are for redirects - seems like an error to record those
as page loads).

For now I switched the API to return the status code, and the policy for
deciding if it's an error page now lives in the base class's ShouldTrack method.
I need to do a separate pass to decide about 204 and non-304 3xx, but I think
returning the status code is an improvement over the boolean method.

[Charlie Harrison, 2016/09/14 16:27:32]

Your change sounds good. This makes it easier for an implementer to know what to
actually implement in the method :)

+
+  // Whether the current page is an HTML or XHTML page.
+  virtual bool IsHTMLOrXHTMLPage() = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PageTrackDecider);
+};
+
+}  // namespace page_load_metrics
+
+#endif  // CHROME_COMMON_PAGE_LOAD_METRICS_PAGE_TRACK_DECIDER_H_