Add UKM for page-wide CLS before input or scroll.

As a side effect, this fixes layoutShiftScoreBeforeInputOrScroll in
the "CumulativeShiftScore::AllFrames::UMA" trace event added in
crrev.com/768267, which was previously always 0 as we did not set
layout_shift_score_before_input_or_scroll in PLMUD::page_render_data_.

Also correct description of MainFrame.BeforeInputOrScroll UKM to reflect
that both element and document scrolls terminate the accumulation.

UKM collection review: go/hofeq

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

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

@@ -837,6 +837,11 @@ void UkmPageLoadMetricsObserver::ReportLayoutStability() {
       .SetLayoutInstability_CumulativeShiftScore(
           page_load_metrics::LayoutShiftUkmValue(
               GetDelegate().GetPageRenderData().layout_shift_score))
+      .SetLayoutInstability_CumulativeShiftScore_BeforeInputOrScroll(
+          page_load_metrics::LayoutShiftUkmValue(
+              GetDelegate()
+                  .GetPageRenderData()
+                  .layout_shift_score_before_input_or_scroll))
       .SetLayoutInstability_CumulativeShiftScore_MainFrame(
           page_load_metrics::LayoutShiftUkmValue(
               GetDelegate().GetMainFrameRenderData().layout_shift_score))

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

@@ -1860,3 +1860,95 @@ TEST_F(UkmPageLoadMetricsObserverTest, NavigationTiming) {
     }
   }
 }
+
+class CLSUkmPageLoadMetricsObserverTest
+    : public UkmPageLoadMetricsObserverTest {
+ protected:
+  void RunBeforeInputOrScrollCase(bool input_in_subframe);
+  void SimulateShiftDelta(float delta, content::RenderFrameHost* frame);
+  RenderFrameHost* NavigateSubframe();
+  void VerifyUKMBuckets(int total, int before_input_or_scroll);
+  void InitPageLoadTimingWithInputOrScroll(
+      page_load_metrics::mojom::PageLoadTiming& timing,
+      base::TimeDelta timestamp);
+};
+
+void CLSUkmPageLoadMetricsObserverTest::SimulateShiftDelta(
+    float delta,
+    content::RenderFrameHost* frame) {
+  page_load_metrics::mojom::FrameRenderDataUpdate render_data(delta, delta, 0,
+                                                              0, 0, 0);
+  tester()->SimulateRenderDataUpdate(render_data, frame);
+}
+
+RenderFrameHost* CLSUkmPageLoadMetricsObserverTest::NavigateSubframe() {
+  return NavigationSimulator::NavigateAndCommitFromDocument(
+      GURL(kSubframeTestUrl),
+      RenderFrameHostTester::For(web_contents()->GetMainFrame())
+          ->AppendChild("subframe"));
+}
+
+void CLSUkmPageLoadMetricsObserverTest::VerifyUKMBuckets(
+    int total,
+    int before_input_or_scroll) {
+  const char* total_name =
+      PageLoad::kLayoutInstability_CumulativeShiftScoreName;
+  const char* before_input_or_scroll_name =
+      PageLoad::kLayoutInstability_CumulativeShiftScore_BeforeInputOrScrollName;
+
+  const auto& ukm_recorder = tester()->test_ukm_recorder();
+  std::map<ukm::SourceId, ukm::mojom::UkmEntryPtr> merged_entries =
+      ukm_recorder.GetMergedEntriesByName(PageLoad::kEntryName);
+  EXPECT_EQ(1ul, merged_entries.size());
+
+  for (const auto& kv : merged_entries) {
+    const ukm::mojom::UkmEntry* ukm_entry = kv.second.get();
+    ukm_recorder.ExpectEntrySourceHasUrl(ukm_entry, GURL(kTestUrl1));
+
+    ukm_recorder.ExpectEntryMetric(ukm_entry, total_name, total);
+    ukm_recorder.ExpectEntryMetric(ukm_entry, before_input_or_scroll_name,
+                                   before_input_or_scroll);
+  }
+}
+
+void CLSUkmPageLoadMetricsObserverTest::InitPageLoadTimingWithInputOrScroll(
+    page_load_metrics::mojom::PageLoadTiming& timing,
+    base::TimeDelta timestamp) {
+  page_load_metrics::InitPageLoadTimingForTest(&timing);
+  PopulateRequiredTimingFields(&timing);
+  timing.paint_timing->first_input_or_scroll_notified_timestamp = timestamp;
+}
+
+void CLSUkmPageLoadMetricsObserverTest::RunBeforeInputOrScrollCase(
+    bool input_in_subframe) {
+  NavigateAndCommit(GURL(kTestUrl1));
+
+  RenderFrameHost* main_frame = web_contents()->GetMainFrame();
+  RenderFrameHost* subframe = NavigateSubframe();
+
+  SimulateShiftDelta(1.0, main_frame);
+  SimulateShiftDelta(1.5, subframe);
+
+  // Simulate input.
+  page_load_metrics::mojom::PageLoadTiming timing;
+  InitPageLoadTimingWithInputOrScroll(timing, base::TimeDelta::FromSeconds(1));
+  tester()->SimulateTimingUpdate(timing,
+                                 input_in_subframe ? subframe : main_frame);
+
+  SimulateShiftDelta(1.2, main_frame);
+  SimulateShiftDelta(0.8, subframe);
+
+  DeleteContents();
+
+  // Total CLS: 1.0 + 1.5 + 1.2 + 0.8 = 4.5 (bucket 450).
+  // Before input: 1.0 + 1.5 = 2.5 (bucket 250).
+  VerifyUKMBuckets(450, 250);
+}
+
+TEST_F(CLSUkmPageLoadMetricsObserverTest, BeforeInputOrScroll_Main) {
+  RunBeforeInputOrScrollCase(false);
+}
+
+TEST_F(CLSUkmPageLoadMetricsObserverTest, BeforeInputOrScroll_Sub) {
+  RunBeforeInputOrScrollCase(true);
+}

components/page_load_metrics/browser/page_load_metrics_observer.h

@@ -94,15 +94,16 @@ struct UserInitiatedInfo {
 // Information about how the page rendered during the browsing session.
 // Derived from the FrameRenderDataUpdate that is sent via UpdateTiming IPC.
 struct PageRenderData {
-  PageRenderData()
-      : layout_shift_score(0), layout_shift_score_before_input_or_scroll(0) {}
+  PageRenderData() = default;
 
   // How much visible elements on the page shifted (bit.ly/lsm-explainer).
-  float layout_shift_score;
+  float layout_shift_score = 0;
 
   // How much visible elements on the page shifted (bit.ly/lsm-explainer),
-  // before user input or document scroll.
-  float layout_shift_score_before_input_or_scroll;
+  // before user input or document scroll. This field's meaning is context-
+  // dependent (see comments on page_render_data_ and main_frame_render_data_
+  // in PageLoadMetricsUpdateDispatcher).
+  float layout_shift_score_before_input_or_scroll = 0;
 
   // How many LayoutBlock instances were created.
   uint64_t all_layout_block_count = 0;

components/page_load_metrics/browser/page_load_metrics_update_dispatcher.cc

@@ -340,10 +340,14 @@ class PageLoadTimingMerger {
                          navigation_start_offset,
                          new_paint_timing.first_contentful_paint);
     if (is_main_frame) {
-      // FMP and FCP++ are only tracked in the main frame.
+      // FMP is only tracked in the main frame.
       target_paint_timing->first_meaningful_paint =
           new_paint_timing.first_meaningful_paint;
 
+      // LCP and the first input/scroll timestamp are not merged by us; instead,
+      // PLMUD passes the per-frame data to its client (PageLoadTracker) via
+      // OnTimingChanged and OnSubFrameTimingChanged, and merged results are
+      // calculated and held by the LargestContentfulPaintHandler.
       target_paint_timing->largest_contentful_paint =
           new_paint_timing.largest_contentful_paint->Clone();
       target_paint_timing->experimental_largest_contentful_paint =
@@ -480,6 +484,8 @@ void PageLoadMetricsUpdateDispatcher::UpdateMetrics(
   // Report new deferral info.
   client_->OnNewDeferredResourceCounts(*new_deferred_resource_data);
 
+  UpdateHasSeenInputOrScroll(*new_timing);
+
   bool is_main_frame = IsMainFrame(render_frame_host);
   if (is_main_frame) {
     UpdateMainFrameMetadata(render_frame_host, std::move(new_metadata));
@@ -499,6 +505,20 @@ void PageLoadMetricsUpdateDispatcher::UpdateMetrics(
   client_->UpdateFeaturesUsage(render_frame_host, *new_features);
 }
 
+void PageLoadMetricsUpdateDispatcher::UpdateHasSeenInputOrScroll(
+    const mojom::PageLoadTiming& new_timing) {
+  const mojom::PaintTiming* paint_timing = new_timing.paint_timing.get();
+  if (!paint_timing)
+    return;
+
+  // NOTE: we cannot use the first input/scroll in current_merged_page_timing_,
+  // because PageLoadTimingMerger ignores this field.  We could reach into the
+  // LargestContentfulPaintHandler, but it is simpler to just watch the
+  // per-frame timing updates and remember a boolean.
+  if (paint_timing->first_input_or_scroll_notified_timestamp.has_value())
+    has_seen_input_or_scroll_ = true;
+}
+
 void PageLoadMetricsUpdateDispatcher::UpdateFeatures(
     content::RenderFrameHost* render_frame_host,
     const mojom::PageLoadFeatures& new_features) {
@@ -707,6 +727,15 @@ void PageLoadMetricsUpdateDispatcher::UpdatePageRenderData(
     const mojom::FrameRenderDataUpdate& render_data) {
   page_render_data_.layout_shift_score += render_data.layout_shift_delta;
 
+  // Stop accumulating page-wide layout_shift_score_before_input_or_scroll after
+  // input or scroll in any frame. Note that we can't unconditionally accumulate
+  // layout_shift_delta_before_input_or_scroll, because that field only reflects
+  // input/scroll in the same frame as the shift.
+  if (!has_seen_input_or_scroll_) {
+    page_render_data_.layout_shift_score_before_input_or_scroll +=
+        render_data.layout_shift_delta_before_input_or_scroll;
+  }
+
   page_render_data_.all_layout_block_count +=
       render_data.all_layout_block_count_delta;
   page_render_data_.ng_layout_block_count +=
@@ -720,6 +749,10 @@ void PageLoadMetricsUpdateDispatcher::UpdatePageRenderData(
 void PageLoadMetricsUpdateDispatcher::UpdateMainFrameRenderData(
     const mojom::FrameRenderDataUpdate& render_data) {
   main_frame_render_data_.layout_shift_score += render_data.layout_shift_delta;
+
+  // Track main frame cumulative score up to the first input or scroll in the
+  // main frame. For this we do not care about inputs sent to subframes, so we
+  // should not check has_seen_input_or_scroll_ (but see crbug.com/1136207).
   main_frame_render_data_.layout_shift_score_before_input_or_scroll +=
       render_data.layout_shift_delta_before_input_or_scroll;
 

components/page_load_metrics/browser/page_load_metrics_update_dispatcher.h

@@ -218,6 +218,8 @@ class PageLoadMetricsUpdateDispatcher {
   void MaybeDispatchTimingUpdates(bool did_merge_new_timing_value);
   void DispatchTimingUpdates();
 
+  void UpdateHasSeenInputOrScroll(const mojom::PageLoadTiming& new_timing);
+
   // The client is guaranteed to outlive this object.
   Client* const client_;
 
@@ -229,14 +231,15 @@ class PageLoadMetricsUpdateDispatcher {
   // Time the navigation for this page load was initiated.
   const base::TimeTicks navigation_start_;
 
-  // PageLoadTiming for the currently tracked page. The fields in |paint_timing|
-  // are merged across all frames in the document. All other fields are from the
-  // main frame document. |current_merged_page_timing_| contains the most recent
-  // valid page load timing data, while pending_merged_page_timing_ contains
-  // pending updates received since |current_merged_page_timing_| was last
-  // dispatched to the client. pending_merged_page_timing_ will be copied to
-  // |current_merged_page_timing_| once it is valid, at the time the
-  // Client::OnTimingChanged callback is invoked.
+  // PageLoadTiming for the currently tracked page. Some fields, such as FCP,
+  // are merged across all frames in the document, while other fields are from
+  // the main frame only (see PageLoadTimingMerger).
+  //
+  // |current_merged_page_timing_| contains the most recent valid timing data,
+  // while |pending_merged_page_timing_| contains pending updates received since
+  // |current_merged_page_timing_| was last dispatched to the client (see
+  // DispatchTimingUpdates, which invokes the Client::OnTimingChanged callback).
+  //
   mojom::PageLoadTimingPtr current_merged_page_timing_;
   mojom::PageLoadTimingPtr pending_merged_page_timing_;
 
@@ -248,6 +251,19 @@ class PageLoadMetricsUpdateDispatcher {
   // InputTiming data accumulated across all frames.
   mojom::InputTiming page_input_timing_;
 
+  // In general, page_render_data_ contains combined data across all frames on
+  // the page, while main_frame_render_data_ contains data specific to the main
+  // frame.
+  //
+  // The layout_shift_score_before_input_or_scroll field in page_render_data_
+  // represents CLS across all frames (with subframe weighting), measured until
+  // first input/scroll in any frame (including an OOPIF).
+  //
+  // The main frame layout_shift_score_before_input_or_scroll represents CLS
+  // occurring within the main frame, measured until the first input/scroll seen
+  // by the main frame (or an input sent to a same-site subframe, due to
+  // crbug.com/1136207).
+  //
   PageRenderData page_render_data_;
   PageRenderData main_frame_render_data_;
 
@@ -260,6 +276,12 @@ class PageLoadMetricsUpdateDispatcher {
   // frame.
   std::map<FrameTreeNodeId, base::TimeDelta> subframe_navigation_start_offset_;
 
+  // Whether we have seen an input or scroll event in any frame. This comes to
+  // us via PaintTimingDetector::OnInputOrScroll, which triggers on user scrolls
+  // and most input types (but not mousemove or pinch zoom). More comments in
+  // UpdateHasSeenInputOrScroll.
+  bool has_seen_input_or_scroll_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(PageLoadMetricsUpdateDispatcher);
 };
 

tools/metrics/ukm/ukm.xml

@@ -8977,6 +8977,15 @@ be describing additional metrics about the same event.
       </history>
     </aggregation>
   </metric>
+  <metric name="LayoutInstability.CumulativeShiftScore.BeforeInputOrScroll">
+    <summary>
+      Measures the cumulative layout shift (bit.ly/lsm-explainer) that has
+      occurred on the page (including all subframes) during the session, before
+      any user input or scroll (in any frame). This metric's integral value is
+      100x the fractional cumulative layout shift score described in the
+      explainer.
+    </summary>
+  </metric>
   <metric name="LayoutInstability.CumulativeShiftScore.MainFrame">
     <summary>
       Measures the cumulative layout shift (bit.ly/lsm-explainer) that has
@@ -8990,8 +8999,8 @@ be describing additional metrics about the same event.
     <summary>
       Measures the cumulative layout shift (bit.ly/lsm-explainer) that has
       occurred in the main frame during the session, before any user input or
-      document scroll. This metric's integral value is 100x the fractional
-      cumulative layout shift score described in the explainer.
+      scroll. This metric's integral value is 100x the fractional cumulative
+      layout shift score described in the explainer.
     </summary>
   </metric>
   <metric name="LayoutStability.JankScore">