BackForwardCache: Make PageLoadTiming::back_forward_cache_timings an array

We are adding a new metrics related to back-forward cache e.g., first
input delay after back-forward cache navigation happens. This CL is a
preparation for that change.

Bug: 1014174
Change-Id: I38549e4acb0659877bcd3aa976f0697a596e45dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2246360
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: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780213}

components/page_load_metrics/browser/observers/back_forward_cache_page_load_metrics_observer.cc

@@ -27,11 +27,10 @@ BackForwardCachePageLoadMetricsObserver::OnEnterBackForwardCache(
 
 void BackForwardCachePageLoadMetricsObserver::
     OnFirstPaintAfterBackForwardCacheRestoreInPage(
-        const page_load_metrics::mojom::PageLoadTiming& timing) {
-  auto first_paint = timing.back_forward_cache_timing
-                         ->first_paint_after_back_forward_cache_restore.back();
-  if (!first_paint.is_zero() &&
-      page_load_metrics::
+        const page_load_metrics::mojom::BackForwardCacheTiming& timing) {
+  auto first_paint = timing.first_paint_after_back_forward_cache_restore;
+  DCHECK(!first_paint.is_zero());
+  if (page_load_metrics::
           WasStartedInForegroundOptionalEventInForegroundAfterBackForwardCacheRestore(
               first_paint, GetDelegate())) {
     PAGE_LOAD_HISTOGRAM(

components/page_load_metrics/browser/observers/back_forward_cache_page_load_metrics_observer.h

@@ -24,7 +24,7 @@ class BackForwardCachePageLoadMetricsObserver
   OnEnterBackForwardCache(
       const page_load_metrics::mojom::PageLoadTiming& timing) override;
   void OnFirstPaintAfterBackForwardCacheRestoreInPage(
-      const page_load_metrics::mojom::PageLoadTiming& timing) override;
+      const page_load_metrics::mojom::BackForwardCacheTiming& timing) override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(BackForwardCachePageLoadMetricsObserver);

components/page_load_metrics/browser/page_load_metrics_observer.h

@@ -367,7 +367,7 @@ class PageLoadMetricsObserver {
   // This is called once every time when the page is restored from the
   // back-forward cache.
   virtual void OnFirstPaintAfterBackForwardCacheRestoreInPage(
-      const mojom::PageLoadTiming& timing) {}
+      const mojom::BackForwardCacheTiming& timing) {}
 
   // Unlike other paint callbacks, OnFirstMeaningfulPaintInMainFrameDocument is
   // tracked per document, and is reported for the main frame document only.

components/page_load_metrics/browser/page_load_metrics_test_waiter.cc

@@ -246,8 +246,9 @@ PageLoadMetricsTestWaiter::GetMatchedBits(
     matched_bits.Set(TimingField::kFirstInputOrScroll);
   if (timing.interactive_timing->first_input_delay)
     matched_bits.Set(TimingField::kFirstInputDelay);
-  if (!timing.back_forward_cache_timing
-           ->first_paint_after_back_forward_cache_restore.empty()) {
+  if (!timing.back_forward_cache_timings.empty() &&
+      !timing.back_forward_cache_timings.back()
+           ->first_paint_after_back_forward_cache_restore.is_zero()) {
     matched_bits.Set(TimingField::kFirstPaintAfterBackForwardCacheRestore);
   }
 

components/page_load_metrics/browser/page_load_metrics_update_dispatcher.cc

@@ -263,7 +263,7 @@ class PageLoadTimingMerger {
                            *new_page_load_timing.interactive_timing,
                            is_main_frame);
     MergeBackForwardCacheTiming(navigation_start_offset,
-                                *new_page_load_timing.back_forward_cache_timing,
+                                new_page_load_timing.back_forward_cache_timings,
                                 is_main_frame);
   }
 
@@ -396,16 +396,15 @@ class PageLoadTimingMerger {
 
   void MergeBackForwardCacheTiming(
       base::TimeDelta navigation_start_offset,
-      const mojom::BackForwardCacheTiming& new_back_forward_cache_timing,
+      const std::vector<mojo::StructPtr<mojom::BackForwardCacheTiming>>&
+          new_back_forward_cache_timings,
       bool is_main_frame) {
-    mojom::BackForwardCacheTiming* target_back_forward_cache_timing =
-        target_->back_forward_cache_timing.get();
-
     if (is_main_frame) {
-      target_back_forward_cache_timing
-          ->first_paint_after_back_forward_cache_restore =
-          new_back_forward_cache_timing
-              .first_paint_after_back_forward_cache_restore;
+      target_->back_forward_cache_timings.clear();
+      target_->back_forward_cache_timings.reserve(
+          new_back_forward_cache_timings.size());
+      for (const auto& timing : new_back_forward_cache_timings)
+        target_->back_forward_cache_timings.push_back(timing.Clone());
     }
   }
 

components/page_load_metrics/browser/page_load_tracker.cc

@@ -122,6 +122,39 @@ void RecordAppBackgroundPageLoadCompleted(bool completed_after_background) {
                         completed_after_background);
 }
 
+void DispatchFirstPaintAfterBackForwardCacheRestore(
+    PageLoadMetricsObserver* observer,
+    const std::vector<mojo::StructPtr<mojom::BackForwardCacheTiming>>&
+        last_timings,
+    const std::vector<mojo::StructPtr<mojom::BackForwardCacheTiming>>&
+        new_timings) {
+  DCHECK_GE(new_timings.size(), last_timings.size());
+
+  for (size_t i = 0; i < new_timings.size(); i++) {
+    auto first_paint =
+        new_timings[i]->first_paint_after_back_forward_cache_restore;
+
+    // The back-forward navigation happened, but the first-paint event has not
+    // happened yet.
+    if (first_paint.is_zero())
+      continue;
+
+    if (i < last_timings.size()) {
+      auto last_first_paint =
+          last_timings[i]->first_paint_after_back_forward_cache_restore;
+
+      // The first-paint after the page was restored from the cache was already
+      // recorded.
+      if (!last_first_paint.is_zero()) {
+        DCHECK_EQ(last_first_paint, first_paint);
+        continue;
+      }
+    }
+
+    observer->OnFirstPaintAfterBackForwardCacheRestoreInPage(*new_timings[i]);
+  }
+}
+
 void DispatchObserverTimingCallbacks(PageLoadMetricsObserver* observer,
                                      const mojom::PageLoadTiming& last_timing,
                                      const mojom::PageLoadTiming& new_timing) {
@@ -143,12 +176,9 @@ void DispatchObserverTimingCallbacks(PageLoadMetricsObserver* observer,
       !last_timing.paint_timing->first_paint) {
     observer->OnFirstPaintInPage(new_timing);
   }
-  if (new_timing.back_forward_cache_timing
-          ->first_paint_after_back_forward_cache_restore.size() !=
-      last_timing.back_forward_cache_timing
-          ->first_paint_after_back_forward_cache_restore.size()) {
-    observer->OnFirstPaintAfterBackForwardCacheRestoreInPage(new_timing);
-  }
+  DispatchFirstPaintAfterBackForwardCacheRestore(
+      observer, last_timing.back_forward_cache_timings,
+      new_timing.back_forward_cache_timings);
   if (new_timing.paint_timing->first_image_paint &&
       !last_timing.paint_timing->first_image_paint) {
     observer->OnFirstImagePaintInPage(new_timing);

components/page_load_metrics/common/page_load_metrics.mojom

@@ -146,13 +146,9 @@ struct PageLoadTiming {
   PaintTiming paint_timing;
   ParseTiming parse_timing;
 
-  // Time relative to the back-forward cache. This records only the last timing
-  // of back-foward cache usages.
-  //
-  // TODO(hajimehoshi): Now only the last one is recorded, but we might miss
-  // the information when, e.g., another back-forward navigation happens before
-  // recording the first contentful paint. We should have this as an array.
-  BackForwardCacheTiming back_forward_cache_timing;
+  // List of back-forward cache timings, one for each time a page was restored
+  // from the cache.
+  array<BackForwardCacheTiming> back_forward_cache_timings;
 
   // Time between user input and navigation start. This is set for navigations
   // where the input start timing is known; currently when the navigation is
@@ -360,11 +356,10 @@ interface PageLoadMetrics {
   SubmitThroughputData(ThroughputUkmData throughput_data);
 };
 
-// TimeDeltas below relative to the navigation restoring page from the back-
-// forward cache.
+// TimeDelta below relative to the navigation start of the navigation restoring
+// page from the back- forward cache.
 struct BackForwardCacheTiming {
-  // Times when the first paint is performed after the time when the page
+  // Time when the first paint is performed after the time when the page
   // is restored from the back-forward cache.
-  array<mojo_base.mojom.TimeDelta>
-      first_paint_after_back_forward_cache_restore;
+  mojo_base.mojom.TimeDelta first_paint_after_back_forward_cache_restore;
 };

components/page_load_metrics/common/page_load_timing.cc

@@ -15,7 +15,8 @@ mojom::PageLoadTimingPtr CreatePageLoadTiming() {
                               mojom::LargestContentfulPaintTiming::New(),
                               mojom::LargestContentfulPaintTiming::New(),
                               base::nullopt, base::nullopt),
-      mojom::ParseTiming::New(), mojom::BackForwardCacheTiming::New(),
+      mojom::ParseTiming::New(),
+      std::vector<mojo::StructPtr<mojom::BackForwardCacheTiming>>{},
       base::Optional<base::TimeDelta>());
 }
 
@@ -53,10 +54,6 @@ bool IsEmpty(const page_load_metrics::mojom::ParseTiming& timing) {
          !timing.parse_blocked_on_script_execution_from_document_write_duration;
 }
 
-bool IsEmpty(const page_load_metrics::mojom::BackForwardCacheTiming& timing) {
-  return timing.first_paint_after_back_forward_cache_restore.empty();
-}
-
 bool IsEmpty(const page_load_metrics::mojom::PageLoadTiming& timing) {
   return timing.navigation_start.is_null() && !timing.response_start &&
          (!timing.document_timing ||
@@ -67,8 +64,7 @@ bool IsEmpty(const page_load_metrics::mojom::PageLoadTiming& timing) {
           page_load_metrics::IsEmpty(*timing.paint_timing)) &&
          (!timing.parse_timing ||
           page_load_metrics::IsEmpty(*timing.parse_timing)) &&
-         (!timing.back_forward_cache_timing ||
-          page_load_metrics::IsEmpty(*timing.back_forward_cache_timing));
+         timing.back_forward_cache_timings.empty();
 }
 
 void InitPageLoadTimingForTest(mojom::PageLoadTiming* timing) {
@@ -80,7 +76,7 @@ void InitPageLoadTimingForTest(mojom::PageLoadTiming* timing) {
   timing->paint_timing->experimental_largest_contentful_paint =
       mojom::LargestContentfulPaintTiming::New();
   timing->parse_timing = mojom::ParseTiming::New();
-  timing->back_forward_cache_timing = mojom::BackForwardCacheTiming::New();
+  timing->back_forward_cache_timings.clear();
 }
 
 }  // namespace page_load_metrics

components/page_load_metrics/common/page_load_timing.h

@@ -20,7 +20,6 @@ bool IsEmpty(const mojom::PaintTiming& timing);
 bool IsEmpty(const mojom::ParseTiming& timing);
 bool IsEmpty(const mojom::PageLoadTiming& timing);
 bool IsEmpty(const mojom::InteractiveTiming& timing);
-bool IsEmpty(const mojom::BackForwardCacheTiming& timing);
 
 void InitPageLoadTimingForTest(mojom::PageLoadTiming* timing);
 

components/page_load_metrics/renderer/metrics_render_frame_observer.cc

@@ -454,12 +454,15 @@ MetricsRenderFrameObserver::Timing MetricsRenderFrameObserver::GetTiming()
     for (const auto& restore_timing : restore_timings) {
       double navigation_start = restore_timing.navigation_start;
       double first_paint = restore_timing.first_paint;
-      if (!first_paint) {
-        continue;
+
+      auto back_forward_cache_timing = mojom::BackForwardCacheTiming::New();
+      if (first_paint) {
+        back_forward_cache_timing
+            ->first_paint_after_back_forward_cache_restore =
+            ClampDelta(first_paint, navigation_start);
       }
-      timing->back_forward_cache_timing
-          ->first_paint_after_back_forward_cache_restore.push_back(
-              ClampDelta(first_paint, navigation_start));
+      timing->back_forward_cache_timings.push_back(
+          std::move(back_forward_cache_timing));
     }
   }
   if (perf.FirstImagePaint() > 0.0) {