Add and report LayoutStability.JankScore UKM.

We prolong the lifetime of UkmPageLoadMetricsObserver to permit jank
score reporting at session end for tabs that start or become hidden,
but avoid extra reporting of other metrics by checking was_hidden_.

The OnFinalLayoutStabilityUpdate hook is removed in favor of passing
PageRenderData to existing hooks through PageLoadExtraInfo.  This lets
the observer decide when to report.

UKM collection review doc: go/lsukmrev

Bug: 581518
Change-Id: I2983dff155872f3080474e09df784d20b3cb08d3
Reviewed-on: https://chromium-review.googlesource.com/c/1299917
Commit-Queue: Steve Kobes <skobes@chromium.org>
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606877}

chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer.cc

@@ -16,6 +16,7 @@
 #include "services/metrics/public/cpp/ukm_builders.h"
 #include "services/metrics/public/cpp/ukm_recorder.h"
 #include "services/network/public/cpp/network_quality_tracker.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/metrics_proto/system_profile.pb.h"
 
 // static
@@ -40,8 +41,10 @@ UkmPageLoadMetricsObserver::ObservePolicy UkmPageLoadMetricsObserver::OnStart(
     content::NavigationHandle* navigation_handle,
     const GURL& currently_committed_url,
     bool started_in_foreground) {
-  if (!started_in_foreground)
-    return STOP_OBSERVING;
+  if (!started_in_foreground) {
+    was_hidden_ = true;
+    return CONTINUE_OBSERVING;
+  }
 
   // When OnStart is invoked, we don't yet know whether we're observing a web
   // page load, vs another kind of load (e.g. a download or a PDF). Thus,
@@ -76,23 +79,31 @@ UkmPageLoadMetricsObserver::ObservePolicy
 UkmPageLoadMetricsObserver::FlushMetricsOnAppEnterBackground(
     const page_load_metrics::mojom::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
-  RecordPageLoadExtraInfoMetrics(info, base::TimeTicks::Now());
-  RecordTimingMetrics(timing, info);
+  if (!was_hidden_) {
+    RecordPageLoadExtraInfoMetrics(info, base::TimeTicks::Now());
+    RecordTimingMetrics(timing, info);
+  }
+  ReportLayoutStability(info);
   return STOP_OBSERVING;
 }
 
 UkmPageLoadMetricsObserver::ObservePolicy UkmPageLoadMetricsObserver::OnHidden(
     const page_load_metrics::mojom::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
-  RecordPageLoadExtraInfoMetrics(
-      info, base::TimeTicks() /* no app_background_time */);
-  RecordTimingMetrics(timing, info);
-  return STOP_OBSERVING;
+  if (!was_hidden_) {
+    RecordPageLoadExtraInfoMetrics(
+        info, base::TimeTicks() /* no app_background_time */);
+    RecordTimingMetrics(timing, info);
+    was_hidden_ = true;
+  }
+  return CONTINUE_OBSERVING;
 }
 
 void UkmPageLoadMetricsObserver::OnFailedProvisionalLoad(
     const page_load_metrics::FailedProvisionalLoadInfo& failed_load_info,
     const page_load_metrics::PageLoadExtraInfo& extra_info) {
+  if (was_hidden_)
+    return;
   RecordPageLoadExtraInfoMetrics(
       extra_info, base::TimeTicks() /* no app_background_time */);
 
@@ -111,14 +122,19 @@ void UkmPageLoadMetricsObserver::OnFailedProvisionalLoad(
 void UkmPageLoadMetricsObserver::OnComplete(
     const page_load_metrics::mojom::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
-  RecordPageLoadExtraInfoMetrics(
-      info, base::TimeTicks() /* no app_background_time */);
-  RecordTimingMetrics(timing, info);
+  if (!was_hidden_) {
+    RecordPageLoadExtraInfoMetrics(
+        info, base::TimeTicks() /* no app_background_time */);
+    RecordTimingMetrics(timing, info);
+  }
+  ReportLayoutStability(info);
 }
 
 void UkmPageLoadMetricsObserver::OnLoadedResource(
     const page_load_metrics::ExtraRequestCompleteInfo&
         extra_request_complete_info) {
+  if (was_hidden_)
+    return;
   if (extra_request_complete_info.was_cached) {
     cache_bytes_ += extra_request_complete_info.raw_body_bytes;
   } else {
@@ -331,3 +347,20 @@ void UkmPageLoadMetricsObserver::ReportMainResourceTimingMetrics(
   builder->SetMainFrameResource_RequestStartToReceiveHeadersEnd(
       request_start_to_receive_headers_end_ms);
 }
+
+void UkmPageLoadMetricsObserver::ReportLayoutStability(
+    const page_load_metrics::PageLoadExtraInfo& info) {
+  // Avoid reporting when the feature is disabled. This ensures that a report
+  // with score == 0 only occurs for a page that was actually jank-free.
+  if (!base::FeatureList::IsEnabled(blink::features::kJankTracking))
+    return;
+
+  // Report (jank_score * 100) as an int in the range [0, 1000].
+  float jank_score = info.main_frame_render_data.layout_jank_score;
+  int64_t ukm_value =
+      static_cast<int>(roundf(std::min(jank_score, 10.0f) * 100.0f));
+
+  ukm::builders::PageLoad builder(info.source_id);
+  builder.SetLayoutStability_JankScore(ukm_value);
+  builder.Record(ukm::UkmRecorder::Get());
+}

chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer.h

@@ -78,6 +78,8 @@ class UkmPageLoadMetricsObserver
   // Adds main resource timing metrics to |builder|.
   void ReportMainResourceTimingMetrics(ukm::builders::PageLoad* builder);
 
+  void ReportLayoutStability(const page_load_metrics::PageLoadExtraInfo& info);
+
   // Guaranteed to be non-null during the lifetime of |this|.
   network::NetworkQualityTracker* network_quality_tracker_;
 
@@ -100,6 +102,9 @@ class UkmPageLoadMetricsObserver
   // PAGE_TRANSITION_LINK is the default PageTransition value.
   ui::PageTransition page_transition_ = ui::PAGE_TRANSITION_LINK;
 
+  // True if the page started hidden, or ever became hidden.
+  bool was_hidden_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(UkmPageLoadMetricsObserver);
 };
 

chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer_unittest.cc

@@ -8,6 +8,7 @@
 
 #include "base/metrics/metrics_hashes.h"
 #include "base/optional.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/test/simple_test_clock.h"
 #include "base/time/time.h"
 #include "chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.h"
@@ -21,6 +22,7 @@
 #include "services/metrics/public/cpp/ukm_source.h"
 #include "services/network/public/cpp/network_quality_tracker.h"
 #include "testing/gmock/include/gmock/gmock.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/metrics_proto/system_profile.pb.h"
 
 using testing::AnyNumber;
@@ -762,3 +764,34 @@ TEST_F(UkmPageLoadMetricsObserverTest, BodySizeMetrics) {
                                           bucketed_cache_bytes);
   }
 }
+
+TEST_F(UkmPageLoadMetricsObserverTest, LayoutStability) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(blink::features::kJankTracking);
+
+  NavigateAndCommit(GURL(kTestUrl1));
+
+  page_load_metrics::mojom::PageRenderData render_data(1.0);
+  SimulateRenderDataUpdate(render_data);
+
+  // Simulate hiding the tab (the report should include jank after hide).
+  web_contents()->WasHidden();
+
+  render_data.layout_jank_score = 2.5;
+  SimulateRenderDataUpdate(render_data);
+
+  // Simulate closing the tab.
+  DeleteContents();
+
+  const auto& ukm_recorder = 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, PageLoad::kLayoutStability_JankScoreName, 250);
+  }
+}

chrome/browser/page_load_metrics/page_load_metrics_observer.cc

@@ -22,6 +22,7 @@ PageLoadExtraInfo::PageLoadExtraInfo(
     const base::Optional<base::TimeDelta>& page_end_time,
     const mojom::PageLoadMetadata& main_frame_metadata,
     const mojom::PageLoadMetadata& subframe_metadata,
+    const mojom::PageRenderData& main_frame_render_data,
     ukm::SourceId source_id)
     : navigation_start(navigation_start),
       first_background_time(first_background_time),
@@ -36,6 +37,7 @@ PageLoadExtraInfo::PageLoadExtraInfo(
       page_end_time(page_end_time),
       main_frame_metadata(main_frame_metadata),
       subframe_metadata(subframe_metadata),
+      main_frame_render_data(main_frame_render_data),
       source_id(source_id) {}
 
 PageLoadExtraInfo::PageLoadExtraInfo(const PageLoadExtraInfo& other) = default;
@@ -55,7 +57,8 @@ PageLoadExtraInfo PageLoadExtraInfo::CreateForTesting(
       page_load_metrics::END_NONE,
       page_load_metrics::UserInitiatedInfo::NotUserInitiated(),
       base::TimeDelta(), page_load_metrics::mojom::PageLoadMetadata(),
-      page_load_metrics::mojom::PageLoadMetadata(), 0 /* source_id */);
+      page_load_metrics::mojom::PageLoadMetadata(),
+      page_load_metrics::mojom::PageRenderData(), 0 /* source_id */);
 }
 
 ExtraRequestCompleteInfo::ExtraRequestCompleteInfo(

chrome/browser/page_load_metrics/page_load_metrics_observer.h

@@ -136,6 +136,7 @@ struct PageLoadExtraInfo {
       const base::Optional<base::TimeDelta>& page_end_time,
       const mojom::PageLoadMetadata& main_frame_metadata,
       const mojom::PageLoadMetadata& subframe_metadata,
+      const mojom::PageRenderData& main_frame_render_data,
       ukm::SourceId source_id);
 
   // Simplified version of the constructor, intended for use in tests.
@@ -210,6 +211,8 @@ struct PageLoadExtraInfo {
   // PageLoadMetadata for subframes of the current page load.
   const mojom::PageLoadMetadata subframe_metadata;
 
+  const mojom::PageRenderData main_frame_render_data;
+
   // UKM SourceId for the current page load.
   const ukm::SourceId source_id;
 };
@@ -502,10 +505,6 @@ class PageLoadMetricsObserver {
   // Called when the event corresponding to |event_key| occurs in this page
   // load.
   virtual void OnEventOccurred(const void* const event_key) {}
-
-  // Called when the final layout jank score for the session is ready to be
-  // reported (immediately before OnComplete).
-  virtual void OnFinalLayoutStabilityUpdate(float jank_score) {}
 };
 
 }  // namespace page_load_metrics

chrome/browser/page_load_metrics/page_load_tracker.cc

@@ -255,9 +255,6 @@ PageLoadTracker::~PageLoadTracker() {
     if (failed_provisional_load_info_) {
       observer->OnFailedProvisionalLoad(*failed_provisional_load_info_, info);
     } else if (did_commit_) {
-      observer->OnFinalLayoutStabilityUpdate(
-          metrics_update_dispatcher_.main_frame_render_data()
-              .layout_jank_score);
       observer->OnComplete(metrics_update_dispatcher_.timing(), info);
     }
   }
@@ -511,7 +508,8 @@ PageLoadExtraInfo PageLoadTracker::ComputePageLoadExtraInfo() const {
       started_in_foreground_, user_initiated_info_, url(), start_url_,
       did_commit_, page_end_reason_, page_end_user_initiated_info_,
       page_end_time, metrics_update_dispatcher_.main_frame_metadata(),
-      metrics_update_dispatcher_.subframe_metadata(), source_id_);
+      metrics_update_dispatcher_.subframe_metadata(),
+      metrics_update_dispatcher_.main_frame_render_data(), source_id_);
 }
 
 bool PageLoadTracker::HasMatchingNavigationRequestID(

tools/metrics/ukm/ukm.xml

@@ -3233,6 +3233,12 @@ be describing additional metrics about the same event.
       meaningful input with longest queuing delay per navigation. In ms.
     </summary>
   </metric>
+  <metric name="LayoutStability.JankScore">
+    <summary>
+      Measures the amount of layout jank (bit.ly/lsm-explainer) that has
+      occurred during the session.
+    </summary>
+  </metric>
   <metric name="MainFrameResource.ConnectDelay">
     <summary>
       The duration between the start of the connection establishment to the end