Revert "Remove AutofillProvider reference from ContentAutofillDriver"

This reverts commit 5654262. Reason for revert: https://crbug.com/1205788 Original change's description: > Remove AutofillProvider reference from ContentAutofillDriver > > Add factory callback, so ContentAutofillDriver doesn't need to pass > AutofillProvider to AndroidAutofillManager. > > Test: existing tests > Bug: 1200511 > Change-Id: Id4097e54be45320b5649af1bca9f436b0d587a23 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2862142 > Reviewed-by: Dominic Battré <battre@chromium.org> > Reviewed-by: Scott Violet <sky@chromium.org> > Reviewed-by: Bo <boliu@chromium.org> > Commit-Queue: Michael Bai <michaelbai@chromium.org> > Cr-Commit-Position: refs/heads/master@{#879224} Bug: 1200511 Change-Id: I8a7816f0dc64dbfbaf7328f5222e57e3fb301ed3 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2874453 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Commit-Queue: Antonio Sartori <antoniosartori@chromium.org> Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Antonio Sartori <antoniosartori@chromium.org> Owners-Override: Arthur Sonzogni <arthursonzogni@chromium.org> Auto-Submit: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#879294}
qweri0p · May 5, 2021 · f5fc947 · f5fc947
1 parent 85d2e40
commit f5fc947
Show file tree

Hide file tree

Showing 12 changed files with 87 additions and 100 deletions.
diff --git a/android_webview/browser/aw_contents.cc b/android_webview/browser/aw_contents.cc
@@ -63,7 +63,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/autofill/android/provider/autofill_provider_android.h"
 #include "components/autofill/content/browser/content_autofill_driver_factory.h"
-#include "components/autofill/core/browser/android_autofill_manager.h"
+#include "components/autofill/core/browser/browser_autofill_manager.h"
 #include "components/autofill/core/browser/webdata/autofill_webdata_service.h"
 #include "components/autofill/core/common/autofill_features.h"
 #include "components/navigation_interception/intercept_navigation_delegate.h"
@@ -327,8 +327,7 @@ void AwContents::InitAutofillIfNecessary(bool autocomplete_enabled) {
                    is_download_manager_disabled_for_testing())
           ? autofill::AutofillManager::ENABLE_AUTOFILL_DOWNLOAD_MANAGER
           : autofill::AutofillManager::DISABLE_AUTOFILL_DOWNLOAD_MANAGER,
-      base::BindRepeating(&autofill::AndroidAutofillManager::Create,
-                          autofill_provider_.get()));
+      autofill_provider_.get());
 }
 
 void AwContents::SetAwAutofillClient(const JavaRef<jobject>& client) {

diff --git a/chrome/browser/autofill/autofill_provider_browsertest.cc b/chrome/browser/autofill/autofill_provider_browsertest.cc
@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 #include "base/base_switches.h"
-#include "base/bind.h"
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "chrome/browser/ui/browser.h"
@@ -13,7 +12,6 @@
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/autofill/content/browser/content_autofill_driver.h"
 #include "components/autofill/content/browser/content_autofill_driver_factory.h"
-#include "components/autofill/core/browser/android_autofill_manager.h"
 #include "components/autofill/core/browser/test_autofill_client.h"
 #include "components/autofill/core/browser/test_autofill_provider.h"
 #include "components/autofill/core/common/autofill_features.h"
@@ -121,8 +119,18 @@ class AutofillProviderBrowserTest : public InProcessBrowserTest {
     ContentAutofillDriverFactory::CreateForWebContentsAndDelegate(
         web_contents, autofill_client_.get(), "en-US",
         BrowserAutofillManager::DISABLE_AUTOFILL_DOWNLOAD_MANAGER,
-        base::BindRepeating(&AndroidAutofillManager::Create,
-                            autofill_provider_.get()));
+        autofill_provider_.get());
+  }
+
+  void ReplaceAutofillDriver() {
+    content::WebContents* web_contents = WebContents();
+    // Set AutofillProvider for current WebContents.
+    ContentAutofillDriverFactory* factory =
+        ContentAutofillDriverFactory::FromWebContents(web_contents);
+    ContentAutofillDriver* driver =
+        factory->DriverForFrame(web_contents->GetMainFrame());
+    driver->SetAutofillProviderForTesting(autofill_provider_.get(),
+                                          autofill_client_.get());
   }
 
   void TearDownOnMainThread() override {
@@ -158,6 +166,7 @@ class AutofillProviderBrowserTest : public InProcessBrowserTest {
   }
 
   void SetLabelChangeExpectationAndTriggerQuery() {
+    ReplaceAutofillDriver();
     // One query for the single click, and a second query when the typing is
     // simulated.
     base::RunLoop run_loop;

diff --git a/chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc b/chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc
@@ -21,7 +21,6 @@
 #include "components/autofill/content/browser/content_autofill_driver.h"
 #include "components/autofill/content/browser/content_autofill_driver_factory.h"
 #include "components/autofill/core/browser/autofill_external_delegate.h"
-#include "components/autofill/core/browser/autofill_manager.h"
 #include "components/autofill/core/browser/autofill_test_utils.h"
 #include "components/autofill/core/browser/browser_autofill_manager.h"
 #include "components/autofill/core/browser/test_autofill_client.h"
@@ -78,12 +77,11 @@ class MockAutofillClient : public autofill::TestAutofillClient {
 class MockAutofillDriver : public ContentAutofillDriver {
  public:
   MockAutofillDriver(content::RenderFrameHost* rfh, MockAutofillClient* client)
-      : ContentAutofillDriver(
-            rfh,
-            client,
-            kAppLocale,
-            kDownloadState,
-            AutofillManager::AutofillManagerFactoryCallback()) {}
+      : ContentAutofillDriver(rfh,
+                              client,
+                              kAppLocale,
+                              kDownloadState,
+                              nullptr) {}
 
   ~MockAutofillDriver() override = default;
   MOCK_CONST_METHOD0(GetAxTreeId, ui::AXTreeID());

diff --git a/components/autofill/content/browser/content_autofill_driver.cc b/components/autofill/content/browser/content_autofill_driver.cc
@@ -4,7 +4,6 @@
 
 #include "components/autofill/content/browser/content_autofill_driver.h"
 
-#include <memory>
 #include <tuple>
 #include <utility>
 #include <vector>
@@ -13,6 +12,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "build/build_config.h"
 #include "components/autofill/content/browser/content_autofill_driver_factory.h"
+#include "components/autofill/core/browser/android_autofill_manager.h"
 #include "components/autofill/core/browser/autofill_client.h"
 #include "components/autofill/core/browser/browser_autofill_manager.h"
 #include "components/autofill/core/browser/form_structure.h"
@@ -64,21 +64,15 @@ ContentAutofillDriver::ContentAutofillDriver(
     AutofillClient* client,
     const std::string& app_locale,
     AutofillManager::AutofillDownloadManagerState enable_download_manager,
-    AutofillManager::AutofillManagerFactoryCallback
-        autofill_manager_factory_callback)
+    AutofillProvider* provider)
     : render_frame_host_(render_frame_host),
       browser_autofill_manager_(nullptr),
       key_press_handler_manager_(this),
       log_manager_(client->GetLogManager()) {
-  // AutofillManager isn't used if provider is valid, Autofill provider is
-  // currently used by Android WebView only.
-  if (autofill_manager_factory_callback) {
-    autofill_manager_ = autofill_manager_factory_callback.Run(
-        this, client, app_locale, enable_download_manager);
-    GetAutofillAgent()->SetUserGestureRequired(false);
-    GetAutofillAgent()->SetSecureContextRequired(true);
-    GetAutofillAgent()->SetFocusRequiresScroll(false);
-    GetAutofillAgent()->SetQueryPasswordSuggestion(true);
+  // BrowserAutofillManager isn't used if provider is valid, Autofill provider
+  // is currently used by Android WebView only.
+  if (provider) {
+    SetAutofillProvider(provider, client, enable_download_manager);
   } else {
     SetBrowserAutofillManager(std::make_unique<BrowserAutofillManager>(
         this, client, app_locale, enable_download_manager));
@@ -177,11 +171,11 @@ void ContentAutofillDriver::SendFormDataToRenderer(
 
 void ContentAutofillDriver::PropagateAutofillPredictions(
     const std::vector<FormStructure*>& forms) {
-  AutofillManager* manager = browser_autofill_manager_
+  AutofillManager* handler = browser_autofill_manager_
                                  ? browser_autofill_manager_
                                  : autofill_manager_.get();
-  DCHECK(manager);
-  manager->PropagateAutofillPredictions(render_frame_host_, forms);
+  DCHECK(handler);
+  handler->PropagateAutofillPredictions(render_frame_host_, forms);
 }
 
 void ContentAutofillDriver::HandleParsedForms(
@@ -377,7 +371,7 @@ void ContentAutofillDriver::DidNavigateFrame(
   ShowOfferNotificationIfApplicable(navigation_handle);
 
   // When IsServedFromBackForwardCache, the form data is not parsed
-  // again. So, we should keep and use the autofill manager's
+  // again. So, we should keep and use the autofill handler's
   // form_structures from BFCache for form submit.
   if (navigation_handle->IsServedFromBackForwardCache())
     return;
@@ -435,6 +429,18 @@ void ContentAutofillDriver::RemoveHandler(
   view->GetRenderWidgetHost()->RemoveKeyPressEventCallback(handler);
 }
 
+void ContentAutofillDriver::SetAutofillProvider(
+    AutofillProvider* provider,
+    AutofillClient* client,
+    AutofillManager::AutofillDownloadManagerState enable_download_manager) {
+  autofill_manager_ = std::make_unique<AndroidAutofillManager>(
+      this, client, provider, enable_download_manager);
+  GetAutofillAgent()->SetUserGestureRequired(false);
+  GetAutofillAgent()->SetSecureContextRequired(true);
+  GetAutofillAgent()->SetFocusRequiresScroll(false);
+  GetAutofillAgent()->SetQueryPasswordSuggestion(true);
+}
+
 bool ContentAutofillDriver::DocumentUsedWebOTP() const {
   return render_frame_host_->DocumentUsedWebOTP();
 }
@@ -475,6 +481,16 @@ void ContentAutofillDriver::ReportAutofillWebOTPMetrics(
       static_cast<PhoneCollectionMetricState>(phone_collection_metric_state_));
 }
 
+void ContentAutofillDriver::SetAutofillProviderForTesting(
+    AutofillProvider* provider,
+    AutofillClient* client) {
+  SetAutofillProvider(provider, client,
+                      AutofillManager::AutofillDownloadManagerState::
+                          DISABLE_AUTOFILL_DOWNLOAD_MANAGER);
+  // BrowserAutofillManager isn't used if provider is valid.
+  browser_autofill_manager_ = nullptr;
+}
+
 void ContentAutofillDriver::ShowOfferNotificationIfApplicable(
     content::NavigationHandle* navigation_handle) {
   if (!navigation_handle->IsInMainFrame())

diff --git a/components/autofill/content/browser/content_autofill_driver.h b/components/autofill/content/browser/content_autofill_driver.h
@@ -29,6 +29,7 @@ class RenderFrameHost;
 namespace autofill {
 
 class AutofillClient;
+class AutofillProvider;
 class LogManager;
 
 // Use <Phone><WebOTP><OTC> as the bit pattern to identify the metrics state.
@@ -62,8 +63,7 @@ class ContentAutofillDriver : public AutofillDriver,
       AutofillClient* client,
       const std::string& app_locale,
       AutofillManager::AutofillDownloadManagerState enable_download_manager,
-      AutofillManager::AutofillManagerFactoryCallback
-          autofill_manager_factory_callback);
+      AutofillProvider* provider);
   ~ContentAutofillDriver() override;
 
   // Gets the driver for |render_frame_host|.
@@ -159,6 +159,9 @@ class ContentAutofillDriver : public AutofillDriver,
       const content::RenderWidgetHost::KeyPressEventCallback& handler);
   void RemoveKeyPressHandler();
 
+  void SetAutofillProviderForTesting(AutofillProvider* provider,
+                                     AutofillClient* client);
+
   // Sets the manager to |manager|. Takes ownership of |manager|.
   void SetBrowserAutofillManager(
       std::unique_ptr<BrowserAutofillManager> manager);
@@ -185,6 +188,11 @@ class ContentAutofillDriver : public AutofillDriver,
   void RemoveHandler(
       const content::RenderWidgetHost::KeyPressEventCallback& handler) override;
 
+  void SetAutofillProvider(
+      AutofillProvider* provider,
+      AutofillClient* client,
+      AutofillManager::AutofillDownloadManagerState enable_download_manager);
+
   // Returns whether navigator.credentials.get({otp: {transport:"sms"}}) has
   // been used.
   bool DocumentUsedWebOTP() const;

diff --git a/components/autofill/content/browser/content_autofill_driver_factory.cc b/components/autofill/content/browser/content_autofill_driver_factory.cc
@@ -30,11 +30,9 @@ std::unique_ptr<AutofillDriver> CreateDriver(
     const std::string& app_locale,
     BrowserAutofillManager::AutofillDownloadManagerState
         enable_download_manager,
-    AutofillManager::AutofillManagerFactoryCallback
-        autofill_manager_factory_callback) {
+    AutofillProvider* provider) {
   return std::make_unique<ContentAutofillDriver>(
-      render_frame_host, client, app_locale, enable_download_manager,
-      std::move(autofill_manager_factory_callback));
+      render_frame_host, client, app_locale, enable_download_manager, provider);
 }
 
 }  // namespace
@@ -52,9 +50,8 @@ void ContentAutofillDriverFactory::CreateForWebContentsAndDelegate(
     const std::string& app_locale,
     BrowserAutofillManager::AutofillDownloadManagerState
         enable_download_manager) {
-  CreateForWebContentsAndDelegate(
-      contents, client, app_locale, enable_download_manager,
-      AutofillManager::AutofillManagerFactoryCallback());
+  CreateForWebContentsAndDelegate(contents, client, app_locale,
+                                  enable_download_manager, nullptr);
 }
 
 void ContentAutofillDriverFactory::CreateForWebContentsAndDelegate(
@@ -63,16 +60,14 @@ void ContentAutofillDriverFactory::CreateForWebContentsAndDelegate(
     const std::string& app_locale,
     BrowserAutofillManager::AutofillDownloadManagerState
         enable_download_manager,
-    AutofillManager::AutofillManagerFactoryCallback
-        autofill_manager_factory_callback) {
+    AutofillProvider* provider) {
   if (FromWebContents(contents))
     return;
 
   contents->SetUserData(
       kContentAutofillDriverFactoryWebContentsUserDataKey,
       std::make_unique<ContentAutofillDriverFactory>(
-          contents, client, app_locale, enable_download_manager,
-          std::move(autofill_manager_factory_callback)));
+          contents, client, app_locale, enable_download_manager, provider));
 }
 
 // static
@@ -112,25 +107,23 @@ ContentAutofillDriverFactory::ContentAutofillDriverFactory(
     const std::string& app_locale,
     BrowserAutofillManager::AutofillDownloadManagerState
         enable_download_manager,
-    AutofillManager::AutofillManagerFactoryCallback
-        autofill_manager_factory_callback)
+    AutofillProvider* provider)
     : AutofillDriverFactory(client),
       content::WebContentsObserver(web_contents),
       app_locale_(app_locale),
       enable_download_manager_(enable_download_manager),
-      autofill_manager_factory_callback_(
-          std::move(autofill_manager_factory_callback)) {}
+      provider_(provider) {}
 
 ContentAutofillDriver* ContentAutofillDriverFactory::DriverForFrame(
     content::RenderFrameHost* render_frame_host) {
   AutofillDriver* driver = DriverForKey(render_frame_host);
 
   // ContentAutofillDriver are created on demand here.
   if (!driver) {
-    AddForKey(render_frame_host,
-              base::BindRepeating(CreateDriver, render_frame_host, client(),
-                                  app_locale_, enable_download_manager_,
-                                  autofill_manager_factory_callback_));
+    AddForKey(
+        render_frame_host,
+        base::BindRepeating(CreateDriver, render_frame_host, client(),
+                            app_locale_, enable_download_manager_, provider_));
     driver = DriverForKey(render_frame_host);
   }
 

diff --git a/components/autofill/content/browser/content_autofill_driver_factory.h b/components/autofill/content/browser/content_autofill_driver_factory.h
@@ -10,7 +10,6 @@
 #include "base/supports_user_data.h"
 #include "components/autofill/content/common/mojom/autofill_driver.mojom.h"
 #include "components/autofill/core/browser/autofill_driver_factory.h"
-#include "components/autofill/core/browser/autofill_manager.h"
 #include "components/autofill/core/browser/browser_autofill_manager.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "mojo/public/cpp/bindings/pending_associated_receiver.h"
@@ -22,6 +21,7 @@ class RenderFrameHost;
 namespace autofill {
 
 class ContentAutofillDriver;
+class AutofillProvider;
 
 // Manages lifetime of ContentAutofillDriver. One Factory per WebContents
 // creates one Driver per RenderFrame.
@@ -37,8 +37,7 @@ class ContentAutofillDriverFactory : public AutofillDriverFactory,
       const std::string& app_locale,
       BrowserAutofillManager::AutofillDownloadManagerState
           enable_download_manager,
-      AutofillManager::AutofillManagerFactoryCallback
-          autofill_manager_factory_callback);
+      AutofillProvider* provider);
 
   ~ContentAutofillDriverFactory() override;
 
@@ -55,8 +54,7 @@ class ContentAutofillDriverFactory : public AutofillDriverFactory,
       const std::string& app_locale,
       BrowserAutofillManager::AutofillDownloadManagerState
           enable_download_manager,
-      AutofillManager::AutofillManagerFactoryCallback
-          autofill_manager_factory_callback);
+      AutofillProvider* provider);
 
   static ContentAutofillDriverFactory* FromWebContents(
       content::WebContents* contents);
@@ -82,8 +80,7 @@ class ContentAutofillDriverFactory : public AutofillDriverFactory,
  private:
   std::string app_locale_;
   BrowserAutofillManager::AutofillDownloadManagerState enable_download_manager_;
-  AutofillManager::AutofillManagerFactoryCallback
-      autofill_manager_factory_callback_;
+  AutofillProvider* provider_;
 };
 
 }  // namespace autofill

diff --git a/components/autofill/content/browser/content_autofill_driver_unittest.cc b/components/autofill/content/browser/content_autofill_driver_unittest.cc
@@ -284,12 +284,11 @@ class TestContentAutofillDriver : public ContentAutofillDriver {
  public:
   TestContentAutofillDriver(content::RenderFrameHost* rfh,
                             AutofillClient* client)
-      : ContentAutofillDriver(
-            rfh,
-            client,
-            kAppLocale,
-            kDownloadState,
-            AutofillManager::AutofillManagerFactoryCallback()) {
+      : ContentAutofillDriver(rfh,
+                              client,
+                              kAppLocale,
+                              kDownloadState,
+                              nullptr) {
     std::unique_ptr<BrowserAutofillManager> autofill_manager(
         new MockBrowserAutofillManager(this, client));
     SetBrowserAutofillManager(std::move(autofill_manager));