[Autofill assistant] Fix failure with GeneratePasswordAction

Component manager driver was invalid at password generation. This CL
fixes the issue by passing web_contents_ to |WebsiteLoginFetcher|
instead of thedriver. Driver is later fetched just before password
generation.

Bug: 1131519
Change-Id: Ic903dfc33fd867efc185260817364fecc5d6ec8b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2426427Reviewed-by: Maxim Kolosovskiy  <kolos@chromium.org>
Reviewed-by: Clemens Arbesser <arbesser@google.com>
Commit-Queue: Milica Selakovic <selakovic@google.com>
Cr-Commit-Position: refs/heads/master@{#810269}

chrome/browser/android/autofill_assistant/client_android.cc

@@ -31,8 +31,6 @@
 #include "components/autofill_assistant/browser/features.h"
 #include "components/autofill_assistant/browser/switches.h"
 #include "components/autofill_assistant/browser/website_login_manager_impl.h"
-#include "components/password_manager/content/browser/content_password_manager_driver.h"
-#include "components/password_manager/content/browser/content_password_manager_driver_factory.h"
 #include "components/password_manager/core/browser/password_manager_client.h"
 #include "components/signin/public/identity_manager/account_info.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
@@ -485,15 +483,8 @@ ClientAndroid::GetPasswordManagerClient() const {
 WebsiteLoginManager* ClientAndroid::GetWebsiteLoginManager() const {
   if (!website_login_manager_) {
     auto* client = GetPasswordManagerClient();
-    auto* factory =
-        password_manager::ContentPasswordManagerDriverFactory::FromWebContents(
-            web_contents_);
-    // TODO(crbug.com/1043132): Add support for non-main frames. If another
-    // frame has a different origin than the main frame, passwords-related
-    // features may not work.
-    auto* driver = factory->GetDriverForFrame(web_contents_->GetMainFrame());
     website_login_manager_ =
-        std::make_unique<WebsiteLoginManagerImpl>(client, driver);
+        std::make_unique<WebsiteLoginManagerImpl>(client, web_contents_);
   }
   return website_login_manager_.get();
 }

components/autofill_assistant/browser/BUILD.gn

@@ -206,6 +206,7 @@ static_library("browser") {
     "//components/autofill/core/common",
     "//components/autofill_assistant/browser/devtools",
     "//components/google/core/common:common",
+    "//components/password_manager/content/browser:browser",
     "//components/password_manager/core/browser:browser",
     "//components/password_manager/core/browser/form_parsing:form_parsing",
     "//components/signin/public/identity_manager",

components/autofill_assistant/browser/DEPS

@@ -2,6 +2,7 @@ include_rules = [
   "+chrome/android/features/autofill_assistant/test_support_jni_headers",
   "+components/autofill",
   "+components/google/core/common",
+  "+components/password_manager/content/browser",
   "+components/password_manager/core/browser",
   "+components/password_manager/core/common",
   "+components/version_info",

components/autofill_assistant/browser/website_login_manager_impl.cc

@@ -6,12 +6,13 @@
 
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "components/password_manager/content/browser/content_password_manager_driver.h"
+#include "components/password_manager/content/browser/content_password_manager_driver_factory.h"
 #include "components/password_manager/core/browser/form_fetcher_impl.h"
 #include "components/password_manager/core/browser/form_parsing/form_parser.h"
 #include "components/password_manager/core/browser/password_form_metrics_recorder.h"
 #include "components/password_manager/core/browser/password_generation_frame_helper.h"
 #include "components/password_manager/core/browser/password_manager_client.h"
-#include "components/password_manager/core/browser/password_manager_driver.h"
 #include "components/password_manager/core/browser/password_save_manager_impl.h"
 #include "components/password_manager/core/browser/votes_uploader.h"
 #include "content/public/browser/browser_task_traits.h"
@@ -231,8 +232,8 @@ class WebsiteLoginManagerImpl::UpdatePasswordRequest
 
 WebsiteLoginManagerImpl::WebsiteLoginManagerImpl(
     password_manager::PasswordManagerClient* client,
-    password_manager::PasswordManagerDriver* driver)
-    : client_(client), driver_(driver), weak_ptr_factory_(this) {}
+    content::WebContents* web_contents)
+    : client_(client), web_contents_(web_contents), weak_ptr_factory_(this) {}
 
 WebsiteLoginManagerImpl::~WebsiteLoginManagerImpl() = default;
 
@@ -266,9 +267,19 @@ std::string WebsiteLoginManagerImpl::GeneratePassword(
     autofill::FormSignature form_signature,
     autofill::FieldSignature field_signature,
     uint64_t max_length) {
+  auto* factory =
+      password_manager::ContentPasswordManagerDriverFactory::FromWebContents(
+          web_contents_);
+  DCHECK(factory);
+  // TODO(crbug.com/1043132): Add support for non-main frames. If another
+  // frame has a different origin than the main frame, passwords-related
+  // features may not work.
+  auto* driver = factory->GetDriverForFrame(web_contents_->GetMainFrame());
+  DCHECK(driver);
+
   return base::UTF16ToUTF8(
-      driver_->GetPasswordGenerationHelper()->GeneratePassword(
-          driver_->GetLastCommittedURL(), form_signature, field_signature,
+      driver->GetPasswordGenerationHelper()->GeneratePassword(
+          driver->GetLastCommittedURL(), form_signature, field_signature,
           max_length));
 }
 

components/autofill_assistant/browser/website_login_manager_impl.h

@@ -9,10 +9,10 @@
 #include "base/memory/weak_ptr.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/autofill_assistant/browser/website_login_manager.h"
+#include "content/public/browser/web_contents.h"
 
 namespace password_manager {
 class PasswordManagerClient;
-class PasswordManagerDriver;
 }  // namespace password_manager
 
 namespace autofill_assistant {
@@ -22,7 +22,7 @@ namespace autofill_assistant {
 class WebsiteLoginManagerImpl : public WebsiteLoginManager {
  public:
   WebsiteLoginManagerImpl(password_manager::PasswordManagerClient* client,
-                          password_manager::PasswordManagerDriver* driver);
+                          content::WebContents* web_contents);
   ~WebsiteLoginManagerImpl() override;
 
   // From WebsiteLoginManager:
@@ -55,7 +55,7 @@ class WebsiteLoginManagerImpl : public WebsiteLoginManager {
 
   password_manager::PasswordManagerClient* const client_;
 
-  password_manager::PasswordManagerDriver* const driver_;
+  content::WebContents* const web_contents_;
 
   // Update password request will be created in PresaveGeneratedPassword and
   // released in CommitGeneratedPassword after committing presaved password to
@@ -74,4 +74,4 @@ class WebsiteLoginManagerImpl : public WebsiteLoginManager {
 
 }  // namespace autofill_assistant
 
-#endif  // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_website_login_manager_IMPL_H_
+#endif  // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_WEBSITE_LOGIN_MANAGER_IMPL_H_

components/autofill_assistant/browser/website_login_manager_impl_unittest.cc

@@ -11,9 +11,10 @@
 #include "components/password_manager/core/browser/mock_password_store.h"
 #include "components/password_manager/core/browser/password_store_consumer.h"
 #include "components/password_manager/core/browser/stub_password_manager_client.h"
-#include "components/password_manager/core/browser/stub_password_manager_driver.h"
 #include "components/password_manager/core/common/password_manager_features.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_task_environment.h"
+#include "content/public/test/test_renderer_host.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -50,15 +51,6 @@ class MockPasswordManagerClient
                      password_manager::PasswordStore*());
 };
 
-class MockPasswordManagerDriver
-    : public password_manager::StubPasswordManagerDriver {
- public:
-  MockPasswordManagerDriver() = default;
-
-  MOCK_CONST_METHOD0(GetId, int());
-  MOCK_CONST_METHOD0(IsMainFrame, bool());
-};
-
 FormData MakeFormDataWithPasswordField() {
   FormData form_data;
   form_data.url = GURL(kFakeUrl);
@@ -101,9 +93,12 @@ PasswordForm MakeSimplePasswordFormWithoutUsername() {
 
 }  // namespace
 
-class WebsiteLoginManagerImplTest : public testing::Test {
+class WebsiteLoginManagerImplTest : public content::RenderViewHostTestHarness {
  public:
-  WebsiteLoginManagerImplTest() = default;
+  WebsiteLoginManagerImplTest()
+      : RenderViewHostTestHarness(
+            base::test::TaskEnvironment::MainThreadType::UI,
+            base::test::TaskEnvironment::TimeSource::MOCK_TIME) {}
   ~WebsiteLoginManagerImplTest() override = default;
 
  protected:
@@ -125,10 +120,8 @@ class WebsiteLoginManagerImplTest : public testing::Test {
           .WillByDefault(Return(account_store_.get()));
     }
 
-    ON_CALL(driver_, GetId()).WillByDefault(Return(0));
-    ON_CALL(driver_, IsMainFrame()).WillByDefault(Return(true));
-
-    manager_ = std::make_unique<WebsiteLoginManagerImpl>(&client_, &driver_);
+    manager_ =
+        std::make_unique<WebsiteLoginManagerImpl>(&client_, web_contents());
   }
 
   void TearDown() override {
@@ -141,17 +134,13 @@ class WebsiteLoginManagerImplTest : public testing::Test {
   WebsiteLoginManagerImpl* manager() { return manager_.get(); }
   password_manager::MockPasswordStore* store() { return profile_store_.get(); }
 
-  void WaitForPasswordStore() { task_environment_.RunUntilIdle(); }
+  void WaitForPasswordStore() { task_environment()->RunUntilIdle(); }
 
  private:
   testing::NiceMock<MockPasswordManagerClient> client_;
-  MockPasswordManagerDriver driver_;
   std::unique_ptr<WebsiteLoginManagerImpl> manager_;
   scoped_refptr<password_manager::MockPasswordStore> profile_store_;
   scoped_refptr<password_manager::MockPasswordStore> account_store_;
-  content::BrowserTaskEnvironment task_environment_{
-      base::test::TaskEnvironment::TimeSource::MOCK_TIME,
-      content::BrowserTaskEnvironment::IO_MAINLOOP};
 };
 
 // Checks if PasswordForm matches other PasswordForm.