Report page load timing information for child frames.

chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc

Index: chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc b/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
index 1b52ea8f37126e974d13ff08f527fb609c81d5e5..62f49157cb87974d46e9927a6e5b383b09909409 100644
--- a/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
+++ b/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/command_line.h"
+#include "base/feature_list.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/test/histogram_tester.h"
@@ -28,8 +30,10 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/download_test_observer.h"
+#include "net/dns/mock_host_resolver.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"
@@ -45,9 +49,11 @@ class PageLoadMetricsWaiter
  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
+    FIRST_LAYOUT = 1 << 0,
+    FIRST_PAINT = 1 << 1,
+    FIRST_CONTENTFUL_PAINT = 1 << 2,
+    STYLE_UPDATE_BEFORE_FCP = 1 << 3,
+    DOCUMENT_WRITE_BLOCK_RELOAD = 1 << 4
   };
 
   explicit PageLoadMetricsWaiter(content::WebContents* web_contents)
@@ -56,14 +62,11 @@ class PageLoadMetricsWaiter
   ~PageLoadMetricsWaiter() override { DCHECK_EQ(nullptr, run_loop_.get()); }
 
   // Add the given expectation to match on.
-  void AddExpectation(ExpectedTimingFields fields) {
-    matching_fields_ |= fields;
+  void AddMainFrameExpectation(ExpectedTimingFields fields) {
+    main_frame_expected_fields_ |= fields;
   }
-
-  // Instructs observer to also watch for |count|
-  // WebLoadingBehaviorDocumentWriteBlockReload events.
-  void ExpectDocumentWriteBlockReload(int count) {
-    match_document_write_block_reload_ = count;
+  void AddSubFrameExpectation(ExpectedTimingFields fields) {
+    child_frame_expected_fields_ |= fields;
   }
 
   // Waits for a TimingUpdated IPC that matches the fields set by
@@ -81,26 +84,40 @@ class PageLoadMetricsWaiter
   }
 
  private:
+  static int GetMatchedBits(
+      const page_load_metrics::PageLoadTiming& timing,
+      const page_load_metrics::PageLoadMetadata& metadata) {
+    int matched_bits = 0;
+    if (timing.document_timing.first_layout)
+      matched_bits |= FIRST_LAYOUT;
+    if (timing.paint_timing.first_paint)
+      matched_bits |= FIRST_PAINT;
+    if (timing.paint_timing.first_contentful_paint)
+      matched_bits |= FIRST_CONTENTFUL_PAINT;
+    if (timing.style_sheet_timing.update_style_duration_before_fcp)
+      matched_bits |= STYLE_UPDATE_BEFORE_FCP;
+    if (metadata.behavior_flags &
+        blink::WebLoadingBehaviorFlag::
+            kWebLoadingBehaviorDocumentWriteBlockReload)
+      matched_bits |= 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 (match_document_write_block_reload_ > 0) {
+    if (expectations_satisfied_)
       return;
-    }
 
-    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)) {
+    int matched_bits = GetMatchedBits(timing, metadata);
+    if (is_main_frame)
+      main_frame_expected_fields_ &= ~matched_bits;
+    else
+      child_frame_expected_fields_ &= ~matched_bits;
+
+    if (main_frame_expected_fields_ == 0 && child_frame_expected_fields_ == 0) {
       expectations_satisfied_ = true;
       if (run_loop_)
         run_loop_->Quit();
@@ -108,11 +125,26 @@ class PageLoadMetricsWaiter
   }
 
   std::unique_ptr<base::RunLoop> run_loop_;
-  int matching_fields_ = 0;  // A bitvector composed from ExpectedTimingFields.
+
+  // Bitvectors composed from ExpectedTimingFields.
+  int child_frame_expected_fields_ = 0;
+  int main_frame_expected_fields_ = 0;
+
   bool expectations_satisfied_ = false;
-  int match_document_write_block_reload_ = 0;
 };
 
+// Due to crbug/705315, paints in child frames are associated with the main
+// frame, unless the child frame is cross-origin and Chrome is running with out
+// of process cross-origin child frames. As a result, some tests wait for
+// different behavior to be observed depending on which mode we are in.
+// TODO(crbug/705315): remove this method once the bug is addressed.
+static bool AreCrossOriginChildFramesOutOfProcess() {
+  return base::CommandLine::ForCurrentProcess()->HasSwitch(
+             switches::kSitePerProcess) ||
+         base::CommandLine::ForCurrentProcess()->HasSwitch(
+             switches::kTopDocumentIsolation);
+}
+
 }  // namespace
 
 class PageLoadMetricsBrowserTest : public InProcessBrowserTest {
@@ -125,6 +157,15 @@ class PageLoadMetricsBrowserTest : public InProcessBrowserTest {
     ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
   }
 
+  // TODO(crbug/705315): remove this method once the bug is addressed.
+  void SetUpOnMainThread() override {
+    InProcessBrowserTest::SetUpOnMainThread();
+    host_resolver()->AddRule("a.com", "127.0.0.1");
+    host_resolver()->AddRule("b.com", "127.0.0.1");
+    host_resolver()->AddRule("c.com", "127.0.0.1");
+    content::SetupCrossSiteRedirector(embedded_test_server());
+  }
+
   bool NoPageLoadMetricsRecorded() {
     // Determine whether any 'public' page load metrics are recorded. We exclude
     // 'internal' metrics as these may be recorded for debugging purposes.
@@ -169,13 +210,18 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, NoNavigation) {
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, NewPage) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
+  std::unique_ptr<PageLoadMetricsWaiter> timing_waiter =
+      CreatePageLoadMetricsWaiter();
+  timing_waiter->AddMainFrameExpectation(PageLoadMetricsWaiter::FIRST_PAINT);
   ui_test_utils::NavigateToURL(browser(),
                                embedded_test_server()->GetURL("/title1.html"));
+  timing_waiter->Wait();
   NavigateToUntrackedUrl();
 
   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);
@@ -190,6 +236,105 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, NewPage) {
   EXPECT_FALSE(NoPageLoadMetricsRecorded());
 }
 
+IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, NoPaintForEmptyDocument) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  std::unique_ptr<PageLoadMetricsWaiter> timing_waiter =
+      CreatePageLoadMetricsWaiter();
+  timing_waiter->AddMainFrameExpectation(PageLoadMetricsWaiter::FIRST_LAYOUT);
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/empty.html"));
+  timing_waiter->Wait();
+
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstLayout, 1);
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstPaint, 0);
+}
+
+IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
+                       NoPaintForEmptyDocumentInChildFrame) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  std::unique_ptr<PageLoadMetricsWaiter> timing_waiter =
+      CreatePageLoadMetricsWaiter();
+  timing_waiter->AddMainFrameExpectation(PageLoadMetricsWaiter::FIRST_LAYOUT);
+  timing_waiter->AddSubFrameExpectation(PageLoadMetricsWaiter::FIRST_LAYOUT);
+  // TODO(crbug/705315): remove the a.com domain once the bug is addressed.
+  GURL a_url(embedded_test_server()->GetURL(
+      "a.com", "/page_load_metrics/empty_iframe.html"));
+  ui_test_utils::NavigateToURL(browser(), a_url);
+  timing_waiter->Wait();
+
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstLayout, 1);
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstPaint, 0);
+}
+
+IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, PaintInChildFrame) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  std::unique_ptr<PageLoadMetricsWaiter> timing_waiter =
+      CreatePageLoadMetricsWaiter();
+  // TODO(crbug/705315): Once the bug is fixed, remove the else case and make
+  // the if case the default behavior.
+  if (AreCrossOriginChildFramesOutOfProcess()) {
+    timing_waiter->AddSubFrameExpectation(PageLoadMetricsWaiter::FIRST_PAINT);
+  } else {
+    timing_waiter->AddMainFrameExpectation(PageLoadMetricsWaiter::FIRST_PAINT);
+  }
+  // TODO(crbug/705315): remove the a.com domain once the bug is addressed.
+  GURL a_url(embedded_test_server()->GetURL("a.com",
+                                            "/page_load_metrics/iframe.html"));
+  ui_test_utils::NavigateToURL(browser(), a_url);
+  timing_waiter->Wait();
+
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstLayout, 1);
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstPaint, 1);
+}
+
+IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, PaintInMultipleChildFrames) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  std::unique_ptr<PageLoadMetricsWaiter> timing_waiter =
+      CreatePageLoadMetricsWaiter();
+  // TODO(crbug/705315): Once the bug is fixed, remove the else case and make
+  // the if case the default behavior.
+  if (AreCrossOriginChildFramesOutOfProcess()) {
+    timing_waiter->AddSubFrameExpectation(PageLoadMetricsWaiter::FIRST_PAINT);
+  } else {
+    timing_waiter->AddMainFrameExpectation(PageLoadMetricsWaiter::FIRST_PAINT);
+  }
+
+  // TODO(crbug/705315): remove the a.com domain once the bug is addressed.
+  GURL a_url(embedded_test_server()->GetURL("a.com",
+                                            "/page_load_metrics/iframes.html"));
+  ui_test_utils::NavigateToURL(browser(), a_url);
+  timing_waiter->Wait();
+
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstLayout, 1);
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstPaint, 1);
+}
+
+IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, PaintInMainAndChildFrame) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  std::unique_ptr<PageLoadMetricsWaiter> timing_waiter =
+      CreatePageLoadMetricsWaiter();
+  timing_waiter->AddMainFrameExpectation(PageLoadMetricsWaiter::FIRST_PAINT);
+  // TODO(crbug/705315): Once the bug is fixed, make the if case the default
+  // behavior.
+  if (AreCrossOriginChildFramesOutOfProcess()) {
+    timing_waiter->AddSubFrameExpectation(PageLoadMetricsWaiter::FIRST_PAINT);
+  }
+
+  // TODO(crbug/705315): remove the a.com domain once the bug is addressed.
+  GURL a_url(embedded_test_server()->GetURL(
+      "a.com", "/page_load_metrics/main_frame_with_iframe.html"));
+  ui_test_utils::NavigateToURL(browser(), a_url);
+  timing_waiter->Wait();
+
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstLayout, 1);
+  histogram_tester_.ExpectTotalCount(internal::kHistogramFirstPaint, 1);
+}
+
 IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, SameDocumentNavigation) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
@@ -298,7 +443,8 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, PreloadDocumentWrite) {
 
   std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
       CreatePageLoadMetricsWaiter();
-  fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
+  fcp_waiter->AddMainFrameExpectation(
+      PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
 
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(
@@ -326,7 +472,8 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, NoDocumentWrite) {
 
   std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
       CreatePageLoadMetricsWaiter();
-  fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
+  fcp_waiter->AddMainFrameExpectation(
+      PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
 
   ui_test_utils::NavigateToURL(browser(),
                                embedded_test_server()->GetURL("/title1.html"));
@@ -344,7 +491,8 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, DocumentWriteBlock) {
 
   std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
       CreatePageLoadMetricsWaiter();
-  fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
+  fcp_waiter->AddMainFrameExpectation(
+      PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
 
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(
@@ -361,28 +509,37 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, DocumentWriteReload) {
 
   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);
-
+  fcp_waiter->AddMainFrameExpectation(
+      PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(
                      "/page_load_metrics/document_write_script_block.html"));
+  fcp_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(
+      PageLoadMetricsWaiter::DOCUMENT_WRITE_BLOCK_RELOAD);
+  reload_waiter->AddMainFrameExpectation(
+      PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(
                      "/page_load_metrics/document_write_script_block.html"));
+  reload_waiter->Wait();
 
+  reload_waiter = CreatePageLoadMetricsWaiter();
+  reload_waiter->AddMainFrameExpectation(
+      PageLoadMetricsWaiter::DOCUMENT_WRITE_BLOCK_RELOAD);
+  reload_waiter->AddMainFrameExpectation(
+      PageLoadMetricsWaiter::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(
@@ -443,7 +600,7 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, MAYBE_BadXhtml) {
 
   std::unique_ptr<PageLoadMetricsWaiter> timing_waiter =
       CreatePageLoadMetricsWaiter();
-  timing_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_PAINT);
+  timing_waiter->AddMainFrameExpectation(PageLoadMetricsWaiter::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
@@ -643,7 +800,8 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
 
   std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
       CreatePageLoadMetricsWaiter();
-  fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
+  fcp_waiter->AddMainFrameExpectation(
+      PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
 
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL(
@@ -668,7 +826,8 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
 
   std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
       CreatePageLoadMetricsWaiter();
-  fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
+  fcp_waiter->AddMainFrameExpectation(
+      PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
 
   ui_test_utils::NavigateToURL(browser(),
                                embedded_test_server()->GetURL("/title1.html"));
@@ -687,7 +846,8 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
 
   std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
       CreatePageLoadMetricsWaiter();
-  fcp_waiter->AddExpectation(PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
+  fcp_waiter->AddMainFrameExpectation(
+      PageLoadMetricsWaiter::FIRST_CONTENTFUL_PAINT);
 
   ui_test_utils::NavigateToURL(browser(),
                                embedded_test_server()->GetURL("/nostore.html"));
@@ -705,7 +865,8 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, CSSTiming) {
 
   std::unique_ptr<PageLoadMetricsWaiter> fcp_waiter =
       CreatePageLoadMetricsWaiter();
-  fcp_waiter->AddExpectation(PageLoadMetricsWaiter::STYLE_UPDATE_BEFORE_FCP);
+  fcp_waiter->AddMainFrameExpectation(
+      PageLoadMetricsWaiter::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