Make PageLoadMetricsWaiter more resilient

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 197a9884df341e6bd7de16e171152d58f6d6aa3d..0c35c5922a14155955e8f3d874faa49622f66d7a 100644
--- a/chrome/browser/page_load_metrics/page_load_metrics_observer.h
+++ b/chrome/browser/page_load_metrics/page_load_metrics_observer.h
@@ -336,13 +336,14 @@ class PageLoadMetricsObserver {
   // tracked at the time a navigation commits will not receive any of the
   // callbacks below.
 
-  // OnTimingUpdate is triggered when an updated PageLoadTiming is
-  // available. This method may be called multiple times over the course of the
-  // 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 mojom::PageLoadTiming& timing,
+  // OnTimingUpdate is triggered when an updated PageLoadTiming is available at
+  // the page or subframe level. This method may be called multiple times over

[Charlie Harrison, 2017/05/25 19:35:34]

Aside: It's weird to me to distinguish "page" vs "subframe" when usually we're
comparing main frame vs subframe. I think we really do want that distinction
because for "page" we aggregate, is that intuition correct?

[Bryan McQuade, 2017/05/25 19:54:06]

Correct - page is essentially main frame + aggregated data for a few page-level
fields (paint timings). I agree this isn't super clear. I added a comment to
explain.

+  // the course of the 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(bool is_subframe,

[Charlie Harrison, 2017/05/25 19:35:34]

optional: For consistency would it make sense to have two methods OnSubframe...
and OnMainFrame... like how we do it in the dispatcher client?

[Bryan McQuade, 2017/05/25 19:54:06]

I did this initially but decided that since this was really intended only for
testing, keeping the testing API thin was beneficial.

[Charlie Harrison, 2017/05/25 19:59:44]

Makes sense to me

+                              const mojom::PageLoadTiming& timing,
                               const PageLoadExtraInfo& extra_info) {}
 
   // OnUserInput is triggered when a new user input is passed in to