Remove set_autofill_client_for_test in PasswordAutofillManager.

There is no need for this test-only method - instead, use Autofill's injector mechanism to inject an ObservingAutofillClient directly into the WebContents. Bug: 1473235 Change-Id: I265faec3a560a3b01058e42e8240f33afceb2ec6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4777761 Reviewed-by: Christoph Schwering <schwering@google.com> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Commit-Queue: Jan Keitel <jkeitel@google.com> Cr-Commit-Position: refs/heads/main@{#1184699}
chromium · Aug 17, 2023 · 27b9c79 · 27b9c79
1 parent 102b122
commit 27b9c79
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 64 deletions.
diff --git a/chrome/browser/password_manager/password_generation_interactive_uitest.cc b/chrome/browser/password_manager/password_generation_interactive_uitest.cc
@@ -20,6 +20,7 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/autofill/content/browser/content_autofill_client.h"
+#include "components/autofill/content/browser/test_autofill_client_injector.h"
 #include "components/autofill/core/browser/autofill_test_utils.h"
 #include "components/autofill/core/browser/test_autofill_client.h"
 #include "components/autofill/core/common/autofill_features.h"
@@ -155,6 +156,20 @@ class PasswordGenerationInteractiveTest
   TestGenerationPopupObserver observer_;
 };
 
+// A test fixture that injects an `ObservingAutofillClient` into newly created
+// tabs to allow waiting for an Autofill popup to open.
+class PasswordGenerationAutofillPopupInteractiveTest
+    : public PasswordGenerationInteractiveTest {
+ protected:
+  ObservingAutofillClient& autofill_client() {
+    return *autofill_client_injector_[WebContents()];
+  }
+
+ private:
+  autofill::TestAutofillClientInjector<ObservingAutofillClient>
+      autofill_client_injector_;
+};
+
 IN_PROC_BROWSER_TEST_F(PasswordGenerationInteractiveTest,
                        PopupShownAndPasswordSelected) {
   FocusPasswordField();
@@ -243,7 +258,7 @@ IN_PROC_BROWSER_TEST_F(PasswordGenerationInteractiveTest,
 // Verify that password generation popup is hidden when popup
 // with generation and password suggestions is visible.
 IN_PROC_BROWSER_TEST_F(
-    PasswordGenerationInteractiveTest,
+    PasswordGenerationAutofillPopupInteractiveTest,
     HidesGenerationPopupWhenShowingPasswordSuggestionsWithGeneration) {
   // Save the credentials since the autofill popup with generation and
   // password suggestion would not appear without stored passwords.
@@ -267,22 +282,11 @@ IN_PROC_BROWSER_TEST_F(
   WaitForStatus(TestGenerationPopupObserver::GenerationPopup::kShown);
   EXPECT_TRUE(GenerationPopupShowing());
 
-  password_manager::ContentPasswordManagerDriverFactory* driver_factory =
-      password_manager::ContentPasswordManagerDriverFactory::FromWebContents(
-          WebContents());
-  ObservingAutofillClient::CreateForWebContents(WebContents());
-  ObservingAutofillClient* observing_autofill_client =
-      ObservingAutofillClient::FromWebContents(WebContents());
-  password_manager::ContentPasswordManagerDriver* driver =
-      driver_factory->GetDriverForFrame(WebContents()->GetPrimaryMainFrame());
-  driver->GetPasswordAutofillManager()->set_autofill_client_for_test(
-      observing_autofill_client);
-
   // Click on the password field to display the autofill popup.
   content::SimulateMouseClickOrTapElementWithId(WebContents(),
                                                 "password_field");
-  // Make sure the autofill popup would be shown.
-  observing_autofill_client->WaitForAutofillPopup();
+  // Make sure that the autofill popup is showing.
+  autofill_client().WaitForAutofillPopup();
   // Make sure the generation popup is dismissed.
   WaitForStatus(TestGenerationPopupObserver::GenerationPopup::kHidden);
 }

diff --git a/chrome/browser/password_manager/password_manager_browsertest.cc b/chrome/browser/password_manager/password_manager_browsertest.cc
@@ -44,6 +44,7 @@
 #include "components/autofill/content/browser/content_autofill_client.h"
 #include "components/autofill/content/browser/content_autofill_driver.h"
 #include "components/autofill/content/browser/content_autofill_driver_factory.h"
+#include "components/autofill/content/browser/test_autofill_client_injector.h"
 #include "components/autofill/content/common/mojom/autofill_driver.mojom-test-utils.h"
 #include "components/autofill/content/common/mojom/autofill_driver.mojom.h"
 #include "components/autofill/core/browser/autofill_test_utils.h"
@@ -141,6 +142,20 @@ class PasswordManagerBrowserTest : public PasswordManagerBrowserTestBase {
   ~PasswordManagerBrowserTest() override = default;
 };
 
+// A test fixture that injects an `ObservingAutofillClient` into newly created
+// tabs to allow waiting for an Autofill popup to open.
+class PasswordManagerAutofillPopupBrowserTest
+    : public PasswordManagerBrowserTest {
+ protected:
+  ObservingAutofillClient& autofill_client() {
+    return *autofill_client_injector_[WebContents()];
+  }
+
+ private:
+  autofill::TestAutofillClientInjector<ObservingAutofillClient>
+      autofill_client_injector_;
+};
+
 // This fixture enables communication to the Autofill crowdsourcing server, but
 // denies any such requests.
 class PasswordManagerVotingBrowserTest : public PasswordManagerBrowserTest {
@@ -2017,7 +2032,7 @@ IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest,
   EXPECT_FALSE(prompt_observer.IsSavePromptShownAutomatically());
 }
 
-IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest,
+IN_PROC_BROWSER_TEST_F(PasswordManagerAutofillPopupBrowserTest,
                        InFrameNavigationDoesNotClearPopupState) {
   password_manager::PasswordStoreInterface* password_store =
       PasswordStoreFactory::GetForProfile(browser()->profile(),
@@ -2030,21 +2045,6 @@ IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest,
   password_store->AddLogin(signin_form);
 
   NavigateToFile("/password/password_form.html");
-
-  // Mock out the AutofillClient so we know how long to wait. Unfortunately
-  // there isn't otherwise a good event to wait on to verify that the popup
-  // would have been shown.
-  password_manager::ContentPasswordManagerDriverFactory* driver_factory =
-      password_manager::ContentPasswordManagerDriverFactory::FromWebContents(
-          WebContents());
-  ObservingAutofillClient::CreateForWebContents(WebContents());
-  ObservingAutofillClient* observing_autofill_client =
-      ObservingAutofillClient::FromWebContents(WebContents());
-  password_manager::ContentPasswordManagerDriver* driver =
-      driver_factory->GetDriverForFrame(WebContents()->GetPrimaryMainFrame());
-  driver->GetPasswordAutofillManager()->set_autofill_client_for_test(
-      observing_autofill_client);
-
   // Trigger in page navigation.
   std::string in_page_navigate = "location.hash = '#blah';";
   ASSERT_TRUE(content::ExecJs(RenderFrameHost(), in_page_navigate,
@@ -2053,8 +2053,8 @@ IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest,
   // Click on the username field to display the popup.
   content::SimulateMouseClickOrTapElementWithId(WebContents(),
                                                 "username_field");
-  // Make sure the popup would be shown.
-  observing_autofill_client->WaitForAutofillPopup();
+  // Make sure that the popup is showing.
+  autofill_client().WaitForAutofillPopup();
 }
 
 IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest, ChangePwdFormBubbleShown) {

diff --git a/chrome/browser/password_manager/password_manager_uitest_util.cc b/chrome/browser/password_manager/password_manager_uitest_util.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/password_manager/password_manager_uitest_util.h"
 
+#include "chrome/browser/ui/autofill/chrome_autofill_client.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 void TestGenerationPopupObserver::OnPopupShown(
@@ -52,6 +53,10 @@ void TestGenerationPopupObserver::MaybeQuitRunLoop() {
   }
 }
 
+ObservingAutofillClient::ObservingAutofillClient(
+    content::WebContents* web_contents)
+    : autofill::ChromeAutofillClient(web_contents) {}
+
 void ObservingAutofillClient::WaitForAutofillPopup() {
   base::RunLoop run_loop;
   run_loop_ = &run_loop;
@@ -66,5 +71,3 @@ void ObservingAutofillClient::ShowAutofillPopup(
     run_loop_->Quit();
   run_loop_ = nullptr;
 }
-
-WEB_CONTENTS_USER_DATA_KEY_IMPL(ObservingAutofillClient);
diff --git a/chrome/browser/password_manager/password_manager_uitest_util.h b/chrome/browser/password_manager/password_manager_uitest_util.h
@@ -11,7 +11,6 @@
 #include "chrome/browser/ui/autofill/chrome_autofill_client.h"
 #include "chrome/browser/ui/passwords/password_generation_popup_controller.h"
 #include "chrome/browser/ui/passwords/password_generation_popup_observer.h"
-#include "components/autofill/core/browser/test_autofill_client.h"
 
 using GenerationUIState = PasswordGenerationPopupController::GenerationUIState;
 
@@ -51,14 +50,10 @@ class TestGenerationPopupObserver : public PasswordGenerationPopupObserver {
       PasswordGenerationPopupController::kOfferGeneration;
 };
 
-// A test AutofillClient client that can block until
-// the ShowAutofillPopup() is called.
-class ObservingAutofillClient
-    : public autofill::TestAutofillClient,
-      public content::WebContentsUserData<ObservingAutofillClient> {
+// An Autofill client that can block until ShowAutofillPopup() is called.
+class ObservingAutofillClient : public autofill::ChromeAutofillClient {
  public:
-  ObservingAutofillClient(const ObservingAutofillClient&) = delete;
-  ObservingAutofillClient& operator=(const ObservingAutofillClient&) = delete;
+  explicit ObservingAutofillClient(content::WebContents* web_contents);
 
   // Blocks the current thread until ShowAutofillPopup() is called.
   void WaitForAutofillPopup();
@@ -68,13 +63,7 @@ class ObservingAutofillClient
       base::WeakPtr<autofill::AutofillPopupDelegate> delegate) override;
 
  private:
-  explicit ObservingAutofillClient(content::WebContents* web_contents)
-      : content::WebContentsUserData<ObservingAutofillClient>(*web_contents) {}
-  friend class content::WebContentsUserData<ObservingAutofillClient>;
-
   raw_ptr<base::RunLoop> run_loop_ = nullptr;
-
-  WEB_CONTENTS_USER_DATA_KEY_DECL();
 };
 
 #endif  // CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_MANAGER_UITEST_UTIL_H_
diff --git a/...ents/password_manager/content/browser/content_password_manager_driver_factory_test_api.cc b/...ents/password_manager/content/browser/content_password_manager_driver_factory_test_api.cc
@@ -23,13 +23,4 @@ ContentPasswordManagerDriverFactoryTestApi::
         ContentPasswordManagerDriverFactory* factory)
     : factory_(factory) {}
 
-void ContentPasswordManagerDriverFactoryTestApi::SetAutofillClient(
-    autofill::AutofillClient* autofill_client) {
-  factory_->autofill_client_ = autofill_client;
-  for (auto& [rfh, driver] : factory_->frame_driver_map_) {
-    driver.GetPasswordAutofillManager()->set_autofill_client_for_test(
-        autofill_client);
-  }
-}
-
 }  // namespace password_manager
diff --git a/...nents/password_manager/content/browser/content_password_manager_driver_factory_test_api.h b/...nents/password_manager/content/browser/content_password_manager_driver_factory_test_api.h
@@ -21,8 +21,6 @@ class ContentPasswordManagerDriverFactoryTestApi {
   explicit ContentPasswordManagerDriverFactoryTestApi(
       ContentPasswordManagerDriverFactory* factory);
 
-  void SetAutofillClient(autofill::AutofillClient* autofill_client);
-
  private:
   raw_ptr<ContentPasswordManagerDriverFactory> factory_;
 };

diff --git a/components/password_manager/core/browser/password_autofill_manager.h b/components/password_manager/core/browser/password_autofill_manager.h
@@ -126,10 +126,6 @@ class PasswordAutofillManager : public autofill::AutofillPopupDelegate {
   // A public version of PreviewSuggestion(), only for use in tests.
   bool PreviewSuggestionForTest(const std::u16string& username);
 
-  void set_autofill_client_for_test(autofill::AutofillClient* autofill_client) {
-    autofill_client_ = autofill_client;
-  }
-
  private:
   using ForPasswordField = base::StrongAlias<class ForPasswordFieldTag, bool>;
   using OffersGeneration = base::StrongAlias<class OffersGenerationTag, bool>;
@@ -224,7 +220,7 @@ class PasswordAutofillManager : public autofill::AutofillPopupDelegate {
   // The driver that owns |this|.
   const raw_ptr<PasswordManagerDriver> password_manager_driver_;
 
-  raw_ptr<autofill::AutofillClient> autofill_client_;
+  const raw_ptr<autofill::AutofillClient> autofill_client_;
 
   const raw_ptr<PasswordManagerClient> password_client_;