[Passwords] Inject PasswordStore to the HttpPasswordStoreMigrator

This is a pure refactoring CL.

Following up CLs will inject the account store when appropriate.

Bug: 1107741
Change-Id: Ic3e31d3f412456c596f9ce97185e90b1b9e5d47b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2315151
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792789}

components/password_manager/core/browser/credential_manager_pending_request_task.cc

@@ -134,8 +134,8 @@ void CredentialManagerPendingRequestTask::OnGetPasswordStoreResultsFrom(
 
     // Try to migrate the HTTP passwords and process them later.
     http_migrator_ = std::make_unique<HttpPasswordStoreMigrator>(
-        origin_, delegate_->client(), delegate_->client()->GetNetworkContext(),
-        this);
+        origin_, delegate_->client()->GetProfilePasswordStore(),
+        delegate_->client()->GetNetworkContext(), this);
     return;
   }
   AggregatePasswordStoreResults(std::move(results));

components/password_manager/core/browser/form_fetcher_impl.cc

@@ -258,8 +258,8 @@ void FormFetcherImpl::OnGetPasswordStoreResults(
   if (should_migrate_http_passwords_ && results.empty() &&
       form_digest_.url.SchemeIs(url::kHttpsScheme)) {
     http_migrator_ = std::make_unique<HttpPasswordStoreMigrator>(
-        url::Origin::Create(form_digest_.url), client_,
-        client_->GetNetworkContext(), this);
+        url::Origin::Create(form_digest_.url),
+        client_->GetProfilePasswordStore(), client_->GetNetworkContext(), this);
     return;
   }
 

components/password_manager/core/browser/http_password_store_migrator.cc

@@ -12,7 +12,6 @@
 #include "base/metrics/histogram_functions.h"
 #include "base/stl_util.h"
 #include "base/strings/strcat.h"
-#include "components/password_manager/core/browser/password_manager_client.h"
 #include "components/password_manager/core/browser/password_manager_metrics_util.h"
 #include "components/password_manager/core/browser/password_manager_util.h"
 #include "components/password_manager/core/browser/password_store.h"
@@ -38,11 +37,11 @@ void OnHSTSQueryResultHelper(
 
 HttpPasswordStoreMigrator::HttpPasswordStoreMigrator(
     const url::Origin& https_origin,
-    const PasswordManagerClient* client,
+    PasswordStore* store,
     network::mojom::NetworkContext* network_context,
     Consumer* consumer)
-    : client_(client), consumer_(consumer) {
-  DCHECK(client_);
+    : store_(store), consumer_(consumer) {
+  DCHECK(store_);
   DCHECK(!https_origin.opaque());
   DCHECK_EQ(https_origin.scheme(), url::kHttpsScheme) << https_origin;
 
@@ -52,7 +51,7 @@ HttpPasswordStoreMigrator::HttpPasswordStoreMigrator(
   PasswordStore::FormDigest form(autofill::PasswordForm::Scheme::kHtml,
                                  http_origin.GetOrigin().spec(), http_origin);
   http_origin_domain_ = url::Origin::Create(http_origin);
-  client_->GetProfilePasswordStore()->GetLogins(form, this);
+  store_->GetLogins(form, this);
 
   PostHSTSQueryForHostAndNetworkContext(
       https_origin, network_context,
@@ -106,8 +105,7 @@ void HttpPasswordStoreMigrator::OnHSTSQueryResult(HSTSResult is_hsts) {
   got_hsts_query_result_ = true;
 
   if (is_hsts == HSTSResult::kYes)
-    client_->GetProfilePasswordStore()->RemoveSiteStats(
-        http_origin_domain_.GetURL());
+    store_->RemoveSiteStats(http_origin_domain_.GetURL());
 
   if (got_password_store_results_)
     ProcessPasswordStoreResults();
@@ -125,10 +123,10 @@ void HttpPasswordStoreMigrator::ProcessPasswordStoreResults() {
   for (const auto& form : results_) {
     autofill::PasswordForm new_form =
         HttpPasswordStoreMigrator::MigrateHttpFormToHttps(*form);
-    client_->GetProfilePasswordStore()->AddLogin(new_form);
+    store_->AddLogin(new_form);
 
     if (mode_ == HttpPasswordMigrationMode::kMove)
-      client_->GetProfilePasswordStore()->RemoveLogin(*form);
+      store_->RemoveLogin(*form);
     *form = std::move(new_form);
   }
 

components/password_manager/core/browser/http_password_store_migrator.h

@@ -20,8 +20,6 @@ struct PasswordForm;
 
 namespace password_manager {
 
-class PasswordManagerClient;
-
 // These values are persisted to logs. Entries should not be renumbered and
 // numeric values should never be reused.
 //
@@ -58,7 +56,7 @@ class HttpPasswordStoreMigrator : public PasswordStoreConsumer {
 
   // |https_origin| should specify a valid HTTPS URL.
   HttpPasswordStoreMigrator(const url::Origin& https_origin,
-                            const PasswordManagerClient* client,
+                            PasswordStore* store,
                             network::mojom::NetworkContext* network_context,
                             Consumer* consumer);
   ~HttpPasswordStoreMigrator() override;
@@ -77,7 +75,7 @@ class HttpPasswordStoreMigrator : public PasswordStoreConsumer {
  private:
   void ProcessPasswordStoreResults();
 
-  const PasswordManagerClient* const client_;
+  PasswordStore* const store_;
   Consumer* consumer_;
 
   // |ProcessPasswordStoreResults| requires that both |OnHSTSQueryResult| and

components/password_manager/core/browser/http_password_store_migrator_unittest.cc

@@ -9,7 +9,6 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/task_environment.h"
 #include "components/password_manager/core/browser/mock_password_store.h"
-#include "components/password_manager/core/browser/stub_password_manager_client.h"
 #include "services/network/test/test_network_context.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -104,17 +103,6 @@ class MockNetworkContext : public network::TestNetworkContext {
               (override));
 };
 
-class TestPasswordManagerClient : public StubPasswordManagerClient {
- public:
-  explicit TestPasswordManagerClient(PasswordStore* store) : store_(store) {}
-
-  // PasswordManagerClient:
-  PasswordStore* GetProfilePasswordStore() const override { return store_; }
-
- private:
-  PasswordStore* store_;
-};
-
 }  // namespace
 
 class HttpPasswordStoreMigratorTest : public testing::Test {
@@ -127,7 +115,6 @@ class HttpPasswordStoreMigratorTest : public testing::Test {
 
   MockConsumer& consumer() { return consumer_; }
   MockPasswordStore& store() { return *mock_store_; }
-  TestPasswordManagerClient& client() { return client_; }
   MockNetworkContext& mock_network_context() { return mock_network_context_; }
 
   void WaitForPasswordStore() { task_environment_.RunUntilIdle(); }
@@ -142,7 +129,6 @@ class HttpPasswordStoreMigratorTest : public testing::Test {
   MockConsumer consumer_;
   scoped_refptr<MockPasswordStore> mock_store_ =
       base::MakeRefCounted<testing::StrictMock<MockPasswordStore>>();
-  TestPasswordManagerClient client_{mock_store_.get()};
   testing::NiceMock<MockNetworkContext> mock_network_context_;
 
   DISALLOW_COPY_AND_ASSIGN(HttpPasswordStoreMigratorTest);
@@ -158,7 +144,7 @@ void HttpPasswordStoreMigratorTest::TestEmptyStore(bool is_hsts) {
           [is_hsts](auto cb) { std::move(cb).Run(is_hsts); }));
 
   HttpPasswordStoreMigrator migrator(url::Origin::Create(GURL(kTestHttpsURL)),
-                                     &client(), &mock_network_context(),
+                                     &store(), &mock_network_context(),
                                      &consumer());
   // We expect a potential call to |RemoveSiteStatsImpl| which is a async task
   // posted from |PasswordStore::RemoveSiteStats|. Hence the following lines are
@@ -181,7 +167,7 @@ void HttpPasswordStoreMigratorTest::TestFullStore(bool is_hsts) {
       .WillOnce(testing::WithArg<1>(
           [is_hsts](auto cb) { std::move(cb).Run(is_hsts); }));
   HttpPasswordStoreMigrator migrator(url::Origin::Create(GURL(kTestHttpsURL)),
-                                     &client(), &mock_network_context(),
+                                     &store(), &mock_network_context(),
                                      &consumer());
   // We expect a potential call to |RemoveSiteStatsImpl| which is a async task
   // posted from |PasswordStore::RemoveSiteStats|. Hence the following lines are
@@ -232,7 +218,7 @@ void HttpPasswordStoreMigratorTest::TestMigratorDeletionByConsumer(
   // Construct the migrator, call |OnGetPasswordStoreResults| explicitly and
   // manually delete it.
   auto migrator = std::make_unique<HttpPasswordStoreMigrator>(
-      url::Origin::Create(GURL(kTestHttpsURL)), &client(),
+      url::Origin::Create(GURL(kTestHttpsURL)), &store(),
       &mock_network_context(), &consumer());
 
   EXPECT_CALL(consumer(), ProcessForms(_)).WillOnce(Invoke([&migrator](Unused) {

components/password_manager/core/browser/multi_store_form_fetcher.cc

@@ -119,8 +119,8 @@ void MultiStoreFormFetcher::OnGetPasswordStoreResultsFrom(
     // TODO(crbug.com/1107741): Consider also supporting HTTP->HTTPS migration
     // for the account store.
     http_migrator_ = std::make_unique<HttpPasswordStoreMigrator>(
-        url::Origin::Create(form_digest_.url), client_,
-        client_->GetNetworkContext(), this);
+        url::Origin::Create(form_digest_.url),
+        client_->GetProfilePasswordStore(), client_->GetNetworkContext(), this);
     // The migrator will call us back at ProcessMigratedForms().
     return;
   }