[LargestContentfulPaint] Allow removed content by default

In this CL we upgrade ExperimentalLargestContentfulPaint to become the
default version. To do this, we do the following:
* Update the LargestContentfulPaintCalculator in Blink since the web
API now should report according to the Experimental version, which is
equivalent to reporting by the values in the detectors monotonically.
That is, we just store the largest text or image size and we report a
new entry whenever we find a candidate with a larger size.
* In Blink's PaintTimingDetector, swap the experimental values with the
non-experimental ones, so that ExperimentalLCP becomes the 'actual' LCP
and the previous LCP becomes 'experimental'.
* In the browser side, change the naming of the experimental vs current
LCP histogram names in both UMA and UKM, so that the experimental name
becomes LCP while the current version becomes LCP2. This means that the
current 'experimental' histograms become obsolete as they are no longer
reported. This is done to continue reporting LCP for some time to
support Finch and other users of these metrics while the transition
occurs.

In a couple of months or so, we will stop reporting the 'experimental'
version of LCP, which means not reporting the LCP (not LCP2) histograms.
This will also enable simplifying the Image and Text
PaintTimingDetectors in Blink, as we will no longer need to keep track
of the paint timestamps of all nodes being painted.

Ukm collection review:
https://docs.google.com/document/d/1gIQ6XuUZtaRb-BmqW8QWpt_cJdIeCsHsAvPVQkiaOhM/edit

Bug: 1045640
Change-Id: I2e27c0f940656b12766feeff3d506c90152f28d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2480845
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Reviewed-by: Annie Sullivan <sullivan@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821436}

chrome/browser/page_load_metrics/integration_tests/data/largest_contentful_paint.html

@@ -62,8 +62,7 @@
 <div id="content_div_1">
   <img src="images/green-16x16.png"></img>
 </div>
-<div id="content_div_2">
-</div>
+<div id="content_div_2"></div>
 
 <script>
   const block_for_next_lcp = async () => {
@@ -84,26 +83,50 @@
     new_img.id = "blue_image";
   };
 
+  // Adds the largest image. We expect this
+  // operation to trigger a new LCP entry.
+  const add_largest_image = () => {
+    let new_img = document.createElement("img");
+    content_div_2.appendChild(new_img);
+    new_img.src = "images/green-256x256.png";
+  };
+
   // Removes the image added by 'add_larger_image'. We expect this operation to
-  // trigger a new LCP entry.
+  // not trigger a new LCP entry.
   const remove_larger_image = () => {
     const blue_image = document.getElementById("blue_image");
     assert_not_equals(blue_image, null);
     content_div_2.removeChild(blue_image);
   };
 
+  const waitForAnimationFrames = frameCount => {
+    return new Promise(resolve => {
+      const handleFrame = () => {
+        if (--frameCount <= 0)
+          resolve();
+        else
+          requestAnimationFrame(handleFrame);
+      };
+      requestAnimationFrame(handleFrame);
+    });
+  };
+
   const run_test = async () => {
     // This test exerciess the following scenario
     //  - have an initial page load with an image
     //  - assert that LCP fires for that image
     //  - add a larger image to the page
     //  - assert that LCP fires for the new image
     //  - remove the larger image
-    //  - assert that LCP fires (again) for the first image
+    //  - wait for some rAFs
+    //  - add the largest image to the page
+    //  - asset that the new LCP is for the largest
     const lcp_0 = await block_for_next_lcp();
     add_larger_image();
     const lcp_1 = await block_for_next_lcp();
     remove_larger_image();
+    await waitForAnimationFrames(3);
+    add_largest_image();
     const lcp_2 = await block_for_next_lcp();
 
     // Now that we've run through the scenario and collected our measurements,

chrome/browser/page_load_metrics/integration_tests/jsdeps.gni

@@ -5,6 +5,7 @@
 metric_integration_jsdeps = [
   "//third_party/blink/web_tests/external/wpt/images/blue96x96.png",
   "//third_party/blink/web_tests/external/wpt/images/green-16x16.png",
+  "//third_party/blink/web_tests/external/wpt/images/green-256x256.png",
   "//third_party/blink/web_tests/external/wpt/layout-instability/resources/util.js",
   "//third_party/blink/web_tests/external/wpt/layout-instability/simple-block-movement.html",
   "//third_party/blink/web_tests/external/wpt/layout-instability/sources-enclosure.html",

chrome/browser/page_load_metrics/integration_tests/largest_contentful_paint_browsertest.cc

@@ -57,8 +57,8 @@ void ValidateTraceEvents(std::unique_ptr<TraceAnalyzer> analyzer) {
   ValidateCandidate(16 * 16, *events[0]);
   // LCP_1 uses blue96x96.png, of size 96 x 96.
   ValidateCandidate(96 * 96, *events[1]);
-  // LCP_2 uses green-16x16.png, of size 16 x 16.
-  ValidateCandidate(16 * 16, *events[2]);
+  // LCP_2 uses green-256x256.png, of size 16 x 16.
+  ValidateCandidate(256 * 256, *events[2]);
 }
 
 }  // namespace
@@ -77,6 +77,8 @@ IN_PROC_BROWSER_TEST_F(MetricIntegrationTest, LargestContentfulPaint) {
       base::StrCat({window_origin, "/images/green-16x16.png"});
   const std::string image_2_url_expected =
       base::StrCat({window_origin, "/images/blue96x96.png"});
+  const std::string image_3_url_expected =
+      base::StrCat({window_origin, "/images/green-256x256.png"});
 
   content::EvalJsResult result = EvalJs(web_contents(), "run_test()");
   EXPECT_EQ("", result.error);
@@ -86,7 +88,7 @@ IN_PROC_BROWSER_TEST_F(MetricIntegrationTest, LargestContentfulPaint) {
   // need to be updated to reflect new semantics.
   const auto& list = result.value.GetList();
   const std::string expected_url[3] = {
-      image_1_url_expected, image_2_url_expected, image_1_url_expected};
+      image_1_url_expected, image_2_url_expected, image_3_url_expected};
   base::Optional<double> lcp_timestamps[3];
   for (size_t i = 0; i < 3; i++) {
     const std::string* url = list[i].FindStringPath("url");
@@ -95,12 +97,10 @@ IN_PROC_BROWSER_TEST_F(MetricIntegrationTest, LargestContentfulPaint) {
     lcp_timestamps[i] = list[i].FindDoublePath("time");
     EXPECT_TRUE(lcp_timestamps[i].has_value());
   }
-  EXPECT_EQ(lcp_timestamps[0], lcp_timestamps[2])
-      << "The first and last LCP reports should be for the same paint so they "
-         "should have the same timestamp";
   EXPECT_LT(lcp_timestamps[0], lcp_timestamps[1])
-      << "The first and second LCP reports should be for different paints so "
-         "should have different timestamps";
+      << "The first LCP report should be before the second";
+  EXPECT_LT(lcp_timestamps[1], lcp_timestamps[2])
+      << "The second LCP report should be before the third";
 
   // Need to navigate away from the test html page to force metrics to get
   // flushed/synced.

chrome/browser/page_load_metrics/observers/core/amp_page_load_metrics_observer.cc

@@ -382,7 +382,7 @@ void AMPPageLoadMetricsObserver::MaybeRecordAmpDocumentMetrics() {
                 *paint_timing->largest_contentful_paint,
                 &largest_content_paint_time, &largest_content_paint_size,
                 &largest_content_type)) {
-      builder.SetSubFrame_PaintTiming_NavigationToLargestContentfulPaint(
+      builder.SetSubFrame_PaintTiming_NavigationToLargestContentfulPaint2(
           largest_content_paint_time.value().InMilliseconds());
 
       // Adjust by the navigation_input_delta.
@@ -401,14 +401,15 @@ void AMPPageLoadMetricsObserver::MaybeRecordAmpDocumentMetrics() {
             largest_content_paint_time.value());
       }
     }
+    // TODO(crbug.com/1045640): Stop reporting the experimental obsolete
+    // version.
     if (page_load_metrics::LargestContentfulPaintHandler::
             AssignTimeAndSizeForLargestContentfulPaint(
                 *paint_timing->experimental_largest_contentful_paint,
                 &largest_content_paint_time, &largest_content_paint_size,
                 &largest_content_type)) {
-      builder
-          .SetSubFrame_PaintTiming_NavigationToExperimentalLargestContentfulPaint(
-              largest_content_paint_time.value().InMilliseconds());
+      builder.SetSubFrame_PaintTiming_NavigationToLargestContentfulPaint(
+          largest_content_paint_time.value().InMilliseconds());
     }
 
     if (subframe_info.timing->interactive_timing->first_input_delay

chrome/browser/page_load_metrics/observers/core/amp_page_load_metrics_observer_unittest.cc

@@ -368,11 +368,11 @@ TEST_F(AMPPageLoadMetricsObserverTest, SubFrameMetrics) {
   tester()->test_ukm_recorder().ExpectEntryMetric(
       entry.get(), "SubFrame.PaintTiming.NavigationToFirstContentfulPaint", 5);
   tester()->test_ukm_recorder().ExpectEntryMetric(
-      entry.get(), "SubFrame.PaintTiming.NavigationToLargestContentfulPaint",
+      entry.get(), "SubFrame.PaintTiming.NavigationToLargestContentfulPaint2",
       10);
   tester()->test_ukm_recorder().ExpectEntryMetric(
-      entry.get(),
-      "SubFrame.PaintTiming.NavigationToExperimentalLargestContentfulPaint", 8);
+      entry.get(), "SubFrame.PaintTiming.NavigationToLargestContentfulPaint",
+      8);
 }
 
 TEST_F(AMPPageLoadMetricsObserverTest, SubFrameMetrics_LayoutInstability) {
@@ -484,11 +484,11 @@ TEST_F(AMPPageLoadMetricsObserverTest, SubFrameMetricsFullNavigation) {
   tester()->test_ukm_recorder().ExpectEntryMetric(
       entry.get(), "SubFrame.PaintTiming.NavigationToFirstContentfulPaint", 5);
   tester()->test_ukm_recorder().ExpectEntryMetric(
-      entry.get(), "SubFrame.PaintTiming.NavigationToLargestContentfulPaint",
+      entry.get(), "SubFrame.PaintTiming.NavigationToLargestContentfulPaint2",
       10);
   tester()->test_ukm_recorder().ExpectEntryMetric(
-      entry.get(),
-      "SubFrame.PaintTiming.NavigationToExperimentalLargestContentfulPaint", 5);
+      entry.get(), "SubFrame.PaintTiming.NavigationToLargestContentfulPaint",
+      5);
 }
 
 TEST_F(AMPPageLoadMetricsObserverTest, SubFrameRecordOnFullNavigation) {

chrome/browser/page_load_metrics/observers/core/ukm_page_load_metrics_observer.cc

@@ -520,7 +520,7 @@ void UkmPageLoadMetricsObserver::RecordTimingMetrics(
   if (main_frame_largest_contentful_paint.ContainsValidTime() &&
       WasStartedInForegroundOptionalEventInForeground(
           main_frame_largest_contentful_paint.Time(), GetDelegate())) {
-    builder.SetPaintTiming_NavigationToLargestContentfulPaint_MainFrame(
+    builder.SetPaintTiming_NavigationToLargestContentfulPaint2_MainFrame(
         main_frame_largest_contentful_paint.Time().value().InMilliseconds());
   }
   const page_load_metrics::ContentfulPaintTimingInfo&
@@ -531,9 +531,10 @@ void UkmPageLoadMetricsObserver::RecordTimingMetrics(
   if (all_frames_largest_contentful_paint.ContainsValidTime() &&
       WasStartedInForegroundOptionalEventInForeground(
           all_frames_largest_contentful_paint.Time(), GetDelegate())) {
-    builder.SetPaintTiming_NavigationToLargestContentfulPaint(
+    builder.SetPaintTiming_NavigationToLargestContentfulPaint2(
         all_frames_largest_contentful_paint.Time().value().InMilliseconds());
   }
+  // TODO(crbug.com/1045640): Stop reporting the experimental obsolete versions.
   const page_load_metrics::ContentfulPaintTimingInfo&
       main_frame_experimental_largest_contentful_paint =
           GetDelegate()
@@ -543,11 +544,10 @@ void UkmPageLoadMetricsObserver::RecordTimingMetrics(
       WasStartedInForegroundOptionalEventInForeground(
           main_frame_experimental_largest_contentful_paint.Time(),
           GetDelegate())) {
-    builder
-        .SetPaintTiming_NavigationToExperimentalLargestContentfulPaint_MainFrame(
-            main_frame_experimental_largest_contentful_paint.Time()
-                .value()
-                .InMilliseconds());
+    builder.SetPaintTiming_NavigationToLargestContentfulPaint_MainFrame(
+        main_frame_experimental_largest_contentful_paint.Time()
+            .value()
+            .InMilliseconds());
   }
   const page_load_metrics::ContentfulPaintTimingInfo&
       all_frames_experimental_largest_contentful_paint =
@@ -558,7 +558,7 @@ void UkmPageLoadMetricsObserver::RecordTimingMetrics(
       WasStartedInForegroundOptionalEventInForeground(
           all_frames_experimental_largest_contentful_paint.Time(),
           GetDelegate())) {
-    builder.SetPaintTiming_NavigationToExperimentalLargestContentfulPaint(
+    builder.SetPaintTiming_NavigationToLargestContentfulPaint(
         all_frames_experimental_largest_contentful_paint.Time()
             .value()
             .InMilliseconds());

chrome/browser/page_load_metrics/observers/core/ukm_page_load_metrics_observer_unittest.cc

@@ -120,23 +120,21 @@ class UkmPageLoadMetricsObserverTest
     tester()->test_ukm_recorder().ExpectEntrySourceHasUrl(entry,
                                                           GURL(kTestUrl1));
     tester()->test_ukm_recorder().ExpectEntryMetric(
-        entry, PageLoad::kPaintTiming_NavigationToLargestContentfulPaintName,
+        entry, PageLoad::kPaintTiming_NavigationToLargestContentfulPaint2Name,
         value);
     tester()->test_ukm_recorder().ExpectEntryMetric(
-        entry,
-        PageLoad::
-            kPaintTiming_NavigationToExperimentalLargestContentfulPaintName,
+        entry, PageLoad::kPaintTiming_NavigationToLargestContentfulPaintName,
         value);
     if (test_main_frame) {
       tester()->test_ukm_recorder().ExpectEntryMetric(
           entry,
           PageLoad::
-              kPaintTiming_NavigationToLargestContentfulPaint_MainFrameName,
+              kPaintTiming_NavigationToLargestContentfulPaint2_MainFrameName,
           value);
       tester()->test_ukm_recorder().ExpectEntryMetric(
           entry,
           PageLoad::
-              kPaintTiming_NavigationToExperimentalLargestContentfulPaint_MainFrameName,
+              kPaintTiming_NavigationToLargestContentfulPaint_MainFrameName,
           value);
     }
     EXPECT_TRUE(tester()->test_ukm_recorder().EntryHasMetric(
@@ -177,20 +175,18 @@ class UkmPageLoadMetricsObserverTest
             PageLoad::kEntryName);
     EXPECT_EQ(1ul, merged_entries.size());
     const ukm::mojom::UkmEntry* entry = merged_entries.begin()->second.get();
+    EXPECT_FALSE(tester()->test_ukm_recorder().EntryHasMetric(
+        entry, PageLoad::kPaintTiming_NavigationToLargestContentfulPaint2Name));
     EXPECT_FALSE(tester()->test_ukm_recorder().EntryHasMetric(
         entry, PageLoad::kPaintTiming_NavigationToLargestContentfulPaintName));
     EXPECT_FALSE(tester()->test_ukm_recorder().EntryHasMetric(
         entry,
         PageLoad::
-            kPaintTiming_NavigationToExperimentalLargestContentfulPaintName));
+            kPaintTiming_NavigationToLargestContentfulPaint2_MainFrameName));
     EXPECT_FALSE(tester()->test_ukm_recorder().EntryHasMetric(
         entry,
         PageLoad::
             kPaintTiming_NavigationToLargestContentfulPaint_MainFrameName));
-    EXPECT_FALSE(tester()->test_ukm_recorder().EntryHasMetric(
-        entry,
-        PageLoad::
-            kPaintTiming_NavigationToExperimentalLargestContentfulPaint_MainFrameName));
 
     std::map<ukm::SourceId, ukm::mojom::UkmEntryPtr> internal_merged_entries =
         tester()->test_ukm_recorder().GetMergedEntriesByName(
@@ -478,7 +474,7 @@ TEST_F(UkmPageLoadMetricsObserverTest,
   EXPECT_EQ(1ul, merged_entries.size());
   const ukm::mojom::UkmEntry* entry = merged_entries.begin()->second.get();
   EXPECT_FALSE(tester()->test_ukm_recorder().EntryHasMetric(
-      entry, PageLoad::kPaintTiming_NavigationToLargestContentfulPaintName));
+      entry, PageLoad::kPaintTiming_NavigationToLargestContentfulPaint2Name));
   tester()->test_ukm_recorder().ExpectEntryMetric(
       entry, PageLoad::kNavigation_PageEndReason2Name,
       page_load_metrics::END_CLOSE);
@@ -553,7 +549,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, FCPPlusPlus_DiscardBackgroundResult) {
   EXPECT_EQ(1ul, merged_entries.size());
   const ukm::mojom::UkmEntry* entry = merged_entries.begin()->second.get();
   EXPECT_FALSE(tester()->test_ukm_recorder().EntryHasMetric(
-      entry, PageLoad::kPaintTiming_NavigationToLargestContentfulPaintName));
+      entry, PageLoad::kPaintTiming_NavigationToLargestContentfulPaint2Name));
   tester()->test_ukm_recorder().ExpectEntryMetric(
       entry, PageLoad::kNavigation_PageEndReason2Name,
       page_load_metrics::END_CLOSE);
@@ -809,11 +805,11 @@ TEST_F(UkmPageLoadMetricsObserverTest,
   const ukm::mojom::UkmEntry* entry = merged_entries.begin()->second.get();
   EXPECT_FALSE(tester()->test_ukm_recorder().EntryHasMetric(
       entry,
-      PageLoad::kPaintTiming_NavigationToLargestContentfulPaint_MainFrameName));
+      PageLoad::
+          kPaintTiming_NavigationToLargestContentfulPaint2_MainFrameName));
   EXPECT_FALSE(tester()->test_ukm_recorder().EntryHasMetric(
       entry,
-      PageLoad::
-          kPaintTiming_NavigationToExperimentalLargestContentfulPaint_MainFrameName));
+      PageLoad::kPaintTiming_NavigationToLargestContentfulPaint_MainFrameName));
 }
 
 TEST_F(UkmPageLoadMetricsObserverTest,

components/page_load_metrics/browser/observers/core/uma_page_load_metrics_observer.cc

@@ -128,21 +128,23 @@ const char kHistogramFirstContentfulPaintInitiatingProcess[] =
 const char kHistogramFirstMeaningfulPaint[] =
     "PageLoad.Experimental.PaintTiming.NavigationToFirstMeaningfulPaint";
 const char kHistogramLargestContentfulPaint[] =
-    "PageLoad.PaintTiming.NavigationToLargestContentfulPaint";
+    "PageLoad.PaintTiming.NavigationToLargestContentfulPaint2";
 const char kHistogramLargestContentfulPaintContentType[] =
     "PageLoad.Internal.PaintTiming.LargestContentfulPaint.ContentType";
 const char kHistogramLargestContentfulPaintMainFrame[] =
-    "PageLoad.PaintTiming.NavigationToLargestContentfulPaint.MainFrame";
+    "PageLoad.PaintTiming.NavigationToLargestContentfulPaint2.MainFrame";
 const char kHistogramLargestContentfulPaintMainFrameContentType[] =
     "PageLoad.Internal.PaintTiming.LargestContentfulPaint.MainFrame."
     "ContentType";
-const char kHistogramExperimentalLargestContentfulPaint[] =
-    "PageLoad.PaintTiming.NavigationToExperimentalLargestContentfulPaint";
+// TODO(crbug.com/1045640): Stop reporting these obsolete versions after some
+// time.
+const char kDeprecatedHistogramLargestContentfulPaint[] =
+    "PageLoad.PaintTiming.NavigationToLargestContentfulPaint";
 const char kHistogramExperimentalLargestContentfulPaintContentType[] =
     "PageLoad.Internal.PaintTiming.ExperimentalLargestContentfulPaint."
     "ContentType";
-const char kHistogramExperimentalLargestContentfulPaintMainFrame[] =
-    "PageLoad.PaintTiming.NavigationToExperimentalLargestContentfulPaint."
+const char kDeprecatedHistogramLargestContentfulPaintMainFrame[] =
+    "PageLoad.PaintTiming.NavigationToLargestContentfulPaint."
     "MainFrame";
 const char kHistogramExperimentalLargestContentfulPaintMainFrameContentType[] =
     "PageLoad.Internal.PaintTiming.ExperimentalLargestContentfulPaint."
@@ -986,7 +988,7 @@ void UmaPageLoadMetricsObserver::RecordTimingHistograms(
           main_frame_experimental_largest_contentful_paint.Time(),
           GetDelegate())) {
     PAGE_LOAD_HISTOGRAM(
-        internal::kHistogramExperimentalLargestContentfulPaintMainFrame,
+        internal::kDeprecatedHistogramLargestContentfulPaintMainFrame,
         main_frame_experimental_largest_contentful_paint.Time().value());
     UMA_HISTOGRAM_ENUMERATION(
         internal::
@@ -1004,7 +1006,7 @@ void UmaPageLoadMetricsObserver::RecordTimingHistograms(
           all_frames_experimental_largest_contentful_paint.Time(),
           GetDelegate())) {
     PAGE_LOAD_HISTOGRAM(
-        internal::kHistogramExperimentalLargestContentfulPaint,
+        internal::kDeprecatedHistogramLargestContentfulPaint,
         all_frames_experimental_largest_contentful_paint.Time().value());
     UMA_HISTOGRAM_ENUMERATION(
         internal::kHistogramExperimentalLargestContentfulPaintContentType,

components/page_load_metrics/browser/observers/core/uma_page_load_metrics_observer.h

@@ -31,9 +31,9 @@ extern const char kHistogramLargestContentfulPaint[];
 extern const char kHistogramLargestContentfulPaintContentType[];
 extern const char kHistogramLargestContentfulPaintMainFrame[];
 extern const char kHistogramLargestContentfulPaintMainFrameContentType[];
-extern const char kHistogramExperimentalLargestContentfulPaint[];
+extern const char kDeprecatedHistogramLargestContentfulPaint[];
 extern const char kHistogramExperimentalLargestContentfulPaintContentType[];
-extern const char kHistogramExperimentalLargestContentfulPaintMainFrame[];
+extern const char kDeprecatedHistogramLargestContentfulPaintMainFrame[];
 extern const char
     kHistogramExperimentalLargestContentfulPaintMainFrameContentType[];
 extern const char kHistogramParseDuration[];