Standardize which page loads are tracked

components/page_load_metrics/browser/page_load_metrics_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_OBSERVER_H_
 #define COMPONENTS_PAGE_LOAD_METRICS_BROWSER_PAGE_LOAD_METRICS_OBSERVER_H_
 
 #include "base/macros.h"
 #include "base/optional.h"
 #include "components/page_load_metrics/common/page_load_timing.h"
@@ skipping 74 matching lines @@
   // aborted, |abort_type| will be |ABORT_NONE|.
   const UserAbortType abort_type;
   const base::Optional<base::TimeDelta> time_to_abort;
 
   // Extra information supplied to the page load metrics system from the
   // renderer.
   const PageLoadMetadata metadata;
 };
 
 // Interface for PageLoadMetrics observers. All instances of this class are
-// owned by the PageLoadTracker tracking a page load. They will be deleted after
-// calling OnComplete.
+// owned by the PageLoadTracker tracking a page load.
 class PageLoadMetricsObserver {
  public:
   virtual ~PageLoadMetricsObserver() {}
 
   // The page load started, with the given navigation handle. Note that OnStart
   // is called for same-page navigations. Implementers of OnStart that only want
   // to process non-same-page navigations should also check to see that the page
   // load committed via OnCommit or committed_url in
   // PageLoadExtraInfo. currently_committed_url contains the URL of the
   // committed page load at the time the navigation for navigation_handle was
@@ skipping 15 matching lines @@
   // reference to it.
   // Note that this does not get called for same page navigations.
   virtual void OnCommit(content::NavigationHandle* navigation_handle) {}
 
   // OnFailedProvisionalLoad is triggered when a provisional load failed and did
   // not commit. Note that provisional loads that result in downloads or 204s
   // are aborted by the system, and thus considered failed provisional loads.
   virtual void OnFailedProvisionalLoad(
       content::NavigationHandle* navigation_handle) {}
 
   // OnTimingUpdate is triggered when an updated PageLoadTiming is

[Charlie Harrison, 2016/07/15 13:01:17]

Can you include a comment that none of the On* callbacks will be called for non
tracked pages? OnComplete is not important for a big chunk of observers.

[Bryan McQuade, 2016/07/15 14:14:43]

Good point, yes, I re-ordered the callbacks and added a comment to note which
callbacks won't be invoked for untracked pages.

   // available. This method may be called multiple times over the course of the
-  // page load. Note that this is currently an experimental API which may be
-  // removed in the future. Please email loading-dev@chromium.org if you intend
-  // to override this method.
+  // page load. This method is currently only intended for use in testing. Most
+  // implementers should implement one of the On* callbacks, such as
+  // OnFirstContentfulPaint or OnDomContentLoadedEventStart. Please email
+  // loading-dev@chromium.org if you intend to override this method.
   virtual void OnTimingUpdate(const PageLoadTiming& timing,
                               const PageLoadExtraInfo& extra_info) {}
 
   // OnHidden is triggered when a page leaves the foreground. It does not fire
   // when a foreground page is permanently closed; for that, listen to
   // OnComplete instead.
   virtual void OnHidden() {}
 
   // OnShown is triggered when a page is brought to the foreground. It does not
   // fire when the page first loads; for that, listen for OnStart instead.
   virtual void OnShown() {}
 
   // OnComplete is triggered when we are ready to record metrics for this page
   // load. This will happen some time after commit. The PageLoadTiming struct
   // contains timing data and the PageLoadExtraInfo struct contains other useful
-  // data collected over the course of the page load. If the load did not
-  // receive any timing information, |timing.IsEmpty()| will be true.
-  // After this call, the object will be deleted.
+  // data collected over the course of the page load. OnComplete will not be
+  // called for page loads that did not meet the criteria for tracking metrics
+  // at the time the navigation committed. After this call, the object will be
+  // deleted.
   virtual void OnComplete(const PageLoadTiming& timing,
                           const PageLoadExtraInfo& extra_info) {}
 
   // OnUserInput is triggered when a new user input is passed in to
   // web_contents. Contains a TimeDelta from navigation start.
   virtual void OnUserInput(const blink::WebInputEvent& event) {}
 
   // The following methods are invoked at most once, when the timing for the
   // associated event first becomes available.
   virtual void OnDomContentLoadedEventStart(
@@ skipping 18 matching lines @@
 
   // Observer method to be invoked when there is a change in PageLoadMetadata's
   // behavior_flags.
   virtual void OnLoadingBehaviorObserved(
       const page_load_metrics::PageLoadExtraInfo& extra_info) {}
 };
 
 }  // namespace page_load_metrics
 
 #endif  // COMPONENTS_PAGE_LOAD_METRICS_BROWSER_PAGE_LOAD_METRICS_OBSERVER_H_