Add largest contentful paint for pages that load a third-party font

Change-Id: Ie8ba0b911176f00be1b1cb77c0fedfe09ce229a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2008288
Commit-Queue: Shivani Sharma <shivanisha@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736607}

chrome/browser/page_load_metrics/observers/third_party_metrics_observer.cc

@@ -9,6 +9,7 @@
 #include "components/page_load_metrics/browser/page_load_metrics_util.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/resource_type.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 
 namespace {
@@ -17,6 +18,20 @@ namespace {
 // keep track of in memory.
 const int kMaxRecordedFrames = 50;
 
+bool IsSameSite(const url::Origin& origin1, const url::Origin& origin2) {
+  return origin1.scheme() == origin2.scheme() &&
+         net::registry_controlled_domains::SameDomainOrHost(
+             origin1, origin2,
+             net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
+}
+
+bool IsSameSite(const GURL& url1, const GURL& url2) {
+  return url1.SchemeIs(url2.scheme()) &&
+         net::registry_controlled_domains::SameDomainOrHost(
+             url1, url2,
+             net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
+}
+
 }  // namespace
 
 ThirdPartyMetricsObserver::AccessedTypes::AccessedTypes(
@@ -45,13 +60,26 @@ ThirdPartyMetricsObserver::FlushMetricsOnAppEnterBackground(
     const page_load_metrics::mojom::PageLoadTiming& timing) {
   // The browser may come back, but there is no guarantee. To be safe, record
   // what we have now and ignore future changes to this navigation.
-  RecordMetrics();
+  RecordMetrics(timing);
   return STOP_OBSERVING;
 }
 
 void ThirdPartyMetricsObserver::OnComplete(
     const page_load_metrics::mojom::PageLoadTiming& timing) {
-  RecordMetrics();
+  RecordMetrics(timing);
+}
+
+void ThirdPartyMetricsObserver::OnLoadedResource(
+    const page_load_metrics::ExtraRequestCompleteInfo&
+        extra_request_complete_info) {
+  if (third_party_font_loaded_ || extra_request_complete_info.resource_type !=
+                                      content::ResourceType::kFontResource) {
+    return;
+  }
+
+  third_party_font_loaded_ =
+      !IsSameSite(GetDelegate().GetUrl(),
+                  extra_request_complete_info.origin_of_final_url.GetURL());
 }
 
 void ThirdPartyMetricsObserver::OnCookiesRead(
@@ -84,6 +112,8 @@ void ThirdPartyMetricsObserver::OnDomStorageAccessed(
 
 void ThirdPartyMetricsObserver::OnDidFinishSubFrameNavigation(
     content::NavigationHandle* navigation_handle) {
+  largest_contentful_paint_handler_.OnDidFinishSubFrameNavigation(
+      navigation_handle, GetDelegate());
   DCHECK(navigation_handle->GetNetworkIsolationKey().GetTopFrameOrigin());
 
   if (!navigation_handle->HasCommitted())
@@ -104,6 +134,8 @@ void ThirdPartyMetricsObserver::OnFrameDeleted(
 void ThirdPartyMetricsObserver::OnTimingUpdate(
     content::RenderFrameHost* subframe_rfh,
     const page_load_metrics::mojom::PageLoadTiming& timing) {
+  largest_contentful_paint_handler_.RecordTiming(timing.paint_timing,
+                                                 subframe_rfh);
   if (!timing.paint_timing->first_contentful_paint)
     return;
 
@@ -127,12 +159,8 @@ void ThirdPartyMetricsObserver::OnTimingUpdate(
 
   const url::Origin& top_frame_origin = top_frame->GetLastCommittedOrigin();
   const url::Origin& subframe_origin = subframe_rfh->GetLastCommittedOrigin();
-  if (top_frame_origin.scheme() == subframe_origin.scheme() &&
-      net::registry_controlled_domains::SameDomainOrHost(
-          top_frame_origin, subframe_origin,
-          net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)) {
+  if (IsSameSite(top_frame_origin, subframe_origin))
     return;
-  }
 
   if (page_load_metrics::WasStartedInForegroundOptionalEventInForeground(
           timing.paint_timing->first_contentful_paint, GetDelegate())) {
@@ -161,12 +189,8 @@ void ThirdPartyMetricsObserver::OnCookieOrStorageAccess(
   // return false, and function execution will continue because it is considered
   // 3rd party. Since |first_party_url| is actually the |site_for_cookies|, this
   // will happen e.g. for a 3rd party iframe on document.cookie access.
-  if (url.SchemeIs(first_party_url.scheme()) &&
-      net::registry_controlled_domains::SameDomainOrHost(
-          url, first_party_url,
-          net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)) {
+  if (IsSameSite(url, first_party_url))
     return;
-  }
 
   std::string registrable_domain =
       net::registry_controlled_domains::GetDomainAndRegistry(
@@ -214,7 +238,8 @@ void ThirdPartyMetricsObserver::OnCookieOrStorageAccess(
   third_party_accessed_types_.emplace(representative_url, access_type);
 }
 
-void ThirdPartyMetricsObserver::RecordMetrics() {
+void ThirdPartyMetricsObserver::RecordMetrics(
+    const page_load_metrics::mojom::PageLoadTiming& main_frame_timing) {
   if (!should_record_metrics_)
     return;
 
@@ -240,4 +265,18 @@ void ThirdPartyMetricsObserver::RecordMetrics() {
   UMA_HISTOGRAM_COUNTS_1000(
       "PageLoad.Clients.ThirdParty.Origins.SessionStorageAccess2",
       session_storage_origin_access);
+
+  const page_load_metrics::ContentfulPaintTimingInfo&
+      all_frames_largest_contentful_paint =
+          largest_contentful_paint_handler_.MergeMainFrameAndSubframes();
+  if (third_party_font_loaded_ &&
+      all_frames_largest_contentful_paint.ContainsValidTime() &&
+      all_frames_largest_contentful_paint.Type() == LargestContentType::kText &&
+      WasStartedInForegroundOptionalEventInForeground(
+          all_frames_largest_contentful_paint.Time(), GetDelegate())) {
+    PAGE_LOAD_HISTOGRAM(
+        "PageLoad.Clients.ThirdParty.PaintTiming."
+        "NavigationToLargestContentfulPaint.HasThirdPartyFont",
+        all_frames_largest_contentful_paint.Time().value());
+  }
 }

chrome/browser/page_load_metrics/observers/third_party_metrics_observer.h

@@ -9,6 +9,7 @@
 #include <string>
 
 #include "base/macros.h"
+#include "components/page_load_metrics/browser/observers/largest_contentful_paint_handler.h"
 #include "components/page_load_metrics/browser/page_load_metrics_observer.h"
 #include "url/gurl.h"
 #include "url/origin.h"
@@ -25,6 +26,8 @@ class ThirdPartyMetricsObserver
       const page_load_metrics::mojom::PageLoadTiming& timing) override;
   void OnComplete(
       const page_load_metrics::mojom::PageLoadTiming& timing) override;
+  void OnLoadedResource(const page_load_metrics::ExtraRequestCompleteInfo&
+                            extra_request_complete_info) override;
   void OnCookiesRead(const GURL& url,
                      const GURL& first_party_url,
                      const net::CookieList& cookie_list,
@@ -64,16 +67,18 @@ class ThirdPartyMetricsObserver
                                const GURL& first_party_url,
                                bool blocked_by_policy,
                                AccessType access_type);
-  void RecordMetrics();
+  void RecordMetrics(
+      const page_load_metrics::mojom::PageLoadTiming& main_frame_timing);
 
-  // A map of third parties that have read or written cookies, or have accessed
-  // local storage or session storage on this page.
+  // A map of third parties that have read or written cookies, or have
+  // accessed local storage or session storage on this page.
   //
-  // A third party document.cookie / window.localStorage / window.sessionStorage
-  // happens when the context's scheme://eTLD+1 differs from the main frame's.
-  // A third party resource request happens when the URL request's
-  // scheme://eTLD+1 differs from the main frame's. For URLs which have no
-  // registrable domain, the hostname is used instead.
+  // A third party document.cookie / window.localStorage /
+  // window.sessionStorage happens when the context's scheme://eTLD+1
+  // differs from the main frame's. A third party resource request happens
+  // when the URL request's scheme://eTLD+1 differs from the main frame's.
+  // For URLs which have no registrable domain, the hostname is used
+  // instead.
   std::map<GURL, AccessedTypes> third_party_accessed_types_;
 
   // A set of RenderFrameHosts that we've recorded timing data for. The
@@ -85,6 +90,12 @@ class ThirdPartyMetricsObserver
   // metrics for the page.
   bool should_record_metrics_ = true;
 
+  // True if this page loaded a third-party font.
+  bool third_party_font_loaded_ = false;
+
+  page_load_metrics::LargestContentfulPaintHandler
+      largest_contentful_paint_handler_;
+
   DISALLOW_COPY_AND_ASSIGN(ThirdPartyMetricsObserver);
 };
 

chrome/browser/page_load_metrics/observers/third_party_metrics_observer_unittest.cc

@@ -7,12 +7,13 @@
 #include "chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.h"
 #include "components/page_load_metrics/browser/page_load_tracker.h"
 #include "components/page_load_metrics/common/page_load_metrics.mojom.h"
+#include "components/page_load_metrics/common/test/page_load_metrics_test_util.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/navigation_simulator.h"
 #include "content/public/test/test_renderer_host.h"
 #include "net/cookies/canonical_cookie.h"
-#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/gmock/include/gmock/gmock.h"
 
 const char kReadCookieHistogram[] =
     "PageLoad.Clients.ThirdParty.Origins.CookieRead2";
@@ -438,6 +439,111 @@ TEST_F(ThirdPartyMetricsObserverTest,
       kAccessSessionStorageHistogram, 1, 1);
 }
 
+TEST_F(ThirdPartyMetricsObserverTest,
+       LargestContentfulPaint_HasThirdPartyFont) {
+  page_load_metrics::mojom::PageLoadTiming timing;
+  page_load_metrics::InitPageLoadTimingForTest(&timing);
+  timing.navigation_start = base::Time::FromDoubleT(1);
+  timing.paint_timing->largest_image_paint = base::TimeDelta();
+  timing.paint_timing->largest_image_paint_size = 100u;
+
+  timing.paint_timing->largest_text_paint =
+      base::TimeDelta::FromMilliseconds(4780);
+  timing.paint_timing->largest_text_paint_size = 120u;
+
+  PopulateRequiredTimingFields(&timing);
+
+  NavigateAndCommit(GURL("https://foo.test"));
+  tester()->SimulateTimingUpdate(timing);
+
+  int frame_tree_node_id = main_rfh()->GetFrameTreeNodeId();
+  tester()->SimulateLoadedResource(
+      {url::Origin::Create(GURL("https://bar.test")), net::IPEndPoint(),
+       frame_tree_node_id, false /* was_cached */,
+       1024 * 20 /* raw_body_bytes */, 0 /* original_network_content_length */,
+       nullptr /* data_reduction_proxy_data */,
+       content::ResourceType::kFontResource, 0, nullptr /* load_timing_info */},
+      content::GlobalRequestID());
+
+  // Navigate again to force histogram recording.
+  NavigateAndCommit(GURL("https://foo.test"));
+
+  EXPECT_THAT(tester()->histogram_tester().GetAllSamples(
+                  "PageLoad.Clients.ThirdParty.PaintTiming."
+                  "NavigationToLargestContentfulPaint.HasThirdPartyFont"),
+              testing::ElementsAre(base::Bucket(4780, 1)));
+}
+
+TEST_F(ThirdPartyMetricsObserverTest,
+       NoLargestContentfulPaint_HasThirdPartyFont) {
+  page_load_metrics::mojom::PageLoadTiming timing;
+  page_load_metrics::InitPageLoadTimingForTest(&timing);
+  timing.navigation_start = base::Time::FromDoubleT(1);
+  timing.paint_timing->largest_image_paint = base::TimeDelta();
+  timing.paint_timing->largest_image_paint_size = 100u;
+
+  timing.paint_timing->largest_text_paint =
+      base::TimeDelta::FromMilliseconds(4780);
+  timing.paint_timing->largest_text_paint_size = 120u;
+
+  PopulateRequiredTimingFields(&timing);
+
+  NavigateAndCommit(GURL("http://a.foo.test"));
+  tester()->SimulateTimingUpdate(timing);
+
+  // Load a same-site font, the histogram should not be recorded.
+  int frame_tree_node_id = main_rfh()->GetFrameTreeNodeId();
+  tester()->SimulateLoadedResource(
+      {url::Origin::Create(GURL("http://b.foo.test")), net::IPEndPoint(),
+       frame_tree_node_id, false /* was_cached */,
+       1024 * 20 /* raw_body_bytes */, 0 /* original_network_content_length */,
+       nullptr /* data_reduction_proxy_data */,
+       content::ResourceType::kFontResource, 0, nullptr /* load_timing_info */},
+      content::GlobalRequestID());
+
+  // Navigate again to force histogram recording.
+  NavigateAndCommit(GURL("https://foo.test"));
+
+  tester()->histogram_tester().ExpectTotalCount(
+      "PageLoad.Clients.ThirdParty.PaintTiming."
+      "NavigationToLargestContentfulPaint.HasThirdPartyFont",
+      0);
+}
+
+TEST_F(ThirdPartyMetricsObserverTest,
+       NoTextLargestContentfulPaint_HasThirdPartyFont) {
+  page_load_metrics::mojom::PageLoadTiming timing;
+  page_load_metrics::InitPageLoadTimingForTest(&timing);
+  timing.navigation_start = base::Time::FromDoubleT(1);
+  timing.paint_timing->largest_image_paint =
+      base::TimeDelta::FromMilliseconds(4780);
+  timing.paint_timing->largest_image_paint_size = 120u;
+
+  PopulateRequiredTimingFields(&timing);
+
+  NavigateAndCommit(GURL("https://foo.test"));
+  tester()->SimulateTimingUpdate(timing);
+
+  int frame_tree_node_id = main_rfh()->GetFrameTreeNodeId();
+  tester()->SimulateLoadedResource(
+      {url::Origin::Create(GURL("https://bar.test")), net::IPEndPoint(),
+       frame_tree_node_id, false /* was_cached */,
+       1024 * 20 /* raw_body_bytes */, 0 /* original_network_content_length */,
+       nullptr /* data_reduction_proxy_data */,
+       content::ResourceType::kFontResource, 0, nullptr /* load_timing_info */},
+      content::GlobalRequestID());
+
+  // Navigate again to force histogram recording.
+  NavigateAndCommit(GURL("https://foo.test"));
+
+  // Since largest contentful paint is of type image, the histogram will not be
+  // recorded.
+  tester()->histogram_tester().ExpectTotalCount(
+      "PageLoad.Clients.ThirdParty.PaintTiming."
+      "NavigationToLargestContentfulPaint.HasThirdPartyFont",
+      0);
+}
+
 class ThirdPartyDomStorageAccessMetricsObserverTest
     : public ThirdPartyMetricsObserverTest,
       public ::testing::WithParamInterface<bool /* is_local_access */> {

tools/metrics/histograms/histograms.xml

@@ -106916,6 +106916,18 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram
+    name="PageLoad.Clients.ThirdParty.PaintTiming.NavigationToLargestContentfulPaint.HasThirdPartyFont"
+    units="ms" expires_after="2020-12-31">
+  <owner>shivanisha@chromium.org</owner>
+  <owner>jkarlin@chromium.org</owner>
+  <summary>
+    Records the largest contentful paint only for pages that load a third party
+    font and whose largest contentful resource type is text. See
+    PageLoad.PaintTiming.NavigationToLargestContentfulPaint for details.
+  </summary>
+</histogram>
+
 <histogram base="true" name="PageLoad.Cpu" units="units"
     expires_after="2022-01-05">
   <owner>ericrobinson@chromium.org</owner>