BackForwardCache: Emit 0 samples for key metrics for bfcache

Now no samples are emitted for regular VOLT metrics after the page is
restored from the back-forward cache. This means that we will miss a lot
of metrics for history navigations after we launch back-forward cache.
As metrics for history navigations tend to be better figures than other
navigations (e.g., due to network cache), the average of such metrics
values will become worse and might seem regression if we don't take any
actions.

To mitigate this issue, we plan to emit 0 samples for such key metrics
for back-forward navigations. This is implemented behind the flag so
far, and we will enable this by default when we reach the conclusion how
to adjust them.

Bug: 1014174
Bug: 1121066
Change-Id: I04bf9646167aaf4c7241496a12ad66be3fe92681
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2354369
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800957}

chrome/browser/page_load_metrics/observers/back_forward_cache_page_load_metrics_observer_browsertest.cc

@@ -10,6 +10,7 @@
 #include "chrome/browser/page_load_metrics/integration_tests/metric_integration_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/page_load_metrics/browser/observers/back_forward_cache_page_load_metrics_observer.h"
+#include "components/page_load_metrics/browser/observers/core_page_load_metrics_observer.h"
 #include "components/page_load_metrics/browser/page_load_metrics_test_waiter.h"
 #include "components/page_load_metrics/browser/page_load_metrics_util.h"
 #include "components/page_load_metrics/common/test/page_load_metrics_test_util.h"
@@ -32,7 +33,8 @@ class BackForwardCachePageLoadMetricsObserverBrowserTest
   void SetUpCommandLine(base::CommandLine* command_line) override {
     feature_list_.InitWithFeaturesAndParameters(
         {{features::kBackForwardCache,
-          {{"TimeToLiveInBackForwardCacheInSeconds", "3600"}}}},
+          {{"TimeToLiveInBackForwardCacheInSeconds", "3600"}}},
+         {internal::kBackForwardCacheEmitZeroSamplesForKeyMetrics, {{}}}},
         {});
 
     MetricIntegrationTest::SetUpCommandLine(command_line);
@@ -124,6 +126,14 @@ IN_PROC_BROWSER_TEST_F(BackForwardCachePageLoadMetricsObserverBrowserTest,
         internal::kHistogramFirstPaintAfterBackForwardCacheRestore, 1);
     ExpectMetricCountForUrl(
         url_a, "NavigationToFirstPaintAfterBackForwardCacheRestore", 1);
+
+    // 0 values are emitted for non-back-forward-cache metrics due to the flag
+    // kBackForwardCacheEmitZeroSamplesForKeyMetrics.
+    histogram_tester().ExpectBucketCount(internal::kHistogramFirstPaint, 0, 1);
+    histogram_tester().ExpectBucketCount(
+        internal::kHistogramFirstContentfulPaint, 0, 1);
+    histogram_tester().ExpectBucketCount(
+        internal::kHistogramLargestContentfulPaint, 0, 1);
   }
 
   // The RenderFrameHost for the page B was likely in the back-forward cache
@@ -150,6 +160,12 @@ IN_PROC_BROWSER_TEST_F(BackForwardCachePageLoadMetricsObserverBrowserTest,
         internal::kHistogramFirstPaintAfterBackForwardCacheRestore, 2);
     ExpectMetricCountForUrl(
         url_a, "NavigationToFirstPaintAfterBackForwardCacheRestore", 2);
+
+    histogram_tester().ExpectBucketCount(internal::kHistogramFirstPaint, 0, 2);
+    histogram_tester().ExpectBucketCount(
+        internal::kHistogramFirstContentfulPaint, 0, 2);
+    histogram_tester().ExpectBucketCount(
+        internal::kHistogramLargestContentfulPaint, 0, 2);
   }
 }
 
@@ -193,6 +209,12 @@ IN_PROC_BROWSER_TEST_F(BackForwardCachePageLoadMetricsObserverBrowserTest,
         internal::kHistogramFirstPaintAfterBackForwardCacheRestore, 0);
     ExpectMetricCountForUrl(
         url_a, "NavigationToFirstPaintAfterBackForwardCacheRestore", 0);
+
+    histogram_tester().ExpectBucketCount(internal::kHistogramFirstPaint, 0, 0);
+    histogram_tester().ExpectBucketCount(
+        internal::kHistogramFirstContentfulPaint, 0, 0);
+    histogram_tester().ExpectBucketCount(
+        internal::kHistogramLargestContentfulPaint, 0, 0);
   }
 }
 
@@ -234,6 +256,11 @@ IN_PROC_BROWSER_TEST_F(BackForwardCachePageLoadMetricsObserverBrowserTest,
         internal::kHistogramFirstInputDelayAfterBackForwardCacheRestore, 1);
     ExpectMetricCountForUrl(url_a,
                             "FirstInputDelayAfterBackForwardCacheRestore", 1);
+
+    // 0 values are emitted for non-back-forward-cache metrics due to the flag
+    // kBackForwardCacheEmitZeroSamplesForKeyMetrics.
+    histogram_tester().ExpectBucketCount(internal::kHistogramFirstInputDelay, 0,
+                                         1);
   }
 }
 
@@ -323,6 +350,15 @@ return score;
                           "CumulativeShiftScoreAfterBackForwardCacheRestore",
                           page_load_metrics::LayoutShiftUkmValue(next_score));
 
+  // 0 values are emitted for non-back-forward-cache metrics due to the flag
+  // kBackForwardCacheEmitZeroSamplesForKeyMetrics.
+  // As back-foward cache is used twice (once for A and once for B), the current
+  // total count is 2.
+  histogram_tester().ExpectBucketCount(
+      "PageLoad.LayoutInstability.CumulativeShiftScore.MainFrame", 0, 2);
+  histogram_tester().ExpectBucketCount(
+      "PageLoad.LayoutInstability.CumulativeShiftScore", 0, 2);
+
   // Go back to A again.
   web_contents()->GetController().GoBack();
   EXPECT_TRUE(WaitForLoadStop(web_contents()));
@@ -342,4 +378,10 @@ return score;
 
   ExpectMetricCountForUrl(
       url_a, "CumulativeShiftScoreAfterBackForwardCacheRestore", 2);
+
+  // As back-foward cache is used fourth in total.
+  histogram_tester().ExpectBucketCount(
+      "PageLoad.LayoutInstability.CumulativeShiftScore.MainFrame", 0, 4);
+  histogram_tester().ExpectBucketCount(
+      "PageLoad.LayoutInstability.CumulativeShiftScore", 0, 4);
 }

components/page_load_metrics/browser/observers/back_forward_cache_page_load_metrics_observer.cc

@@ -4,6 +4,7 @@
 
 #include "components/page_load_metrics/browser/observers/back_forward_cache_page_load_metrics_observer.h"
 
+#include "components/page_load_metrics/browser/observers/core_page_load_metrics_observer.h"
 #include "components/page_load_metrics/browser/page_load_metrics_util.h"
 #include "services/metrics/public/cpp/ukm_builders.h"
 
@@ -21,6 +22,27 @@ extern const char kHistogramCumulativeShiftScoreAfterBackForwardCacheRestore[] =
     "PageLoad.LayoutInstability.CumulativeShiftScore."
     "AfterBackForwardCacheRestore";
 
+// Enables to emit zero values for some key metrics when back-forward cache is
+// used.
+//
+// With this flag disabled, no samples are emitted for regular VOLT metrics
+// after the page is restored from the back-forward cache. This means that we
+// will miss a lot of metrics for history navigations after we launch back-
+// forward cache. As metrics for history navigations tend to be better figures
+// than other navigations (e.g., due to network cache), the average of such
+// metrics values will become worse and might seem regression if we don't take
+// any actions.
+//
+// To mitigate this issue, we plan to emit 0 samples for such key metrics for
+// back-forward navigations. This is implemented behind this flag so far, and we
+// will enable this by default when we reach the conclusion how to adjust them.
+//
+// For cumulative layout shift scores, we use actual score values for back-
+// forward cache navigations instead of 0s.
+const base::Feature kBackForwardCacheEmitZeroSamplesForKeyMetrics{
+    "BackForwardCacheEmitZeroSamplesForKeyMetrics",
+    base::FEATURE_DISABLED_BY_DEFAULT};
+
 }  // namespace internal
 
 BackForwardCachePageLoadMetricsObserver::
@@ -63,6 +85,15 @@ void BackForwardCachePageLoadMetricsObserver::
     builder.SetNavigationToFirstPaintAfterBackForwardCacheRestore(
         first_paint.InMilliseconds());
     builder.Record(ukm::UkmRecorder::Get());
+
+    if (base::FeatureList::IsEnabled(
+            internal::kBackForwardCacheEmitZeroSamplesForKeyMetrics)) {
+      PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstPaint, base::TimeDelta{});
+      PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstContentfulPaint,
+                          base::TimeDelta{});
+      PAGE_LOAD_HISTOGRAM(internal::kHistogramLargestContentfulPaint,
+                          base::TimeDelta{});
+    }
   }
 }
 
@@ -86,6 +117,12 @@ void BackForwardCachePageLoadMetricsObserver::
     builder.SetFirstInputDelayAfterBackForwardCacheRestore(
         first_input_delay.value().InMilliseconds());
     builder.Record(ukm::UkmRecorder::Get());
+
+    if (base::FeatureList::IsEnabled(
+            internal::kBackForwardCacheEmitZeroSamplesForKeyMetrics)) {
+      PAGE_LOAD_HISTOGRAM(internal::kHistogramFirstInputDelay,
+                          base::TimeDelta{});
+    }
   }
 }
 
@@ -151,6 +188,16 @@ void BackForwardCachePageLoadMetricsObserver::
       GetDelegate().GetMainFrameRenderData().layout_shift_score;
   last_layout_shift_score_ =
       GetDelegate().GetPageRenderData().layout_shift_score;
+
+  if (base::FeatureList::IsEnabled(
+          internal::kBackForwardCacheEmitZeroSamplesForKeyMetrics)) {
+    UMA_HISTOGRAM_COUNTS_100(
+        "PageLoad.LayoutInstability.CumulativeShiftScore.MainFrame",
+        page_load_metrics::LayoutShiftUmaValue(layout_main_frame_shift_score));
+    UMA_HISTOGRAM_COUNTS_100(
+        "PageLoad.LayoutInstability.CumulativeShiftScore",
+        page_load_metrics::LayoutShiftUmaValue(layout_shift_score));
+  }
 }
 
 int64_t BackForwardCachePageLoadMetricsObserver::

components/page_load_metrics/browser/observers/back_forward_cache_page_load_metrics_observer.h

@@ -15,6 +15,7 @@ extern const char kHistogramCumulativeShiftScoreAfterBackForwardCacheRestore[];
 extern const char
     kHistogramCumulativeShiftScoreMainFrameAfterBackForwardCacheRestore[];
 extern const char kHistogramCumulativeShiftScoreAfterBackForwardCacheRestore[];
+extern const base::Feature kBackForwardCacheEmitZeroSamplesForKeyMetrics;
 
 }  // namespace internal