[content] Export IsInactiveAndDisallowReactivation to content/public

Remove BackForwardCache::EvictIfCached which is being used
to check if the RenderFrameHost is in cache or not and evict it
accordingly.

For this, we already have IsInactiveAndDisallowReactivation,
which would be a more correct fit as we also check for other
RenderFrameHostImpl::lifecycle_state() for inactivity.

In this CL, we replace the usages in Autofill, PasswordManager
with IsCurrent() and in PermissionContextBase with
IsInactiveAndDisallowReactivation() which should only allow showing
UI when the frame is active.

BUG=1041021

Change-Id: I170fd671fb0c9c9c9979bbc2e78b0d8a9f18afa2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2295819
Commit-Queue: Sreeja Kamishetty <sreejakshetty@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789016}

components/autofill/content/browser/content_autofill_driver.cc

@@ -86,11 +86,10 @@ bool ContentAutofillDriver::IsInMainFrame() const {
 }
 
 bool ContentAutofillDriver::CanShowAutofillUi() const {
-  // TODO(crbug.com/1041021): Use RenderFrameHost::IsActive here when available.
-  return !content::BackForwardCache::EvictIfCached(
-      {render_frame_host_->GetProcess()->GetID(),
-       render_frame_host_->GetRoutingID()},
-      "ContentAutofillDriver::CanShowAutofillUi");
+  // Don't show AutofillUi for non-current RenderFrameHost. Here it is safe to
+  // ignore the calls from inactive RFH as the renderer is not expecting a reply
+  // and it doesn't lead to browser-renderer consistency issues.
+  return render_frame_host_->IsCurrent();
 }
 
 ui::AXTreeID ContentAutofillDriver::GetAxTreeId() const {

components/password_manager/content/browser/content_password_manager_driver.cc

@@ -195,11 +195,8 @@ bool ContentPasswordManagerDriver::IsMainFrame() const {
 }
 
 bool ContentPasswordManagerDriver::CanShowAutofillUi() const {
-  // TODO(crbug.com/1041021): Use RenderFrameHost::IsActive here when available.
-  return !content::BackForwardCache::EvictIfCached(
-      {render_frame_host_->GetProcess()->GetID(),
-       render_frame_host_->GetRoutingID()},
-      "ContentPasswordManagerDriver::CanShowAutofillUi");
+  // Don't show AutofillUi for non-current RenderFrameHost.
+  return render_frame_host_->IsCurrent();
 }
 
 const GURL& ContentPasswordManagerDriver::GetLastCommittedURL() const {

components/permissions/permission_context_base.cc

@@ -188,11 +188,12 @@ 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")) {
+  // Don't show request permission UI for an inactive RenderFrameHost. If this
+  // is called when RenderFrameHost is in BackForwardCache, evict the document
+  // as the page might not distinguish properly between user denying the
+  // permission and automatic rejection, leading to an inconsistent UX after
+  // restoring the page from the cache.
+  if (rfh->IsInactiveAndDisallowReactivation()) {
     std::move(callback).Run(result.content_setting);
     return;
   }

content/browser/back_forward_cache_browsertest.cc

@@ -5392,15 +5392,14 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
 }
 
 IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
-                       EvictIfCachedIsNoopIfNotCached) {
+                       IsInactiveAndDisallowReactivationIsNoopWhenActive) {
   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));
+  EXPECT_FALSE(current_frame_host()->IsInactiveAndDisallowReactivation());
 
   // 2) Navigate to B.
   EXPECT_TRUE(NavigateToURL(shell(), url_b));
@@ -5412,27 +5411,28 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
                 FROM_HERE);
 }
 
-IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, EvictIfCachedDoesEvict) {
+IN_PROC_BROWSER_TEST_F(
+    BackForwardCacheBrowserTest,
+    IsInactiveAndDisallowReactivationDoesEvictForCachedFrames) {
   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();
+  RenderFrameHostImpl* rfh_a = current_frame_host();
 
   // 2) Navigate to B.
   EXPECT_TRUE(NavigateToURL(shell(), url_b));
-  EXPECT_TRUE(BackForwardCache::EvictIfCached(id_a, kDisabledReasonForTest));
+  EXPECT_TRUE(rfh_a->IsInactiveAndDisallowReactivation());
 
   // 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);
+  ExpectNotRestored(
+      {BackForwardCacheMetrics::NotRestoredReason::kIgnoreEventAndEvict},
+      FROM_HERE);
 }
 
 // Test for functionality of memory controls in back-forward cache for low

content/browser/frame_host/back_forward_cache_impl.cc

@@ -526,20 +526,6 @@ 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->IsInBackForwardCache()) {
-    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/browser/frame_host/render_frame_host_impl.h

@@ -326,6 +326,7 @@ class CONTENT_EXPORT RenderFrameHostImpl
   bool IsRenderFrameCreated() override;
   bool IsRenderFrameLive() override;
   bool IsCurrent() override;
+  bool IsInactiveAndDisallowReactivation() override;
   size_t GetProxyCount() override;
   bool HasSelection() override;
   void RequestTextSurroundingSelection(
@@ -394,27 +395,6 @@ class CONTENT_EXPORT RenderFrameHostImpl
   void SendAccessibilityEventsToManager(
       const AXEventNotificationDetails& details);
 
-  // Returns true iff the RenderFrameHost is inactive i.e., when the
-  // |lifecycle_state_| of this RenderFrameHost is in either kInBackForwardCache
-  // or kRunningUnloadHandlers or kReadyToBeDeleted states. This function should
-  // be used when we are unsure if inactive RenderFrameHost can be properly
-  // handled and their processing shouldn't be deferred until the
-  // RenderFrameHost becomes active again.
-  //
-  // This method additionally has a side effect for back-forward cache: it
-  // disallows reactivating by evicting the document from the cache and
-  // triggering deletion. This avoids reactivating the frame as restoring would
-  // be unsafe after dropping an event, which means that the frame will never be
-  // shown to the user again and the event can be safely ignored.
-  //
-  // Note that if |IsInactiveAndDisallowReactivation()| returns false, then
-  // IsCurrent() returns false as well.
-  // We shouldn't be calling this for a speculative RenderFrameHost as we don't
-  // want to support disallowing activation before the document became active
-  // for the first time. In that case |IsInactiveAndDisallowReactivation()|
-  // returns false along with terminating the renderer process.
-  bool IsInactiveAndDisallowReactivation();
-
   void EvictFromBackForwardCacheWithReason(
       BackForwardCacheMetrics::NotRestoredReason reason);
   void EvictFromBackForwardCacheWithReasons(

content/public/browser/back_forward_cache.h

@@ -52,19 +52,6 @@ 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.

content/public/browser/render_frame_host.h

@@ -362,6 +362,27 @@ class CONTENT_EXPORT RenderFrameHost : public IPC::Listener,
   // children.
   virtual bool IsCurrent() = 0;
 
+  // Returns true iff the RenderFrameHost is inactive i.e., when the
+  // RenderFrameHost is either in BackForwardCache or pending deletion. This
+  // function should be used when we are unsure if inactive RenderFrameHosts can
+  // be properly handled and their processing shouldn't be deferred until the
+  // RenderFrameHost becomes active again. Callers that only want to check
+  // whether a RenderFrameHost is current or not should use IsCurrent() instead.
+  //
+  // This method additionally has a side effect for back-forward cache: it
+  // disallows reactivating by evicting the document from the cache and
+  // triggering deletion. This avoids reactivating the frame as restoring would
+  // be unsafe after dropping an event, which means that the frame will never be
+  // shown to the user again and the event can be safely ignored.
+  //
+  // Note that if |IsInactiveAndDisallowReactivation()| returns false, then
+  // IsCurrent() returns false as well.
+  // This should not be called for speculative RenderFrameHosts as disallowing
+  // reactivation before the document became active for the first time is not
+  // supported. In that case |IsInactiveAndDisallowReactivation()|
+  // returns false along with terminating the renderer process.
+  virtual bool IsInactiveAndDisallowReactivation() = 0;
+
   // Get the number of proxies to this frame, in all processes. Exposed for
   // use by resource metrics.
   virtual size_t GetProxyCount() = 0;