Various cleanups for page_load_metrics_browsertest

chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc

 // 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.
 
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/test/histogram_tester.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
 #include "chrome/browser/page_load_metrics/metrics_web_contents_observer.h"
@@ skipping 21 matching lines @@
 #include "content/public/test/download_test_observer.h"
 #include "net/http/failing_http_transaction_factory.h"
 #include "net/http/http_cache.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/url_request/url_request_failed_job.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
 
 namespace {
 
+void FailAllNetworkTransactions(net::URLRequestContextGetter* getter) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  net::HttpCache* cache(
+      getter->GetURLRequestContext()->http_transaction_factory()->GetCache());
+  DCHECK(cache);
+  std::unique_ptr<net::FailingHttpTransactionFactory> factory(
+      new net::FailingHttpTransactionFactory(cache->GetSession(),

[jkarlin, 2017/05/10 18:45:05]

base::MakeUnique while we're here :)

[Bryan McQuade, 2017/05/10 19:14:33]

Done

+                                             net::ERR_FAILED));

[jkarlin, 2017/05/10 18:45:04]

Include the net error header

[Bryan McQuade, 2017/05/10 19:14:33]

Done

+  // Throw away old version; since this is a browser test, there is no
+  // need to restore the old state.
+  cache->SetHttpNetworkTransactionFactoryForTesting(std::move(factory));
+}
+
 // Waits until specified timing and metadata expectations are satisfied.
 class PageLoadMetricsWaiter
     : public page_load_metrics::MetricsWebContentsObserver::TestingObserver {
  public:
   // A bitvector to express which timing fields to match on.
-  enum ExpectedTimingFields {
-    FIRST_PAINT = 1 << 0,
-    FIRST_CONTENTFUL_PAINT = 1 << 1,
-    STYLE_UPDATE_BEFORE_FCP = 1 << 2
+  enum class ExpectedTimingField : int {
+    FIRST_LAYOUT = 1 << 0,
+    FIRST_PAINT = 1 << 1,
+    FIRST_CONTENTFUL_PAINT = 1 << 2,
+    FIRST_MEANINGFUL_PAINT = 1 << 3,
+    STYLE_UPDATE_BEFORE_FCP = 1 << 4,
+    DOCUMENT_WRITE_BLOCK_RELOAD = 1 << 5,
+    LOAD_EVENT = 1 << 6
   };
 
   explicit PageLoadMetricsWaiter(content::WebContents* web_contents)
       : TestingObserver(web_contents) {}
 
   ~PageLoadMetricsWaiter() override { DCHECK_EQ(nullptr, run_loop_.get()); }
 
   // Add the given expectation to match on.
-  void AddExpectation(ExpectedTimingFields fields) {
-    matching_fields_ |= fields;
+  void AddMainFrameExpectation(ExpectedTimingField field) {
+    main_frame_expected_fields_.set(field);
   }
-
-  // Instructs observer to also watch for |count|
-  // WebLoadingBehaviorDocumentWriteBlockReload events.
-  void ExpectDocumentWriteBlockReload(int count) {
-    match_document_write_block_reload_ = count;
+  void AddSubFrameExpectation(ExpectedTimingField field) {
+    child_frame_expected_fields_.set(field);
   }
 
   // Waits for a TimingUpdated IPC that matches the fields set by
-  // |AddExpectation|.  All matching fields must be set in a TimingUpdated
-  // IPC for it to end this wait.
+  // |AddMainFrameExpectation| and |AddSubFrameExpectation|. All matching fields
+  // must be set in a TimingUpdated IPC for it to end this wait.
   void Wait() {
-    if (expectations_satisfied_)
+    if (expectations_satisfied())
       return;
 
     run_loop_.reset(new base::RunLoop());
     run_loop_->Run();
     run_loop_.reset(nullptr);
 
-    EXPECT_TRUE(expectations_satisfied_);
+    EXPECT_TRUE(expectations_satisfied());
   }
 
  private:
+  // Manages a bitset of ExpectedTimingFields.
+  class ExpectedTimingFieldBitSet {
+   public:
+    ExpectedTimingFieldBitSet() {}
+
+    // Returns whether this bitset has all bits unset.
+    bool empty() const { return bitmask_ == 0; }

[jkarlin, 2017/05/10 18:45:04]

Empty

[Bryan McQuade, 2017/05/10 19:14:32]

Done. I interpreted the style guide as indicating this is ok style, but perhaps
these aren't quite accessors / mutators:
https://google.github.io/styleguide/cppguide.html#Function_Names "accessors and
mutators may be named like variables."

[jkarlin, 2017/05/11 14:04:05]

I mostly agree with you that they could be lowercase, but clear and empty are
slightly more complex than getting and setting. Grep wars shows 3x as many
Empty() functions as empty().

+
+    // Sets the bit for the given |field|.
+    void set(ExpectedTimingField field) { bitmask_ |= static_cast<int>(field); }

[jkarlin, 2017/05/10 18:45:05]

Set

[Bryan McQuade, 2017/05/10 19:14:32]

Done

+
+    // Clears all bits set in the |other| bitset.
+    void clear(const ExpectedTimingFieldBitSet& other) {

[jkarlin, 2017/05/10 18:45:04]

Typically I think of clear as clearing everything. Perhaps, ClearMatchingBits?

[Bryan McQuade, 2017/05/10 19:14:33]

changed to ClearMatching.

+      bitmask_ &= ~other.bitmask_;
+    }
+
+   private:
+    int bitmask_ = 0;
+  };
+
+  static ExpectedTimingFieldBitSet GetMatchedBits(
+      const page_load_metrics::PageLoadTiming& timing,
+      const page_load_metrics::PageLoadMetadata& metadata) {
+    ExpectedTimingFieldBitSet matched_bits;
+    if (timing.document_timing.first_layout)
+      matched_bits.set(ExpectedTimingField::FIRST_LAYOUT);
+    if (timing.document_timing.load_event_start)
+      matched_bits.set(ExpectedTimingField::LOAD_EVENT);
+    if (timing.paint_timing.first_paint)
+      matched_bits.set(ExpectedTimingField::FIRST_PAINT);
+    if (timing.paint_timing.first_contentful_paint)
+      matched_bits.set(ExpectedTimingField::FIRST_CONTENTFUL_PAINT);
+    if (timing.paint_timing.first_meaningful_paint)
+      matched_bits.set(ExpectedTimingField::FIRST_MEANINGFUL_PAINT);
+    if (timing.style_sheet_timing.update_style_duration_before_fcp)
+      matched_bits.set(ExpectedTimingField::STYLE_UPDATE_BEFORE_FCP);
+    if (metadata.behavior_flags &
+        blink::WebLoadingBehaviorFlag::
+            kWebLoadingBehaviorDocumentWriteBlockReload)
+      matched_bits.set(ExpectedTimingField::DOCUMENT_WRITE_BLOCK_RELOAD);
+
+    return matched_bits;
+  }
+
   void OnTimingUpdated(
+      bool is_main_frame,
       const page_load_metrics::PageLoadTiming& timing,
       const page_load_metrics::PageLoadMetadata& metadata) override {
-    if (match_document_write_block_reload_ > 0 &&
-        metadata.behavior_flags &
-            blink::WebLoadingBehaviorFlag::
-                kWebLoadingBehaviorDocumentWriteBlockReload) {
-      --match_document_write_block_reload_;
-    }
+    if (expectations_satisfied())
+      return;
 
-    if (match_document_write_block_reload_ > 0) {
-      return;
-    }
+    ExpectedTimingFieldBitSet matched_bits = GetMatchedBits(timing, metadata);
+    if (is_main_frame)
+      main_frame_expected_fields_.clear(matched_bits);
+    else
+      child_frame_expected_fields_.clear(matched_bits);
 
-    if ((!(matching_fields_ & FIRST_PAINT) ||
-         timing.paint_timing.first_paint) &&
-        (!(matching_fields_ & FIRST_CONTENTFUL_PAINT) ||
-         timing.paint_timing.first_contentful_paint) &&
-        (!(matching_fields_ & STYLE_UPDATE_BEFORE_FCP) ||
-         timing.style_sheet_timing.update_style_duration_before_fcp)) {
-      expectations_satisfied_ = true;
-      if (run_loop_)
-        run_loop_->Quit();
-    }
+    if (expectations_satisfied() && run_loop_)
+      run_loop_->Quit();
+  }
+
+  bool expectations_satisfied() const {
+    return child_frame_expected_fields_.empty() &&
+           main_frame_expected_fields_.empty();
   }
 
   std::unique_ptr<base::RunLoop> run_loop_;
-  int matching_fields_ = 0;  // A bitvector composed from ExpectedTimingFields.
-  bool expectations_satisfied_ = false;
-  int match_document_write_block_reload_ = 0;
+
+  ExpectedTimingFieldBitSet child_frame_expected_fields_;
+  ExpectedTimingFieldBitSet main_frame_expected_fields_;
 };
 
+using ExpectedTimingField = PageLoadMetricsWaiter::ExpectedTimingField;
+
 }  // namespace
 
 class PageLoadMetricsBrowserTest : public InProcessBrowserTest {
  public:
   PageLoadMetricsBrowserTest() {}
   ~PageLoadMetricsBrowserTest() override {}
 
  protected:
+  // Force navigation to a new page, so the currently tracked page load runs its
+  // OnComplete callback. You should prefer to use PageLoadMetricsWaiter, and
+  // only use NavigateToUntrackedUrl for cases where the waiter isn't
+  // sufficient.
   void NavigateToUntrackedUrl() {
     ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
   }
 
   bool NoPageLoadMetricsRecorded() {
     // Determine whether any 'public' page load metrics are recorded. We exclude
     // 'internal' metrics as these may be recorded for debugging purposes.
     size_t total_pageload_histograms =
         histogram_tester_.GetTotalCountsForPrefix("PageLoad.").size();
     size_t total_internal_histograms =
         histogram_tester_.GetTotalCountsForPrefix("PageLoad.Internal.").size();
     DCHECK_GE(total_pageload_histograms, total_internal_histograms);
     return total_pageload_histograms - total_internal_histograms == 0;
   }
 
   std::unique_ptr<PageLoadMetricsWaiter> CreatePageLoadMetricsWaiter() {
     content::WebContents* web_contents =
         browser()->tab_strip_model()->GetActiveWebContents();
     return base::MakeUnique<PageLoadMetricsWaiter>(web_contents);
   }
 
   base::HistogramTester histogram_tester_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(PageLoadMetricsBrowserTest);
 };
 
-void FailAllNetworkTransactions(net::URLRequestContextGetter* getter) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-  net::HttpCache* cache(
-      getter->GetURLRequestContext()->http_transaction_factory()->GetCache());
-  DCHECK(cache);
-  std::unique_ptr<net::FailingHttpTransactionFactory> factory(
-      new net::FailingHttpTransactionFactory(cache->GetSession(),
-                                             net::ERR_FAILED));
-  // Throw away old version; since this is a browser test, there is no
-  // need to restore the old state.
-  cache->SetHttpNetworkTransactionFactoryForTesting(std::move(factory));
-}
-
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, NoNavigation) {
   ASSERT_TRUE(embedded_test_server()->Start());
   EXPECT_TRUE(NoPageLoadMetricsRecorded());
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, NewPage) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();

[jkarlin, 2017/05/10 18:45:05]

optional: auto seems reasonable here

[Bryan McQuade, 2017/05/10 19:14:32]

done

+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_PAINT);

[jkarlin, 2017/05/10 18:45:04]

Seems like it would be useful to have a constructor like so:

auto waiter = CreatePageLoadMetricsWaiter(ExpectedTimingField::FIRST_PAINT);
then you could construct a waiter with all of the flags you want or'd together.

[Bryan McQuade, 2017/05/10 19:14:32]

I'm disinclined to do this since we have main and child frames & it's not
totally clear at the callsite whether the constructor is for main or child. I
think main is a logical default but overall I think the loss of clarity doesn't
make up for saving one line of code.

[jkarlin, 2017/05/11 14:04:05]

Fair point. The constructor could take two values, but then it wouldn't be clear
which is which.

   ui_test_utils::NavigateToURL(browser(),
                                embedded_test_server()->GetURL("/title1.html"));
-  NavigateToUntrackedUrl();
+  waiter->Wait();
 
   histogram_tester_.ExpectTotalCount(internal::kHistogramDomContentLoaded, 1);
   histogram_tester_.ExpectTotalCount(internal::kHistogramLoad, 1);
   histogram_tester_.ExpectTotalCount(internal::kHistogramFirstLayout, 1);
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstPaint, 1);
   histogram_tester_.ExpectTotalCount(internal::kHistogramParseDuration, 1);
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramParseBlockedOnScriptLoad, 1);
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramParseBlockedOnScriptExecution, 1);
+
+  // Force navigation to another page, which should force logging of histograms
+  // persisted at the end of the page load lifetime.
+  NavigateToUntrackedUrl();
   histogram_tester_.ExpectTotalCount(internal::kHistogramTotalBytes, 1);
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramPageTimingForegroundDuration, 1);
 
   // Verify that NoPageLoadMetricsRecorded returns false when PageLoad metrics
   // have been recorded.
   EXPECT_FALSE(NoPageLoadMetricsRecorded());
 }
 
+IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, NoPaintForEmptyDocument) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();

[jkarlin, 2017/05/10 18:45:05]

What if you could also create a set of unexpected matching bits that the test
would fail on if they were detected?

[Bryan McQuade, 2017/05/10 19:14:33]

Sure, I added an API for this.

+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_LAYOUT);
+  waiter->AddMainFrameExpectation(ExpectedTimingField::LOAD_EVENT);
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/empty.html"));
+  waiter->Wait();
+
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstLayout, 1);
+  histogram_tester_.ExpectTotalCount(internal::kHistogramLoad, 1);
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstPaint, 0);
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstContentfulPaint,
+                                     0);
+}
+
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, SameDocumentNavigation) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_LAYOUT);
+  waiter->AddMainFrameExpectation(ExpectedTimingField::LOAD_EVENT);
   ui_test_utils::NavigateToURL(browser(),
                                embedded_test_server()->GetURL("/title1.html"));
+  waiter->Wait();
+
+  histogram_tester_.ExpectTotalCount(internal::kHistogramDomContentLoaded, 1);
+  histogram_tester_.ExpectTotalCount(internal::kHistogramLoad, 1);
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstLayout, 1);
+
+  // Perform a same-document navigation. No additional metrics should be logged.
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL("/title1.html#hash"));
   NavigateToUntrackedUrl();
 
   histogram_tester_.ExpectTotalCount(internal::kHistogramDomContentLoaded, 1);
   histogram_tester_.ExpectTotalCount(internal::kHistogramLoad, 1);
   histogram_tester_.ExpectTotalCount(internal::kHistogramFirstLayout, 1);
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, SameUrlNavigation) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_LAYOUT);
+  waiter->AddMainFrameExpectation(ExpectedTimingField::LOAD_EVENT);
   ui_test_utils::NavigateToURL(browser(),
                                embedded_test_server()->GetURL("/title1.html"));
+  waiter->Wait();
+
+  histogram_tester_.ExpectTotalCount(internal::kHistogramDomContentLoaded, 1);
+  histogram_tester_.ExpectTotalCount(internal::kHistogramLoad, 1);
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstLayout, 1);
+
+  waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_LAYOUT);
+  waiter->AddMainFrameExpectation(ExpectedTimingField::LOAD_EVENT);
   ui_test_utils::NavigateToURL(browser(),
                                embedded_test_server()->GetURL("/title1.html"));
-  NavigateToUntrackedUrl();
+  waiter->Wait();
 
   // We expect one histogram sample for each navigation to title1.html.
   histogram_tester_.ExpectTotalCount(internal::kHistogramDomContentLoaded, 2);
   histogram_tester_.ExpectTotalCount(internal::kHistogramLoad, 2);
   histogram_tester_.ExpectTotalCount(internal::kHistogramFirstLayout, 2);
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, NonHtmlMainResource) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
@@ skipping 64 matching lines @@
       browser(), embedded_test_server()->GetURL("/download-test3.gif"));
   downloads_observer.WaitForFinished();
 
   NavigateToUntrackedUrl();
   EXPECT_TRUE(NoPageLoadMetricsRecorded());
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, PreloadDocumentWrite) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
-  std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
-      CreatePageLoadMetricsWaiter();
-  fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_CONTENTFUL_PAINT);
 
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(
                      "/page_load_metrics/document_write_external_script.html"));
-  fcp_waiter->Wait();
+  waiter->Wait();
 
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramDocWriteParseStartToFirstContentfulPaint, 1);
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, NoPreloadDocumentWrite) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_CONTENTFUL_PAINT);
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(
                      "/page_load_metrics/document_write_no_script.html"));
-  NavigateToUntrackedUrl();
+  waiter->Wait();
 
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstContentfulPaint,
+                                     1);
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramDocWriteParseStartToFirstContentfulPaint, 0);
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, NoDocumentWrite) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
-  std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
-      CreatePageLoadMetricsWaiter();
-  fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_CONTENTFUL_PAINT);
 
   ui_test_utils::NavigateToURL(browser(),
                                embedded_test_server()->GetURL("/title1.html"));
-  fcp_waiter->Wait();
+  waiter->Wait();
 
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstContentfulPaint,
+                                     1);
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramDocWriteParseStartToFirstContentfulPaint, 0);
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramDocWriteBlockParseStartToFirstContentfulPaint, 0);
   histogram_tester_.ExpectTotalCount(internal::kHistogramDocWriteBlockCount, 0);
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, DocumentWriteBlock) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
-  std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
-      CreatePageLoadMetricsWaiter();
-  fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_CONTENTFUL_PAINT);
 
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(
                      "/page_load_metrics/document_write_script_block.html"));
-  fcp_waiter->Wait();
+  waiter->Wait();
 
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramDocWriteBlockParseStartToFirstContentfulPaint, 1);
   histogram_tester_.ExpectTotalCount(internal::kHistogramDocWriteBlockCount, 1);
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, DocumentWriteReload) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
-  std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
-      CreatePageLoadMetricsWaiter();
-  fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
-  std::unique_ptr<PageLoadMetricsWaiter> reload_waiter =
-      CreatePageLoadMetricsWaiter();
-  reload_waiter->ExpectDocumentWriteBlockReload(2);
-
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_CONTENTFUL_PAINT);
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(
                      "/page_load_metrics/document_write_script_block.html"));
+  waiter->Wait();
+
+  histogram_tester_.ExpectTotalCount(
+      internal::kHistogramDocWriteBlockParseStartToFirstContentfulPaint, 1);
+  histogram_tester_.ExpectTotalCount(internal::kHistogramDocWriteBlockCount, 1);
 
   // Reload should not log the histogram as the script is not blocked.
+  std::unique_ptr<PageLoadMetricsWaiter> reload_waiter =
+      CreatePageLoadMetricsWaiter();
+  reload_waiter->AddMainFrameExpectation(
+      ExpectedTimingField::DOCUMENT_WRITE_BLOCK_RELOAD);
+  reload_waiter->AddMainFrameExpectation(
+      ExpectedTimingField::FIRST_CONTENTFUL_PAINT);
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(
                      "/page_load_metrics/document_write_script_block.html"));
+  reload_waiter->Wait();
 
+  histogram_tester_.ExpectTotalCount(
+      internal::kHistogramDocWriteBlockReloadCount, 1);
+
+  reload_waiter = CreatePageLoadMetricsWaiter();
+  reload_waiter->AddMainFrameExpectation(
+      ExpectedTimingField::DOCUMENT_WRITE_BLOCK_RELOAD);
+  reload_waiter->AddMainFrameExpectation(
+      ExpectedTimingField::FIRST_CONTENTFUL_PAINT);
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(
                      "/page_load_metrics/document_write_script_block.html"));
-
-  histogram_tester_.ExpectTotalCount(
-      internal::kHistogramDocWriteBlockParseStartToFirstContentfulPaint, 1);
-
-  fcp_waiter->Wait();
   reload_waiter->Wait();
 
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramDocWriteBlockParseStartToFirstContentfulPaint, 1);
 
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramDocWriteBlockReloadCount, 2);
   histogram_tester_.ExpectTotalCount(internal::kHistogramDocWriteBlockCount, 1);
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, DocumentWriteAsync) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_CONTENTFUL_PAINT);
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(
-                     "/page_load_metrics/document_write_script_async.html"));
-  NavigateToUntrackedUrl();
+                     "/page_load_metrics/document_write_async_script.html"));
+  waiter->Wait();
 
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstContentfulPaint,
+                                     1);
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramDocWriteBlockParseStartToFirstContentfulPaint, 0);
   histogram_tester_.ExpectTotalCount(internal::kHistogramDocWriteBlockCount, 0);
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, DocumentWriteSameDomain) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_CONTENTFUL_PAINT);
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(
                      "/page_load_metrics/document_write_external_script.html"));
-  NavigateToUntrackedUrl();
+  waiter->Wait();
 
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstContentfulPaint,
+                                     1);
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramDocWriteBlockParseStartToFirstContentfulPaint, 0);
   histogram_tester_.ExpectTotalCount(internal::kHistogramDocWriteBlockCount, 0);
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, NoDocumentWriteScript) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_CONTENTFUL_PAINT);
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(
                      "/page_load_metrics/document_write_no_script.html"));
-  NavigateToUntrackedUrl();
+  waiter->Wait();
 
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstContentfulPaint,
+                                     1);
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramDocWriteBlockParseStartToFirstContentfulPaint, 0);
   histogram_tester_.ExpectTotalCount(internal::kHistogramDocWriteBlockCount, 0);
 }
 
 // TODO(crbug.com/712935): Flaky on Linux dbg.
 #if defined(OS_LINUX) && !defined(NDEBUG)
 #define MAYBE_BadXhtml DISABLED_BadXhtml
 #else
 #define MAYBE_BadXhtml BadXhtml
 #endif
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, MAYBE_BadXhtml) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
-  std::unique_ptr<PageLoadMetricsWaiter> timing_waiter =
-      CreatePageLoadMetricsWaiter();
-  timing_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_PAINT);
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_PAINT);
 
   // When an XHTML page contains invalid XML, it causes a paint of the error
   // message without a layout. Page load metrics currently treats this as an
   // error. Eventually, we'll fix this by special casing the handling of
   // documents with non-well-formed XML on the blink side. See crbug.com/627607
   // for more.
   ui_test_utils::NavigateToURL(
       browser(),
       embedded_test_server()->GetURL("/page_load_metrics/badxml.xhtml"));
 
-  timing_waiter->Wait();
+  waiter->Wait();
 
   histogram_tester_.ExpectTotalCount(internal::kHistogramFirstLayout, 0);
   histogram_tester_.ExpectTotalCount(internal::kHistogramFirstPaint, 0);
   histogram_tester_.ExpectTotalCount(page_load_metrics::internal::kErrorEvents,
                                      1);
   histogram_tester_.ExpectBucketCount(
       page_load_metrics::internal::kErrorEvents,
       page_load_metrics::ERR_BAD_TIMING_IPC_INVALID_TIMING, 1);
 
   histogram_tester_.ExpectTotalCount(
@@ skipping 13 matching lines @@
   chrome::NavigateParams params(browser(), url, ui::PAGE_TRANSITION_LINK);
   content::TestNavigationManager manager(
       browser()->tab_strip_model()->GetActiveWebContents(), url);
 
   chrome::Navigate(&params);
   EXPECT_TRUE(manager.WaitForRequestStart());
 
   GURL url2(embedded_test_server()->GetURL("/title2.html"));
   chrome::NavigateParams params2(browser(), url2,
                                  ui::PAGE_TRANSITION_FROM_ADDRESS_BAR);
-  content::TestNavigationManager manager2(
-      browser()->tab_strip_model()->GetActiveWebContents(), url2);
+
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::LOAD_EVENT);
   chrome::Navigate(&params2);
+  waiter->Wait();
 
-  manager2.WaitForNavigationFinished();
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramAbortNewNavigationBeforeCommit, 1);
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, AbortReload) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
   GURL url(embedded_test_server()->GetURL("/title1.html"));
   chrome::NavigateParams params(browser(), url, ui::PAGE_TRANSITION_LINK);
   content::TestNavigationManager manager(
       browser()->tab_strip_model()->GetActiveWebContents(), url);
 
   chrome::Navigate(&params);
   EXPECT_TRUE(manager.WaitForRequestStart());
 
   chrome::NavigateParams params2(browser(), url, ui::PAGE_TRANSITION_RELOAD);
-  content::TestNavigationManager manager2(
-      browser()->tab_strip_model()->GetActiveWebContents(), url);
+
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::LOAD_EVENT);
   chrome::Navigate(&params2);
+  waiter->Wait();
 
-  manager2.WaitForNavigationFinished();
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramAbortReloadBeforeCommit, 1);
 }
 
 // TODO(crbug.com/675061): Flaky on Win7 dbg.
 #if defined(OS_WIN)
 #define MAYBE_AbortClose DISABLED_AbortClose
 #else
 #define MAYBE_AbortClose AbortClose
 #endif
@@ skipping 31 matching lines @@
   chrome::NavigateParams params2(browser(), url2, ui::PAGE_TRANSITION_TYPED);
   content::TestNavigationManager manager2(
       browser()->tab_strip_model()->GetActiveWebContents(), url2);
   chrome::Navigate(&params2);
 
   EXPECT_TRUE(manager2.WaitForRequestStart());
   manager.WaitForNavigationFinished();
 
   GURL url3(embedded_test_server()->GetURL("/title3.html"));
   chrome::NavigateParams params3(browser(), url3, ui::PAGE_TRANSITION_TYPED);
-  content::TestNavigationManager manager3(
-      browser()->tab_strip_model()->GetActiveWebContents(), url3);
+
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::LOAD_EVENT);
   chrome::Navigate(&params3);
+  waiter->Wait();
 
-  EXPECT_TRUE(manager3.WaitForRequestStart());
   manager2.WaitForNavigationFinished();
 
-  manager3.WaitForNavigationFinished();
-
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramAbortNewNavigationBeforeCommit, 2);
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
                        NoAbortMetricsOnClientRedirect) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
   GURL first_url(embedded_test_server()->GetURL("/title1.html"));
   ui_test_utils::NavigateToURL(browser(), first_url);
 
   GURL second_url(embedded_test_server()->GetURL("/title2.html"));
   chrome::NavigateParams params(browser(), second_url,
                                 ui::PAGE_TRANSITION_LINK);
   content::TestNavigationManager manager(
       browser()->tab_strip_model()->GetActiveWebContents(), second_url);
   chrome::Navigate(&params);
   EXPECT_TRUE(manager.WaitForRequestStart());
 
   {
+    std::unique_ptr<PageLoadMetricsWaiter> waiter =
+        CreatePageLoadMetricsWaiter();
+    waiter->AddMainFrameExpectation(ExpectedTimingField::LOAD_EVENT);
     content::TestNavigationManager reload_manager(
         browser()->tab_strip_model()->GetActiveWebContents(), first_url);
     EXPECT_TRUE(content::ExecuteScript(
         browser()->tab_strip_model()->GetActiveWebContents(),
         "window.location.reload();"));
+    waiter->Wait();
   }
 
   manager.WaitForNavigationFinished();
 
   EXPECT_TRUE(histogram_tester_
                   .GetTotalCountsForPrefix("PageLoad.Experimental.AbortTiming.")
                   .empty());
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
                        FirstMeaningfulPaintRecorded) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_MEANINGFUL_PAINT);
   ui_test_utils::NavigateToURL(browser(),
                                embedded_test_server()->GetURL("/title1.html"));
+  waiter->Wait();
 
+#if 0
   // Wait until the renderer finishes observing layouts.
   const int kNetworkIdleTime = 3000;
   const int kMargin = 500;
   const std::string javascript = base::StringPrintf(
       "setTimeout(() => window.domAutomationController.send(true), %d)",
       kNetworkIdleTime + kMargin);
   bool result;
   EXPECT_TRUE(content::ExecuteScriptAndExtractBool(
       browser()->tab_strip_model()->GetActiveWebContents(),
       javascript, &result));
   EXPECT_TRUE(result);
+#endif
 
-  NavigateToUntrackedUrl();
   histogram_tester_.ExpectUniqueSample(
       internal::kHistogramFirstMeaningfulPaintStatus,
       internal::FIRST_MEANINGFUL_PAINT_RECORDED, 1);
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramFirstMeaningfulPaint, 1);
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramParseStartToFirstMeaningfulPaint, 1);
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
                        FirstMeaningfulPaintNotRecorded) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
-  std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
-      CreatePageLoadMetricsWaiter();
-  fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_CONTENTFUL_PAINT);
 
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(
                      "/page_load_metrics/page_with_active_connections.html"));
-  fcp_waiter->Wait();
+  waiter->Wait();
 
   // Navigate away before a FMP is reported.
   NavigateToUntrackedUrl();
 
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstContentfulPaint,
+                                     1);
   histogram_tester_.ExpectUniqueSample(
       internal::kHistogramFirstMeaningfulPaintStatus,
       internal::FIRST_MEANINGFUL_PAINT_DID_NOT_REACH_NETWORK_STABLE, 1);
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramFirstMeaningfulPaint, 0);
   histogram_tester_.ExpectTotalCount(
       internal::kHistogramParseStartToFirstMeaningfulPaint, 0);
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
                        NoStatePrefetchObserverCacheable) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
-  std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
-      CreatePageLoadMetricsWaiter();
-  fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_CONTENTFUL_PAINT);
 
   ui_test_utils::NavigateToURL(browser(),
                                embedded_test_server()->GetURL("/title1.html"));
 
-  fcp_waiter->Wait();
+  waiter->Wait();
 
   histogram_tester_.ExpectTotalCount(
       "Prerender.none_PrefetchTTFCP.Reference.NoStore.Visible", 0);
   histogram_tester_.ExpectTotalCount(
       "Prerender.none_PrefetchTTFCP.Reference.Cacheable.Visible", 1);
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
                        NoStatePrefetchObserverNoStore) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
-  std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
-      CreatePageLoadMetricsWaiter();
-  fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::FIRST_CONTENTFUL_PAINT);
 
   ui_test_utils::NavigateToURL(browser(),
                                embedded_test_server()->GetURL("/nostore.html"));
 
-  fcp_waiter->Wait();
+  waiter->Wait();
 
   histogram_tester_.ExpectTotalCount(
       "Prerender.none_PrefetchTTFCP.Reference.NoStore.Visible", 1);
   histogram_tester_.ExpectTotalCount(
       "Prerender.none_PrefetchTTFCP.Reference.Cacheable.Visible", 0);
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, CSSTiming) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
-  std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
-      CreatePageLoadMetricsWaiter();
-  fcp_waiter->AddExpectation(PageLoadMetricsWaiter::STYLE_UPDATE_BEFORE_FCP);
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::STYLE_UPDATE_BEFORE_FCP);
 
   // Careful: Blink code clamps timestamps to 5us, so any CSS parsing we do here
   // must take >> 5us, otherwise we'll log 0 for the value and it will remain
   // unset here.
   ui_test_utils::NavigateToURL(
       browser(),
       embedded_test_server()->GetURL("/page_load_metrics/page_with_css.html"));
-  NavigateToUntrackedUrl();
-  fcp_waiter->Wait();
+  waiter->Wait();
 
   histogram_tester_.ExpectTotalCount(internal::kHistogramFirstContentfulPaint,
                                      1);
   histogram_tester_.ExpectTotalCount(
       "PageLoad.CSSTiming.Parse.BeforeFirstContentfulPaint", 1);
   histogram_tester_.ExpectTotalCount(
       "PageLoad.CSSTiming.Update.BeforeFirstContentfulPaint", 1);
   histogram_tester_.ExpectTotalCount(
       "PageLoad.CSSTiming.ParseAndUpdate.BeforeFirstContentfulPaint", 1);
 }
 
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, PayloadSize) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
+  std::unique_ptr<PageLoadMetricsWaiter> waiter = CreatePageLoadMetricsWaiter();
+  waiter->AddMainFrameExpectation(ExpectedTimingField::LOAD_EVENT);
   ui_test_utils::NavigateToURL(browser(), embedded_test_server()->GetURL(
                                               "/page_load_metrics/large.html"));
+  waiter->Wait();
+
+  // Payload histograms are only logged when a page load terminates, so force
+  // navigation to another page.
   NavigateToUntrackedUrl();
 
   histogram_tester_.ExpectTotalCount(internal::kHistogramTotalBytes, 1);
 
   // Verify that there is a single sample recorded in the 10kB bucket (the size
   // of the main HTML response).
   histogram_tester_.ExpectBucketCount(internal::kHistogramTotalBytes, 10, 1);
 }