[bfcache] Do not show autofill popups on behalf of bfcached frames

Our previous approach of disabling back-forward cache proved to be too
aggressive. Instead, this patch adds a check that the popups would not
be shown on behalf of the inactive frames and removes the old blocklist logic.

R=sreejakshetty@chromium.org,vasilii@chromium.org,fhorschig@chromium.org
BUG=1031076,1017693

Change-Id: I220eab19c17f9821d86b7be5ee83a7af7e674c2c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1995166
Commit-Queue: Alexander Timin <altimin@chromium.org>
Reviewed-by: Christos Froussios <cfroussios@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Friedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#730309}

chrome/browser/password_manager/chrome_password_manager_client.cc

@@ -1237,6 +1237,9 @@ void ChromePasswordManagerClient::FocusedInputChanged(
           web_contents(), content_driver, focused_field_type))
     return;
 
+  if (!content_driver->CanShowAutofillUi())
+    return;
+
   if (!PasswordAccessoryController::AllowedForWebContents(web_contents()))
     return;
 

chrome/browser/ui/views/autofill/payments/save_card_bubble_views_browsertest.cc

@@ -63,7 +63,6 @@
 #include "components/sync/test/fake_server/fake_server_network_resources.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents_observer.h"
-#include "content/public/test/back_forward_cache_util.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
@@ -190,8 +189,6 @@ class SaveCardBubbleViewsFullFormBrowserTest
         browser()->profile(), username, "password",
         ProfileSyncServiceHarness::SigninType::FAKE_SIGNIN);
 
-    content::BackForwardCacheDisabledTester back_forward_cache_disabled_tester;
-
     // Set up the URL loader factory for the payments client so we can intercept
     // those network requests too.
     test_shared_loader_factory_ =
@@ -204,11 +201,6 @@ class SaveCardBubbleViewsFullFormBrowserTest
         ->GetPaymentsClient()
         ->set_url_loader_factory_for_testing(test_shared_loader_factory_);
 
-    EXPECT_TRUE(back_forward_cache_disabled_tester.IsDisabledForFrameWithReason(
-        GetActiveWebContents()->GetMainFrame()->GetProcess()->GetID(),
-        GetActiveWebContents()->GetMainFrame()->GetRoutingID(),
-        "autofill::ContentAutofillDriver"));
-
     // Set up this class as the ObserverForTest implementation.
     credit_card_save_manager_ = ContentAutofillDriver::GetForRenderFrameHost(
                                     GetActiveWebContents()->GetMainFrame())

components/autofill/content/browser/content_autofill_driver.cc

@@ -20,6 +20,7 @@
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/render_widget_host_view.h"
@@ -63,14 +64,6 @@ ContentAutofillDriver::~ContentAutofillDriver() {}
 // static
 ContentAutofillDriver* ContentAutofillDriver::GetForRenderFrameHost(
     content::RenderFrameHost* render_frame_host) {
-  // With back-forward cache, the page stays alive and its mojo connections are
-  // not closed. The page would be frozen and would eventually stop doing work,
-  // but the messages can still arrive when the frame is not active. Given that
-  // autofill logic can show popups, it's problematic - prevent pages using
-  // autofill from entering back-forward cache for now to avoid it.
-  content::BackForwardCache::DisableForRenderFrameHost(
-      render_frame_host, "autofill::ContentAutofillDriver");
-
   ContentAutofillDriverFactory* factory =
       ContentAutofillDriverFactory::FromWebContents(
           content::WebContents::FromRenderFrameHost(render_frame_host));
@@ -92,6 +85,14 @@ bool ContentAutofillDriver::IsInMainFrame() const {
   return render_frame_host_->GetParent() == nullptr;
 }
 
+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");
+}
+
 ui::AXTreeID ContentAutofillDriver::GetAxTreeId() const {
   return render_frame_host_->GetAXTreeID();
 }

components/autofill/content/browser/content_autofill_driver.h

@@ -58,6 +58,7 @@ class ContentAutofillDriver : public AutofillDriver,
   // AutofillDriver:
   bool IsIncognito() const override;
   bool IsInMainFrame() const override;
+  bool CanShowAutofillUi() const override;
   ui::AXTreeID GetAxTreeId() const override;
   scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory() override;
   bool RendererIsAvailable() override;

components/autofill/core/browser/autofill_driver.h

@@ -54,6 +54,10 @@ class AutofillDriver {
   // Returns whether AutofillDriver instance is associated to the main frame.
   virtual bool IsInMainFrame() const = 0;
 
+  // Returns true iff a popup can be shown on the behalf of the associated
+  // frame.
+  virtual bool CanShowAutofillUi() const = 0;
+
   // Returns the ax tree id associated with this driver.
   virtual ui::AXTreeID GetAxTreeId() const = 0;
 

components/autofill/core/browser/autofill_external_delegate.cc

@@ -155,7 +155,7 @@ void AutofillExternalDelegate::OnSuggestionsReturned(
   }
 
   // Send to display.
-  if (query_field_.is_focusable) {
+  if (query_field_.is_focusable && GetAutofillDriver()->CanShowAutofillUi()) {
     manager_->client()->ShowAutofillPopup(
         element_bounds_, query_field_.text_direction, suggestions,
         autoselect_first_suggestion, popup_type_, GetWeakPtr());

components/autofill/core/browser/test_autofill_driver.cc

@@ -29,6 +29,10 @@ bool TestAutofillDriver::IsInMainFrame() const {
   return is_in_main_frame_;
 }
 
+bool TestAutofillDriver::CanShowAutofillUi() const {
+  return true;
+}
+
 ui::AXTreeID TestAutofillDriver::GetAxTreeId() const {
   NOTIMPLEMENTED() << "See https://crbug.com/985933";
   return ui::AXTreeIDUnknown();

components/autofill/core/browser/test_autofill_driver.h

@@ -24,6 +24,7 @@ class TestAutofillDriver : public AutofillDriver {
   // AutofillDriver implementation overrides.
   bool IsIncognito() const override;
   bool IsInMainFrame() const override;
+  bool CanShowAutofillUi() const override;
   ui::AXTreeID GetAxTreeId() const override;
   scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory() override;
   bool RendererIsAvailable() override;

components/autofill/ios/browser/autofill_driver_ios.h

@@ -40,6 +40,7 @@ class AutofillDriverIOS : public AutofillDriver {
   // AutofillDriver:
   bool IsIncognito() const override;
   bool IsInMainFrame() const override;
+  bool CanShowAutofillUi() const override;
   ui::AXTreeID GetAxTreeId() const override;
   scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory() override;
   bool RendererIsAvailable() override;

components/autofill/ios/browser/autofill_driver_ios.mm

@@ -71,6 +71,10 @@ bool AutofillDriverIOS::IsInMainFrame() const {
   return web_frame ? web_frame->IsMainFrame() : true;
 }
 
+bool AutofillDriverIOS::CanShowAutofillUi() const {
+  return true;
+}
+
 ui::AXTreeID AutofillDriverIOS::GetAxTreeId() const {
   NOTIMPLEMENTED() << "See https://crbug.com/985933";
   return ui::AXTreeIDUnknown();

components/password_manager/content/browser/content_password_manager_driver.cc

@@ -22,6 +22,7 @@
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/navigation_handle.h"
+#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/ssl_status.h"
 #include "content/public/browser/web_contents.h"
@@ -87,14 +88,6 @@ ContentPasswordManagerDriver::~ContentPasswordManagerDriver() = default;
 ContentPasswordManagerDriver*
 ContentPasswordManagerDriver::GetForRenderFrameHost(
     content::RenderFrameHost* render_frame_host) {
-  // With back-forward cache, the page stays alive and its mojo connections are
-  // not closed. The page would be frozen and would eventually stop doing work,
-  // but the messages can still arrive when the frame is not active. Given that
-  // autofill logic can show popups, it's problematic - prevent pages using
-  // autofill from entering back-forward cache for now to avoid it.
-  content::BackForwardCache::DisableForRenderFrameHost(
-      render_frame_host, "password_manager::ContentPasswordManagerDriver");
-
   ContentPasswordManagerDriverFactory* factory =
       ContentPasswordManagerDriverFactory::FromWebContents(
           content::WebContents::FromRenderFrameHost(render_frame_host));
@@ -198,6 +191,14 @@ bool ContentPasswordManagerDriver::IsMainFrame() const {
   return is_main_frame_;
 }
 
+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");
+}
+
 const GURL& ContentPasswordManagerDriver::GetLastCommittedURL() const {
   return render_frame_host_->GetLastCommittedURL();
 }

components/password_manager/content/browser/content_password_manager_driver.h

@@ -77,6 +77,7 @@ class ContentPasswordManagerDriver
   void SendLoggingAvailability() override;
   autofill::AutofillDriver* GetAutofillDriver() override;
   bool IsMainFrame() const override;
+  bool CanShowAutofillUi() const override;
   const GURL& GetLastCommittedURL() const override;
   void AnnotateFieldsWithParsingResult(
       const autofill::ParsingResult& parsing_result) override;

components/password_manager/core/browser/password_autofill_manager.cc

@@ -357,6 +357,9 @@ void PasswordAutofillManager::OnShowPasswordSuggestions(
     suggestions.push_back(suggestion);
   }
 
+  if (!password_manager_driver_->CanShowAutofillUi())
+    return;
+
   metrics_util::LogPasswordDropdownShown(
       metrics_util::PasswordDropdownState::kStandard,
       password_client_->IsIncognito());
@@ -408,6 +411,9 @@ bool PasswordAutofillManager::MaybeShowPasswordSuggestionsWithGeneration(
         metrics_util::SHOW_ALL_SAVED_PASSWORDS_CONTEXT_PASSWORD);
   }
 
+  if (!password_manager_driver_->CanShowAutofillUi())
+    return false;
+
   metrics_util::LogPasswordDropdownShown(
       metrics_util::PasswordDropdownState::kStandardGenerate,
       password_client_->IsIncognito());

components/password_manager/core/browser/password_manager_driver.h

@@ -105,6 +105,10 @@ class PasswordManagerDriver
   // Return true iff the driver corresponds to the main frame.
   virtual bool IsMainFrame() const = 0;
 
+  // Returns true iff a popup can be shown on the behalf of the associated
+  // frame.
+  virtual bool CanShowAutofillUi() const = 0;
+
   // Returns the last committed URL of the frame.
   virtual const GURL& GetLastCommittedURL() const = 0;
 

components/password_manager/core/browser/stub_password_manager_driver.cc

@@ -55,6 +55,10 @@ bool StubPasswordManagerDriver::IsMainFrame() const {
   return true;
 }
 
+bool StubPasswordManagerDriver::CanShowAutofillUi() const {
+  return true;
+}
+
 const GURL& StubPasswordManagerDriver::GetLastCommittedURL() const {
   return GURL::EmptyGURL();
 }

components/password_manager/core/browser/stub_password_manager_driver.h

@@ -33,6 +33,7 @@ class StubPasswordManagerDriver : public PasswordManagerDriver {
   PasswordAutofillManager* GetPasswordAutofillManager() override;
   autofill::AutofillDriver* GetAutofillDriver() override;
   bool IsMainFrame() const override;
+  bool CanShowAutofillUi() const override;
   const GURL& GetLastCommittedURL() const override;
 
  private:

ios/chrome/browser/passwords/ios_chrome_password_manager_driver.h

@@ -73,6 +73,7 @@ class IOSChromePasswordManagerDriver
       override;
   autofill::AutofillDriver* GetAutofillDriver() override;
   bool IsMainFrame() const override;
+  bool CanShowAutofillUi() const override;
   const GURL& GetLastCommittedURL() const override;
 
  private:

ios/chrome/browser/passwords/ios_chrome_password_manager_driver.mm

@@ -93,6 +93,10 @@ bool IOSChromePasswordManagerDriver::IsMainFrame() const {
   return true;
 }
 
+bool IOSChromePasswordManagerDriver::CanShowAutofillUi() const {
+  return true;
+}
+
 const GURL& IOSChromePasswordManagerDriver::GetLastCommittedURL() const {
   return delegate_.lastCommittedURL;
 }

ios/web_view/internal/passwords/web_view_password_manager_driver.h

@@ -62,6 +62,7 @@ class WebViewPasswordManagerDriver
       override;
   autofill::AutofillDriver* GetAutofillDriver() override;
   bool IsMainFrame() const override;
+  bool CanShowAutofillUi() const override;
   const GURL& GetLastCommittedURL() const override;
 
  private:

ios/web_view/internal/passwords/web_view_password_manager_driver.mm

@@ -82,6 +82,10 @@ bool WebViewPasswordManagerDriver::IsMainFrame() const {
   return true;
 }
 
+bool WebViewPasswordManagerDriver::CanShowAutofillUi() const {
+  return true;
+}
+
 const GURL& WebViewPasswordManagerDriver::GetLastCommittedURL() const {
   return delegate_.lastCommittedURL;
 }