[bfcache] Evict pages in the cache that request permissions

Make sure we do not show a UI for pages in the cache.

Bug: 1001087
Change-Id: I7f77f144252d36070b710506fa3a82a48e95e6f9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1939983
Commit-Queue: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720180}

chrome/browser/chrome_back_forward_cache_browsertest.cc

@@ -4,10 +4,17 @@
 
 #include "base/command_line.h"
 #include "base/macros.h"
+#include "base/test/mock_callback.h"
 #include "base/test/scoped_feature_list.h"
+#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
+#include "chrome/browser/permissions/permission_manager.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "components/content_settings/core/browser/host_content_settings_map.h"
+#include "components/content_settings/core/common/content_settings.h"
+#include "components/content_settings/core/common/content_settings_types.h"
 #include "components/network_session_configurator/common/network_switches.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/common/content_features.h"
@@ -199,3 +206,35 @@ IN_PROC_BROWSER_TEST_F(ChromeBackForwardCacheBrowserTest, WebBluetooth) {
   EXPECT_TRUE(content::NavigateToURL(web_contents(), GetURL("b.com")));
   delete_observer.WaitUntilDeleted();
 }
+
+IN_PROC_BROWSER_TEST_F(ChromeBackForwardCacheBrowserTest,
+                       PermissionContextBase) {
+  // HTTPS needed for GEOLOCATION permission
+  net::EmbeddedTestServer https_server(net::EmbeddedTestServer::TYPE_HTTPS);
+  https_server.AddDefaultHandlers(GetChromeTestDataDir());
+  https_server.SetSSLConfig(net::EmbeddedTestServer::CERT_OK);
+  ASSERT_TRUE(https_server.Start());
+
+  GURL url_a(https_server.GetURL("a.com", "/title1.html"));
+  GURL url_b(https_server.GetURL("b.com", "/title1.html"));
+
+  HostContentSettingsMapFactory::GetForProfile(browser()->profile())
+      ->SetDefaultContentSetting(ContentSettingsType::GEOLOCATION,
+                                 ContentSetting::CONTENT_SETTING_ASK);
+
+  // 1) Navigate to A.
+  EXPECT_TRUE(NavigateToURL(web_contents(), url_a));
+  content::RenderFrameHost* rfh_a = current_frame_host();
+  content::RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);
+
+  // 2) Navigate to B.
+  EXPECT_TRUE(NavigateToURL(web_contents(), url_b));
+  ASSERT_FALSE(delete_observer_rfh_a.deleted());
+  base::MockOnceCallback<void(ContentSetting)> callback;
+  EXPECT_CALL(callback, Run(ContentSetting::CONTENT_SETTING_ASK));
+  PermissionManager::Get(browser()->profile())
+      ->RequestPermission(ContentSettingsType::GEOLOCATION, rfh_a, url_a,
+                          /* user_gesture = */ true, callback.Get());
+
+  delete_observer_rfh_a.WaitUntilDeleted();
+}

chrome/browser/permissions/permission_context_base.cc

@@ -32,7 +32,9 @@
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/prefs/pref_service.h"
 #include "components/variations/variations_associated_data.h"
+#include "content/public/browser/back_forward_cache.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/global_routing_id.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
@@ -190,6 +192,15 @@ void PermissionContextBase::RequestPermission(
     return;
   }
 
+  // Make sure we do not show a UI for cached documents
+  if (content::BackForwardCache::EvictIfCached(
+          content::GlobalFrameRoutingId(id.render_process_id(),
+                                        id.render_frame_id()),
+          "PermissionContextBase::RequestPermission")) {
+    std::move(callback).Run(result.content_setting);
+    return;
+  }
+
   // We are going to show a prompt now.
   PermissionUmaUtil::PermissionRequested(content_settings_type_,
                                          requesting_origin);

content/browser/back_forward_cache_browsertest.cc

@@ -6,6 +6,7 @@
 #include "base/hash/hash.h"
 #include "base/metrics/metrics_hashes.h"
 #include "base/optional.h"
+#include "base/strings/string_piece_forward.h"
 #include "base/system/sys_info.h"
 #include "base/task/post_task.h"
 #include "base/test/bind_test_util.h"
@@ -18,6 +19,8 @@
 #include "content/browser/frame_host/frame_tree_node.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
+#include "content/public/browser/back_forward_cache.h"
+#include "content/public/browser/global_routing_id.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/site_isolation_policy.h"
@@ -4382,6 +4385,50 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
                 FROM_HERE);
 }
 
+IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
+                       EvictIfCachedIsNoopIfNotCached) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));
+
+  // 1) Navigate to A.
+  EXPECT_TRUE(NavigateToURL(shell(), url_a));
+  EXPECT_FALSE(BackForwardCache::EvictIfCached(
+      current_frame_host()->GetGlobalFrameRoutingId(), kDisabledReasonForTest));
+
+  // 2) Navigate to B.
+  EXPECT_TRUE(NavigateToURL(shell(), url_b));
+
+  // 3) Go back to A.
+  web_contents()->GetController().GoBack();
+  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
+  ExpectOutcome(BackForwardCacheMetrics::HistoryNavigationOutcome::kRestored,
+                FROM_HERE);
+}
+
+IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, EvictIfCachedDoesEvict) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));
+
+  // 1) Navigate to A.
+  EXPECT_TRUE(NavigateToURL(shell(), url_a));
+  RenderFrameDeletedObserver delete_observer_rfh_a(current_frame_host());
+  GlobalFrameRoutingId id_a = current_frame_host()->GetGlobalFrameRoutingId();
+
+  // 2) Navigate to B.
+  EXPECT_TRUE(NavigateToURL(shell(), url_b));
+  EXPECT_TRUE(BackForwardCache::EvictIfCached(id_a, kDisabledReasonForTest));
+
+  // 3) Go back to A.
+  web_contents()->GetController().GoBack();
+  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
+  ExpectDisabledWithReason(kDisabledReasonForTest, FROM_HERE);
+  ExpectNotRestored({BackForwardCacheMetrics::NotRestoredReason::
+                         kDisableForRenderFrameHostCalled},
+                    FROM_HERE);
+}
+
 // Test for functionality of memory controls in back-forward cache for low
 // memory devices.
 class BackForwardCacheBrowserTestForLowMemoryDevices

content/browser/frame_host/back_forward_cache_impl.cc

@@ -440,6 +440,20 @@ void BackForwardCache::DisableForRenderFrameHost(GlobalFrameRoutingId id,
     rfh->DisableBackForwardCache(reason);
 }
 
+// static
+bool BackForwardCache::EvictIfCached(GlobalFrameRoutingId id,
+                                     base::StringPiece reason) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  auto* rfh = RenderFrameHostImpl::FromID(id);
+  if (rfh && rfh->is_in_back_forward_cache()) {
+    BackForwardCacheCanStoreDocumentResult can_store;
+    can_store.NoDueToDisableForRenderFrameHostCalled({reason.as_string()});
+    rfh->EvictFromBackForwardCacheWithReasons(can_store);
+    return true;
+  }
+  return false;
+}
+
 void BackForwardCacheImpl::DisableForTesting(DisableForTestingReason reason) {
   is_disabled_for_testing_ = true;
 

content/public/browser/back_forward_cache.h

@@ -52,6 +52,19 @@ class CONTENT_EXPORT BackForwardCache {
   static void DisableForRenderFrameHost(GlobalFrameRoutingId id,
                                         base::StringPiece reason);
 
+  // If the RenderFrameHost referenced by |id| is in the BackForwardCache cache
+  // this method will evict it and return true. A reason can be provided for
+  // logging and metrics purposes. On the other hand, if the RenderFrameHost is
+  // not cached or it no longer exists, nothing happens and false is returned.
+  //
+  // Calling this method will not prevent this RenderFrameHost from entering the
+  // back-forward cache in the future as opposed to the
+  // DisableForRenderFrameHost methods.
+  //
+  // This method is useful to gate operations that are not allowed while a
+  // document is in the cache.
+  static bool EvictIfCached(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.