Update the BackForwardCache::DisableForRenderFrameHost() API.

Following the discussion in:
https://chromium-review.googlesource.com/c/chromium/src/+/1827280/16/content/public/browser/back_forward_cache.h#51

The previous API was very verbose to use. A second one was added in:
https://chromium-review.googlesource.com/c/chromium/src/+/1827280

This CL removes update the first one to match the second.

Design for the new content public API
https://docs.google.com/document/d/1brlmAs1mMumbGwgwdE_M_VGR81YbArzeViI0P-Uak14

Some background on why we sometimes need to disable bfcache:
https://docs.google.com/document/d/1NjZeusdS1kyEkZyfLggndU1A6qVt0Y1sa-LRUxnMoK8

TBR=dominikn@chromium.org (for the name change in AppBanner)

Bug: 1001087
Change-Id: Idd39dff370bd45f7006318b2a1bbad9fec1baa24
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1835617
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703727}

chrome/browser/banners/app_banner_manager.cc

@@ -550,11 +550,8 @@ void AppBannerManager::DidFinishNavigation(content::NavigationHandle* handle) {
   // installable_web_app_check_result_ to kNo.
   if (installable_web_app_check_result_ != InstallableWebAppCheckResult::kNo &&
       state_ != State::INACTIVE) {
-    web_contents()
-        ->GetController()
-        .GetBackForwardCache()
-        .DisableForRenderFrameHost(handle->GetPreviousRenderFrameHostId(),
-                                   "banners::AppBannerManager");
+    content::BackForwardCache::DisableForRenderFrameHost(
+        handle->GetPreviousRenderFrameHostId(), "banners::AppBannerManager");
   }
 
   if (state_ != State::COMPLETE && state_ != State::INACTIVE)

content/browser/back_forward_cache_browsertest.cc

@@ -2394,11 +2394,8 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_TRUE(NavigateToURL(shell(), url_a));
   RenderFrameHostImpl* rfh_a = current_frame_host();
   RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);
-  web_contents()
-      ->GetController()
-      .GetBackForwardCache()
-      .DisableForRenderFrameHost(rfh_a->GetGlobalFrameRoutingId(),
-                                 "DisabledByBackForwardCacheBrowserTest");
+  BackForwardCache::DisableForRenderFrameHost(
+      rfh_a, "DisabledByBackForwardCacheBrowserTest");
 
   // 2) Navigate to B.
   EXPECT_TRUE(NavigateToURL(shell(), url_b));
@@ -2423,22 +2420,16 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
   RenderFrameHostImpl* rfh_a = current_frame_host();
   RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);
   GlobalFrameRoutingId rfh_a_id = rfh_a->GetGlobalFrameRoutingId();
-  web_contents()
-      ->GetController()
-      .GetBackForwardCache()
-      .DisableForRenderFrameHost(rfh_a_id,
-                                 "DisabledByBackForwardCacheBrowserTest");
+  BackForwardCache::DisableForRenderFrameHost(
+      rfh_a_id, "DisabledByBackForwardCacheBrowserTest");
 
   // 2) Navigate to B.
   EXPECT_TRUE(NavigateToURL(shell(), url_b));
   EXPECT_TRUE(delete_observer_rfh_a.deleted());
 
   // This should not die
-  web_contents()
-      ->GetController()
-      .GetBackForwardCache()
-      .DisableForRenderFrameHost(rfh_a_id,
-                                 "DisabledByBackForwardCacheBrowserTest");
+  BackForwardCache::DisableForRenderFrameHost(
+      rfh_a_id, "DisabledByBackForwardCacheBrowserTest");
 
   // 3) Go back to A.
   web_contents()->GetController().GoBack();
@@ -2460,7 +2451,8 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
   RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);
   RenderFrameDeletedObserver delete_observer_rfh_b(rfh_b);
 
-  BackForwardCache::DisableForRenderFrameHost(rfh_b, "test");
+  BackForwardCache::DisableForRenderFrameHost(
+      rfh_b, "DisabledByBackForwardCacheBrowserTest");
 
   // 2) Navigate to C. A and B are immediately deleted.
   EXPECT_TRUE(NavigateToURL(shell(), url_c));
@@ -2488,11 +2480,8 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
   EXPECT_TRUE(rfh_a->is_in_back_forward_cache());
   EXPECT_FALSE(rfh_a->is_evicted_from_back_forward_cache());
 
-  web_contents()
-      ->GetController()
-      .GetBackForwardCache()
-      .DisableForRenderFrameHost(rfh_a->GetGlobalFrameRoutingId(),
-                                 "DisabledByBackForwardCacheBrowserTest");
+  BackForwardCache::DisableForRenderFrameHost(
+      rfh_a, "DisabledByBackForwardCacheBrowserTest");
 
   EXPECT_TRUE(rfh_a->is_evicted_from_back_forward_cache());
   delete_observer_rfh_a.WaitUntilDeleted();

content/browser/devtools/protocol/security_handler.cc

@@ -195,12 +195,9 @@ void SecurityHandler::DidChangeVisibleSecurityState() {
 
 void SecurityHandler::DidFinishNavigation(NavigationHandle* navigation_handle) {
   if (cert_error_override_mode_ == CertErrorOverrideMode::kHandleEvents) {
-    web_contents()
-        ->GetController()
-        .GetBackForwardCache()
-        .DisableForRenderFrameHost(
-            navigation_handle->GetPreviousRenderFrameHostId(),
-            "content::protocol::SecurityHandler");
+    BackForwardCache::DisableForRenderFrameHost(
+        navigation_handle->GetPreviousRenderFrameHostId(),
+        "content::protocol::SecurityHandler");
     FlushPendingCertificateErrorNotifications();
   }
 }

content/browser/frame_host/back_forward_cache_impl.cc

@@ -406,14 +406,22 @@ void BackForwardCacheImpl::PostTaskToDestroyEvictedFrames() {
                                 weak_factory_.GetWeakPtr()));
 }
 
-void BackForwardCacheImpl::DisableForRenderFrameHost(GlobalFrameRoutingId id,
-                                                     base::StringPiece reason) {
+// static
+void BackForwardCache::DisableForRenderFrameHost(RenderFrameHost* rfh,
+                                                 base::StringPiece reason) {
+  DisableForRenderFrameHost(
+      static_cast<RenderFrameHostImpl*>(rfh)->GetGlobalFrameRoutingId(),
+      reason);
+}
+
+// static
+void BackForwardCache::DisableForRenderFrameHost(GlobalFrameRoutingId id,
+                                                 base::StringPiece reason) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   if (g_bfcache_disabled_test_observer)
     g_bfcache_disabled_test_observer->OnDisabledForFrameWithReason(id, reason);
 
-  auto* rfh = RenderFrameHostImpl::FromID(id);
-  if (rfh) {
+  if (auto* rfh = RenderFrameHostImpl::FromID(id)) {
     rfh->DisallowBackForwardCache();
 
     RenderFrameHostImpl* frame = rfh;

content/browser/frame_host/back_forward_cache_impl.h

@@ -148,9 +148,6 @@ class CONTENT_EXPORT BackForwardCacheImpl : public BackForwardCache {
     cache_size_limit_for_testing_ = cache_size_limit_for_testing;
   }
 
-  // BackForwardCache:
-  void DisableForRenderFrameHost(GlobalFrameRoutingId id,
-                                 base::StringPiece reason) override;
   void DisableForTesting(DisableForTestingReason reason) override;
 
  private:

content/public/browser/BUILD.gn

@@ -53,7 +53,6 @@ jumbo_source_set("browser_sources") {
     "authenticator_request_client_delegate.h",
     "ax_event_notification_details.cc",
     "ax_event_notification_details.h",
-    "back_forward_cache.cc",
     "back_forward_cache.h",
     "background_fetch_delegate.cc",
     "background_fetch_delegate.h",

content/public/browser/back_forward_cache.cc

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "content/public/browser/back_forward_cache.h"
-
-#include "base/strings/string_util.h"
-#include "content/public/browser/render_frame_host.h"
-#include "content/public/browser/render_process_host.h"
-#include "content/public/browser/web_contents.h"
-
-namespace content {
-
-void BackForwardCache::DisableForRenderFrameHost(
-    RenderFrameHost* render_frame_host,
-    base::StringPiece reason) {
-  content::WebContents::FromRenderFrameHost(render_frame_host)
-      ->GetController()
-      .GetBackForwardCache()
-      .DisableForRenderFrameHost(content::GlobalFrameRoutingId(
-                                     render_frame_host->GetProcess()->GetID(),
-                                     render_frame_host->GetRoutingID()),
-                                 reason);
-}
-}  // namespace content

content/public/browser/back_forward_cache.h

@@ -28,12 +28,12 @@ class RenderFrameHost;
 // All methods of this class should be called from the UI thread.
 class CONTENT_EXPORT BackForwardCache {
  public:
-  // Prevents the frame for the given |id| from entering the BackForwardCache. A
-  // document can only enter the BackForwardCache if the root frame and all its
-  // children can. This action can not be undone. Any document that is assigned
-  // to this same frame in the future will not be cached either. In practice
-  // this is not a big deal as only navigations that use a new frame can be
-  // cached.
+  // Prevents the |render_frame_host| from entering the BackForwardCache. A
+  // RenderFrameHost can only enter the BackForwardCache if the main one and all
+  // its children can. This action can not be undone. Any document that is
+  // assigned to this same RenderFrameHost in the future will not be cached
+  // either. In practice this is not a big deal as only navigations that use a
+  // new frame can be cached.
   //
   // This might be needed for example by components that listen to events via a
   // WebContentsObserver and keep some sort of per frame state, as this state
@@ -41,15 +41,17 @@ class CONTENT_EXPORT BackForwardCache {
   //
   // If the page is already in the cache an eviction is triggered.
   //
-  // |id|: If no RenderFrameHost can be found for the given id nothing happens.
+  // |render_frame_host|: non-null.
   // |reason|: Free form string to be used in logging and metrics.
-  virtual void DisableForRenderFrameHost(GlobalFrameRoutingId id,
-                                         base::StringPiece reason) = 0;
-
-  // Convenience static method for calling DisableForRenderFameHost.
   static void DisableForRenderFrameHost(RenderFrameHost* render_frame_host,
                                         base::StringPiece reason);
 
+  // Helper function to be used when it is not always possible to guarantee the
+  // |render_frame_host| to be still alive when this is called. In this case,
+  // its |id| can be used.
+  static void DisableForRenderFrameHost(GlobalFrameRoutingId id,
+                                        base::StringPiece reason);
+
   // List of reasons the BackForwardCache was disabled for a specific test. If a
   // test needs to be disabled for a reason not covered below, please add to
   // this enum.