BackForwardCache: Persist PageLoadTrackers for pages in back-forward cache.

When we are restoring the page from bfcache, preserving its state,
corresponding PageLoadTracker should be preserved too.

BUG=1001087
BUG=1014174

Change-Id: Ic207ebfeb09d867e531a94d411efc520e30b5e60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2007095Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756942}

components/page_load_metrics/browser/metrics_web_contents_observer.cc

@@ -17,8 +17,10 @@
 #include "components/page_load_metrics/browser/page_load_metrics_util.h"
 #include "components/page_load_metrics/browser/page_load_tracker.h"
 #include "components/page_load_metrics/common/page_load_timing.h"
+#include "content/public/browser/back_forward_cache.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/global_request_id.h"
+#include "content/public/browser/global_routing_id.h"
 #include "content/public/browser/media_player_id.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_handle.h"
@@ -151,6 +153,14 @@ void MetricsWebContentsObserver::FrameDeleted(content::RenderFrameHost* rfh) {
     committed_load_->FrameDeleted(rfh);
 }
 
+void MetricsWebContentsObserver::RenderFrameDeleted(
+    content::RenderFrameHost* rfh) {
+  // PageLoadTracker can be associated only with a main frame.
+  if (rfh->GetParent())
+    return;
+  back_forward_cached_pages_.erase(rfh);
+}
+
 void MetricsWebContentsObserver::MediaStartedPlaying(
     const content::WebContentsObserver::MediaPlayerInfo& video_type,
     const content::MediaPlayerId& id) {
@@ -443,7 +453,14 @@ void MetricsWebContentsObserver::DidFinishNavigation(
     // the currently committed navigation.
     FinalizeCurrentlyCommittedLoad(navigation_handle,
                                    navigation_handle_tracker.get());
-    committed_load_.reset();
+    // Transfers the ownership of |committed_load_|. This |committed_load_|
+    // might be reused later when restoring the page from the cache.
+    MaybeStorePageLoadTrackerForBackForwardCache(navigation_handle,
+                                                 std::move(committed_load_));
+    // If |navigation_handle| is a back-forward cache navigation, an associated
+    // PageLoadTracker is restored into |committed_load_|.
+    if (MaybeRestorePageLoadTrackerForBackForwardCache(navigation_handle))
+      return;
   }
 
   if (!navigation_handle_tracker)
@@ -502,6 +519,54 @@ void MetricsWebContentsObserver::HandleCommittedNavigationForTrackedLoad(
     observer.OnCommit(committed_load_.get());
 }
 
+void MetricsWebContentsObserver::MaybeStorePageLoadTrackerForBackForwardCache(
+    content::NavigationHandle* next_navigation_handle,
+    std::unique_ptr<PageLoadTracker> previously_committed_load) {
+  if (!previously_committed_load)
+    return;
+
+  content::RenderFrameHost* previous_frame = content::RenderFrameHost::FromID(
+      next_navigation_handle->GetPreviousRenderFrameHostId());
+
+  // The PageLoadTracker is associated with a bfcached document if:
+  bool is_back_forward_cache =
+      // 1. the frame being navigated away from was not already deleted
+      previous_frame &&
+      // 2. the previous frame is in the BFCache
+      previous_frame->IsInBackForwardCache();
+
+  if (!is_back_forward_cache)
+    return;
+
+  previously_committed_load->OnEnterBackForwardCache();
+
+  back_forward_cached_pages_.emplace(previous_frame,
+                                     std::move(previously_committed_load));
+}
+
+bool MetricsWebContentsObserver::MaybeRestorePageLoadTrackerForBackForwardCache(
+    content::NavigationHandle* navigation_handle) {
+  if (!navigation_handle->IsServedFromBackForwardCache())
+    return false;
+
+  auto it =
+      back_forward_cached_pages_.find(navigation_handle->GetRenderFrameHost());
+
+  // There are some cases that the PageLoadTracker does not exist. For example,
+  // if a page is put into the cache before MetricsWebContents is created,
+  // |back_forward_cached_pages_| is empty.
+  if (it == back_forward_cached_pages_.end())
+    return false;
+
+  committed_load_ = std::move(it->second);
+  back_forward_cached_pages_.erase(it);
+  // TODO(hajimehoshi): Add dedicated methods to PageLoadMetricsTracker to allow
+  // them to observe pages being restored from bfcache.
+  if (web_contents()->GetVisibility() == content::Visibility::VISIBLE)
+    committed_load_->PageShown();
+  return true;
+}
+
 void MetricsWebContentsObserver::FinalizeCurrentlyCommittedLoad(
     content::NavigationHandle* newly_committed_navigation,
     PageLoadTracker* newly_committed_navigation_tracker) {
@@ -527,11 +592,6 @@ void MetricsWebContentsObserver::FinalizeCurrentlyCommittedLoad(
     if (is_non_user_initiated_client_redirect) {
       committed_load_->NotifyClientRedirectTo(newly_committed_navigation);
     }
-
-    content::RenderFrameHost* rfh = content::RenderFrameHost::FromID(
-        newly_committed_navigation->GetPreviousRenderFrameHostId());
-    if (rfh && rfh->IsInBackForwardCache())
-      committed_load_->OnEnterBackForwardCache();
   }
 }
 
@@ -777,6 +837,8 @@ bool MetricsWebContentsObserver::ShouldTrackMainFrameNavigation(
   if (embedder_interface_->IsNewTabPageUrl(navigation_handle->GetURL()))
     return false;
 
+  // The navigation served from the back-forward cache will use the previously
+  // created tracker for the document.
   if (navigation_handle->IsServedFromBackForwardCache())
     return false;
 

components/page_load_metrics/browser/metrics_web_contents_observer.h

@@ -108,6 +108,7 @@ class MetricsWebContentsObserver
   void RenderViewHostChanged(content::RenderViewHost* old_host,
                              content::RenderViewHost* new_host) override;
   void FrameDeleted(content::RenderFrameHost* render_frame_host) override;
+  void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
   void MediaStartedPlaying(
       const content::WebContentsObserver::MediaPlayerInfo& video_type,
       const content::MediaPlayerId& id) override;
@@ -238,6 +239,20 @@ class MetricsWebContentsObserver
   void OnBrowserFeatureUsage(content::RenderFrameHost* render_frame_host,
                              const mojom::PageLoadFeatures& new_features);
 
+  // Before deleting PageLoadTracker, check if we need to keep it alive as the
+  // page is stored in back-forward cache. The page can either be restored later
+  // (we will be notified via DidFinishNavigation and NavigationHandle::
+  // IsServedFromBackForwardCache) or will be evicted from the cache (we will be
+  // notified via RenderFrameDeleted).
+  void MaybeStorePageLoadTrackerForBackForwardCache(
+      content::NavigationHandle* next_navigation_handle,
+      std::unique_ptr<PageLoadTracker> page_load_tracker);
+
+  // Try to restore a PageLoadTracker when a navigation restores corresponding
+  // page from back-forward cache. Returns true if the page was restored.
+  bool MaybeRestorePageLoadTrackerForBackForwardCache(
+      content::NavigationHandle* navigation_handle);
+
   // True if the web contents is currently in the foreground.
   bool in_foreground_;
 
@@ -261,6 +276,14 @@ class MetricsWebContentsObserver
 
   std::unique_ptr<PageLoadTracker> committed_load_;
 
+  // A page can be stored in back-forward cache - in this case its
+  // PageLoadTracker should be preserved as well. Here we store PageLoadTracker
+  // for each main frame that we navigated away from until we are notified that
+  // it is deleted (would happen almost immediately if back-forward cache is not
+  // enabled or page is not stored).
+  base::flat_map<content::RenderFrameHost*, std::unique_ptr<PageLoadTracker>>
+      back_forward_cached_pages_;
+
   // Has the MWCO observed at least one navigation?
   bool has_navigated_;
 

components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc

@@ -1460,6 +1460,22 @@ TEST_F(MetricsWebContentsObserverTest, RecordFeatureUsageNoObserver) {
 
 class MetricsWebContentsObserverBackForwardCacheTest
     : public MetricsWebContentsObserverTest {
+  class CreatedPageLoadTrackerObserver
+      : public MetricsWebContentsObserver::TestingObserver {
+   public:
+    explicit CreatedPageLoadTrackerObserver(content::WebContents* web_contents)
+        : MetricsWebContentsObserver::TestingObserver(web_contents) {}
+
+    int tracker_committed_count() const { return tracker_committed_count_; }
+
+    void OnCommit(PageLoadTracker* tracker) override {
+      tracker_committed_count_++;
+    }
+
+   private:
+    int tracker_committed_count_ = 0;
+  };
+
  public:
   MetricsWebContentsObserverBackForwardCacheTest() {
     feature_list_.InitWithFeaturesAndParameters(
@@ -1470,8 +1486,21 @@ class MetricsWebContentsObserverBackForwardCacheTest
 
   ~MetricsWebContentsObserverBackForwardCacheTest() override = default;
 
+  int tracker_committed_count() const {
+    return created_page_load_tracker_observer_->tracker_committed_count();
+  }
+
+  void SetUp() override {
+    MetricsWebContentsObserverTest::SetUp();
+    created_page_load_tracker_observer_ =
+        std::make_unique<CreatedPageLoadTrackerObserver>(web_contents());
+    observer()->AddTestingObserver(created_page_load_tracker_observer_.get());
+  }
+
  private:
   base::test::ScopedFeatureList feature_list_;
+  std::unique_ptr<CreatedPageLoadTrackerObserver>
+      created_page_load_tracker_observer_;
 };
 
 TEST_F(MetricsWebContentsObserverBackForwardCacheTest,
@@ -1514,24 +1543,36 @@ TEST_F(MetricsWebContentsObserverBackForwardCacheTest, EnterBackForwardCache) {
 
   ASSERT_EQ(0, CountCompleteTimingReported());
   EXPECT_EQ(0, CountOnBackForwardCacheEntered());
+  EXPECT_EQ(1, tracker_committed_count());
 
   // Go to the URL2.
   content::NavigationSimulator::NavigateAndCommitFromBrowser(
       web_contents(), GURL(kDefaultTestUrl2));
   ASSERT_EQ(main_rfh()->GetLastCommittedURL().spec(), GURL(kDefaultTestUrl2));
 
+  // With the default implementation of PageLoadMetricsObserver,
+  // OnEnteringBackForwardCache invokes OnComplete and returns STOP_OBSERVING.
   ASSERT_EQ(1, CountCompleteTimingReported());
   EXPECT_EQ(1, CountOnBackForwardCacheEntered());
+  EXPECT_EQ(2, tracker_committed_count());
 
   // Go back.
   content::NavigationSimulator::GoBack(web_contents());
   EXPECT_EQ(2, CountOnBackForwardCacheEntered());
 
-  // With the default implementation of PageLoadMetricsObserver,
-  // OnEnteringBackForwardCache invokes OnComplete and returns STOP_OBSERVING.
+  // Again, OnComplete is assured to be called.
   ASSERT_EQ(2, CountCompleteTimingReported());
+
+  // A new page load tracker is not created or committed. A page load tracker in
+  // the cache is used instead.
+  EXPECT_EQ(2, tracker_committed_count());
 }
 
+// TODO(hajimehoshi): Detect the document eviction so that PageLoadTracker in
+// the cache is destroyed. This would call PageLoadMetricsObserver::OnComplete.
+// This test can be implemented after a PageLoadMetricsObserver's
+// OnEnterBackForwardCache returns CONTINUE_OBSERVING.
+
 class MetricsWebContentsObserverBackForwardCacheDisabledTest
     : public MetricsWebContentsObserverTest {
  public: