BackForwardCache: Do not record anything when the domain doesn't match the condition

If the domain does not match the condition for back-forward cache, the
back-forward cache is not used and then recording the outcome or other
things does not make sense. This CL skips to record anything in such
case.

Bug: 1004676
Change-Id: I7845598f7956bd3f02a099c9eb3053fa49d02fa1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1869229
Commit-Queue: Alexander Timin <altimin@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707449}

content/browser/back_forward_cache_browsertest.cc

@@ -3047,6 +3047,14 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTestWithDomainControlEnabled,
   // not be stored in back-forward cache.
   EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
   delete_observer_rfh_b.WaitUntilDeleted();
+
+  // 6) Go back to B.
+  web_contents()->GetController().GoBack();
+  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
+
+  // Nothing is recorded when the domain does not match.
+  ExpectOutcomeIsEmpty(FROM_HERE);
+  ExpectNotRestoredIsEmpty(FROM_HERE);
 }
 
 // Check the BackForwardCache is disabled when the WebUSB feature is used.

content/browser/frame_host/back_forward_cache_impl.h

@@ -150,6 +150,11 @@ class CONTENT_EXPORT BackForwardCacheImpl : public BackForwardCache {
   // via experiment.
   static base::TimeDelta GetTimeToLiveInBackForwardCache();
 
+  // Checks if the url's host and path matches with the |allowed_urls_| host and
+  // path. This is controlled by "allowed_websites" param on BackForwardCache
+  // feature and if the param is not set, it will allow all websites by default.
+  bool IsAllowed(const GURL& current_url);
+
   // Returns the task runner that should be used by the eviction timer.
   scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner() {
     return task_runner_for_testing_ ? task_runner_for_testing_
@@ -180,11 +185,6 @@ class CONTENT_EXPORT BackForwardCacheImpl : public BackForwardCache {
   CanStoreDocumentResult CanStoreRenderFrameHost(
       RenderFrameHostImpl* render_frame_host);
 
-  // Checks if the url's host and path matches with the |allowed_urls_| host and
-  // path. This is controlled by "allowed_websites" param on BackForwardCache
-  // feature and if the param is not set, it will allow all websites by default.
-  bool IsAllowed(const GURL& current_url);
-
   // Contains the set of stored Entries.
   // Invariant:
   // - Ordered from the most recently used to the last recently used.

content/browser/frame_host/back_forward_cache_metrics.cc

@@ -95,11 +95,12 @@ void BackForwardCacheMetrics::MainFrameDidStartNavigationToDocument() {
 }
 
 void BackForwardCacheMetrics::DidCommitNavigation(
-    NavigationRequest* navigation) {
+    NavigationRequest* navigation,
+    bool back_forward_cache_allowed) {
   bool is_history_navigation =
       navigation->GetPageTransition() & ui::PAGE_TRANSITION_FORWARD_BACK;
   if (navigation->IsInMainFrame() && !navigation->IsSameDocument()) {
-    if (is_history_navigation)
+    if (is_history_navigation && back_forward_cache_allowed)
       RecordMetricsForHistoryNavigationCommit(navigation);
     not_restored_reasons_.reset();
   }
@@ -198,10 +199,6 @@ void BackForwardCacheMetrics::RecordMetricsForHistoryNavigationCommit(
         BackForwardCacheMetrics::EvictedAfterDocumentRestoredReason::kRestored);
   }
 
-  // TODO(hajimehoshi): By |BackForwardCache::IsAllowed(navigation->GetURL())|,
-  // check whether the page matches the experiment condition, and do not record
-  // anything in this case.
-
   UMA_HISTOGRAM_ENUMERATION("BackForwardCache.HistoryNavigationOutcome",
                             outcome);
 

content/browser/frame_host/back_forward_cache_metrics.h

@@ -111,7 +111,10 @@ class BackForwardCacheMetrics
   void MainFrameDidStartNavigationToDocument();
 
   // Notifies that an associated entry has committed a navigation.
-  void DidCommitNavigation(NavigationRequest* navigation_request);
+  // |back_forward_cache_allowed| indicates whether back-forward cache is
+  // allowed for the URL of |navigation_request|.
+  void DidCommitNavigation(NavigationRequest* navigation_request,
+                           bool back_forward_cache_allowed);
 
   // Records when another navigation commits away from the most recent entry
   // associated with |this|.  This is the point in time that the previous

content/browser/frame_host/navigation_controller_impl.cc

@@ -1141,7 +1141,8 @@ bool NavigationControllerImpl::RendererDidNavigate(
         std::move(back_forward_cache_metrics));
   }
   active_entry->back_forward_cache_metrics()->DidCommitNavigation(
-      navigation_request);
+      navigation_request,
+      back_forward_cache_.IsAllowed(navigation_request->GetURL()));
 
   // Grab the corresponding FrameNavigationEntry for a few updates, but only if
   // the SiteInstance matches (to avoid updating the wrong entry by mistake).