Add PLMObserver::FlushMetricsOnAppEnterBackground.

chrome/browser/page_load_metrics/page_load_metrics_observer.h

Index: chrome/browser/page_load_metrics/page_load_metrics_observer.h
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_observer.h b/chrome/browser/page_load_metrics/page_load_metrics_observer.h
index 9262a358aa96cea00330c86a143f83aa89708c66..7b4189a225339ed985dae5c64256c60da4b52e76 100644
--- a/chrome/browser/page_load_metrics/page_load_metrics_observer.h
+++ b/chrome/browser/page_load_metrics/page_load_metrics_observer.h
@@ -128,6 +128,17 @@ struct PageLoadExtraInfo {
 // owned by the PageLoadTracker tracking a page load.
 class PageLoadMetricsObserver {
  public:
+  // ObservePolicy is used as a return value on some PageLoadMetricsObserver
+  // callbacks to indicate whether the observer would like to continue observing
+  // metric callbacks. Observers that wish to continue observing metric
+  // callbacks should return CONTINUE_OBSERVING; observers that wish to stop
+  // observing callbacks should return STOP_OBSERVING. Observers that return
+  // STOP_OBSERVING may be deleted.
+  enum ObservePolicy {

[Charlie Harrison, 2016/09/26 20:56:39]

Cool. This is a nice generalization of what I had :)

[Bryan McQuade, 2016/09/27 13:17:20]

Thanks for putting your change together! Glad we can generalize to support
multiple cases.

+    CONTINUE_OBSERVING,
+    STOP_OBSERVING,
+  };
+
   virtual ~PageLoadMetricsObserver() {}
 
   // The page load started, with the given navigation handle. Note that OnStart
@@ -205,11 +216,26 @@ class PageLoadMetricsObserver {
   virtual void OnParseStop(const PageLoadTiming& timing,
                            const PageLoadExtraInfo& extra_info) {}
 
-  // Observer method to be invoked when there is a change in PageLoadMetadata's
-  // behavior_flags.
+  // Invoked when there is a change in PageLoadMetadata's behavior_flags.
   virtual void OnLoadingBehaviorObserved(
       const page_load_metrics::PageLoadExtraInfo& extra_info) {}
 
+  // Invoked when the UMA metrics subsystem is persisting metrics as the
+  // application goes into the background, on platforms where metrics are

[Charlie Harrison, 2016/09/27 12:50:09]

s/are/should be?

[Bryan McQuade, 2016/09/27 13:17:20]

Updated the comment to be a clearer, thanks!

+  // persisted as part of backgrounding (Android). Implementers should persist

[Charlie Harrison, 2016/09/27 12:50:09]

Note: there have been experiments where background tabs have been killed on
desktop too... Do you know if that experiment failed? I'm mostly now wondering
if this callback is redundant if that work ends up shipping.

[Bryan McQuade, 2016/09/27 13:17:20]

Ah, I didn't know about this. I think we'd want to invoke this callback any time
the browser process may be killed w/o further notification. It sounds like tab
killing on desktop would not kill browser process, so we wouldn't want to invoke
this callback there, is my impression. I agree we should watch for changes and
if there are other platforms where browser process kills can happen, we should
try to invoke this callback on those platforms as well.

+  // any metrics that have been buffered in memory in this callback, as the
+  // application may be killed at any time after this method is invoked without
+  // further notification. Note that this may be called both for provisional
+  // loads as well as committed loads. Implementations that only want to track
+  // committed loads should check extra_info.time_to_commit to determine if the
+  // load had committed. The default implementation returns
+  // CONTINUE_OBSERVING. If the implementation returns CONTINUE_OBSERVING, this
+  // method may be called multiple times per observer, once for each time that
+  // the application enters the backround.
+  virtual ObservePolicy FlushMetricsOnAppEnterBackground(
+      const PageLoadTiming& timing,
+      const PageLoadExtraInfo& extra_info);
+
   // One of OnComplete or OnFailedProvisionalLoad is invoked for tracked page
   // loads, immediately before the observer is deleted. These callbacks will not
   // be invoked for page loads that did not meet the criteria for being tracked