[Passwords] Use PasswordStore* in OnGetPasswordStoreResultsFrom()

This is a mechanical change that passes a raw pointer to
PasswordStoreConsumer::OnGetPasswordStoreResultsFrom() instead
of a scoped_refptr to avoid a potential refcount churn.

Bug: 1108422
Change-Id: I7f435fef2a22a0b9647914ab05367fd0511e536d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2362903
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799738}

components/password_manager/core/browser/credential_manager_pending_prevent_silent_access_task.cc

@@ -36,7 +36,7 @@ void CredentialManagerPendingPreventSilentAccessTask::OnGetPasswordStoreResults(
 
 void CredentialManagerPendingPreventSilentAccessTask::
     OnGetPasswordStoreResultsFrom(
-        scoped_refptr<PasswordStore> store,
+        PasswordStore* store,
         std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
   for (const auto& form : results) {
     if (!form->skip_zero_click) {

components/password_manager/core/browser/credential_manager_pending_prevent_silent_access_task.h

@@ -40,7 +40,7 @@ class CredentialManagerPendingPreventSilentAccessTask
   void OnGetPasswordStoreResults(
       std::vector<std::unique_ptr<autofill::PasswordForm>> results) override;
   void OnGetPasswordStoreResultsFrom(
-      scoped_refptr<PasswordStore> store,
+      PasswordStore* store,
       std::vector<std::unique_ptr<autofill::PasswordForm>> results) override;
 
  private:

components/password_manager/core/browser/credential_manager_pending_prevent_silent_access_task_unittest.cc

@@ -71,7 +71,7 @@ TEST_F(CredentialManagerPendingPreventSilentAccessTaskTest, ProfileStoreOnly) {
   // We are expecting results from only one store, delegate should be called
   // upon getting a response from the store.
   EXPECT_CALL(delegate_mock_, DoneRequiringUserMediation);
-  task.OnGetPasswordStoreResultsFrom(profile_store_, {});
+  task.OnGetPasswordStoreResultsFrom(profile_store_.get(), {});
 }
 
 TEST_F(CredentialManagerPendingPreventSilentAccessTaskTest,
@@ -90,12 +90,12 @@ TEST_F(CredentialManagerPendingPreventSilentAccessTaskTest,
   // We are expecting results from 2 stores, the delegate shouldn't be called
   // until both stores respond.
   EXPECT_CALL(delegate_mock_, DoneRequiringUserMediation).Times(0);
-  task.OnGetPasswordStoreResultsFrom(profile_store_, {});
+  task.OnGetPasswordStoreResultsFrom(profile_store_.get(), {});
 
   testing::Mock::VerifyAndClearExpectations(&delegate_mock_);
 
   EXPECT_CALL(delegate_mock_, DoneRequiringUserMediation);
-  task.OnGetPasswordStoreResultsFrom(account_store_, {});
+  task.OnGetPasswordStoreResultsFrom(account_store_.get(), {});
 }
 
 }  // namespace password_manager

components/password_manager/core/browser/credential_manager_pending_request_task.cc

@@ -164,13 +164,13 @@ void CredentialManagerPendingRequestTask::OnGetPasswordStoreResults(
 }
 
 void CredentialManagerPendingRequestTask::OnGetPasswordStoreResultsFrom(
-    scoped_refptr<PasswordStore> store,
+    PasswordStore* store,
     std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
   // localhost is a secure origin but not https.
   if (results.empty() && origin_.scheme() == url::kHttpsScheme) {
     // Try to migrate the HTTP passwords and process them later.
-    http_migrators_[store.get()] = std::make_unique<HttpPasswordStoreMigrator>(
-        origin_, store.get(), delegate_->client()->GetNetworkContext(), this);
+    http_migrators_[store] = std::make_unique<HttpPasswordStoreMigrator>(
+        origin_, store, delegate_->client()->GetNetworkContext(), this);
     return;
   }
   AggregatePasswordStoreResults(std::move(results));

components/password_manager/core/browser/credential_manager_pending_request_task.h

@@ -76,7 +76,7 @@ class CredentialManagerPendingRequestTask
   void OnGetPasswordStoreResults(
       std::vector<std::unique_ptr<autofill::PasswordForm>> results) override;
   void OnGetPasswordStoreResultsFrom(
-      scoped_refptr<PasswordStore> store,
+      PasswordStore* store,
       std::vector<std::unique_ptr<autofill::PasswordForm>> results) override;
 
  private:

components/password_manager/core/browser/credential_manager_pending_request_task_unittest.cc

@@ -127,7 +127,7 @@ TEST_F(CredentialManagerPendingRequestTaskTest, QueryProfileStore) {
   // We are expecting results from only one store, delegate should be called
   // upon getting a response from the store.
   EXPECT_CALL(delegate_mock_, SendCredential);
-  task.OnGetPasswordStoreResultsFrom(profile_store_, {});
+  task.OnGetPasswordStoreResultsFrom(profile_store_.get(), {});
 }
 
 TEST_F(CredentialManagerPendingRequestTaskTest, QueryProfileAndAccountStores) {
@@ -139,12 +139,12 @@ TEST_F(CredentialManagerPendingRequestTaskTest, QueryProfileAndAccountStores) {
   // We are expecting results from 2 stores, the delegate shouldn't be called
   // until both stores respond.
   EXPECT_CALL(delegate_mock_, SendCredential).Times(0);
-  task.OnGetPasswordStoreResultsFrom(profile_store_, {});
+  task.OnGetPasswordStoreResultsFrom(profile_store_.get(), {});
 
   testing::Mock::VerifyAndClearExpectations(&delegate_mock_);
 
   EXPECT_CALL(delegate_mock_, SendCredential);
-  task.OnGetPasswordStoreResultsFrom(account_store_, {});
+  task.OnGetPasswordStoreResultsFrom(account_store_.get(), {});
 }
 
 TEST_F(CredentialManagerPendingRequestTaskTest,
@@ -170,8 +170,10 @@ TEST_F(CredentialManagerPendingRequestTaskTest,
   account_forms.push_back(
       std::make_unique<autofill::PasswordForm>(account_form));
 
-  task.OnGetPasswordStoreResultsFrom(profile_store_, std::move(profile_forms));
-  task.OnGetPasswordStoreResultsFrom(account_store_, std::move(account_forms));
+  task.OnGetPasswordStoreResultsFrom(profile_store_.get(),
+                                     std::move(profile_forms));
+  task.OnGetPasswordStoreResultsFrom(account_store_.get(),
+                                     std::move(account_forms));
   EXPECT_EQ(2U, client()->forms_passed_to_ui().size());
 }
 
@@ -196,8 +198,10 @@ TEST_F(CredentialManagerPendingRequestTaskTest,
   account_forms.push_back(
       std::make_unique<autofill::PasswordForm>(account_form));
 
-  task.OnGetPasswordStoreResultsFrom(profile_store_, std::move(profile_forms));
-  task.OnGetPasswordStoreResultsFrom(account_store_, std::move(account_forms));
+  task.OnGetPasswordStoreResultsFrom(profile_store_.get(),
+                                     std::move(profile_forms));
+  task.OnGetPasswordStoreResultsFrom(account_store_.get(),
+                                     std::move(account_forms));
   ASSERT_EQ(1U, client()->forms_passed_to_ui().size());
   EXPECT_TRUE(client()->forms_passed_to_ui()[0]->IsUsingAccountStore());
 }
@@ -228,8 +232,10 @@ TEST_F(CredentialManagerPendingRequestTaskTest,
   account_forms.push_back(
       std::make_unique<autofill::PasswordForm>(account_form));
 
-  task.OnGetPasswordStoreResultsFrom(profile_store_, std::move(profile_forms));
-  task.OnGetPasswordStoreResultsFrom(account_store_, std::move(account_forms));
+  task.OnGetPasswordStoreResultsFrom(profile_store_.get(),
+                                     std::move(profile_forms));
+  task.OnGetPasswordStoreResultsFrom(account_store_.get(),
+                                     std::move(account_forms));
   ASSERT_EQ(1U, client()->forms_passed_to_ui().size());
   EXPECT_TRUE(client()->forms_passed_to_ui()[0]->IsUsingAccountStore());
 }

components/password_manager/core/browser/http_auth_manager_unittest.cc

@@ -171,7 +171,7 @@ TEST_F(HttpAuthManagerTest, HttpAuthFilling) {
     ASSERT_TRUE(consumer);
     std::vector<std::unique_ptr<PasswordForm>> result;
     result.push_back(std::make_unique<PasswordForm>(stored_form));
-    consumer->OnGetPasswordStoreResultsFrom(store_, std::move(result));
+    consumer->OnGetPasswordStoreResultsFrom(store_.get(), std::move(result));
     testing::Mock::VerifyAndClearExpectations(&store_);
     httpauth_manager()->DetachObserver(&observer);
   }

components/password_manager/core/browser/multi_store_form_fetcher.cc

@@ -108,15 +108,15 @@ void MultiStoreFormFetcher::OnGetPasswordStoreResults(
 }
 
 void MultiStoreFormFetcher::OnGetPasswordStoreResultsFrom(
-    scoped_refptr<PasswordStore> store,
+    PasswordStore* store,
     std::vector<std::unique_ptr<PasswordForm>> results) {
   DCHECK_EQ(State::WAITING, state_);
   DCHECK_GT(wait_counter_, 0);
 
   if (should_migrate_http_passwords_ && results.empty() &&
       form_digest_.url.SchemeIs(url::kHttpsScheme)) {
-    http_migrators_[store.get()] = std::make_unique<HttpPasswordStoreMigrator>(
-        url::Origin::Create(form_digest_.url), store.get(),
+    http_migrators_[store] = std::make_unique<HttpPasswordStoreMigrator>(
+        url::Origin::Create(form_digest_.url), store,
         client_->GetNetworkContext(), this);
     // The migrator will call us back at ProcessMigratedForms().
     return;

components/password_manager/core/browser/multi_store_form_fetcher.h

@@ -32,7 +32,7 @@ class MultiStoreFormFetcher : public FormFetcherImpl {
   void OnGetPasswordStoreResults(
       std::vector<std::unique_ptr<autofill::PasswordForm>> results) override;
   void OnGetPasswordStoreResultsFrom(
-      scoped_refptr<PasswordStore> store,
+      PasswordStore* store,
       std::vector<std::unique_ptr<autofill::PasswordForm>> results) override;
 
  private:

components/password_manager/core/browser/multi_store_form_fetcher_unittest.cc

@@ -201,9 +201,9 @@ TEST_F(MultiStoreFormFetcherTest, CloningMultiStoreFetcherClonesState) {
   blocked.in_store = PasswordForm::Store::kAccountStore;
   std::vector<std::unique_ptr<PasswordForm>> results;
   results.push_back(std::make_unique<PasswordForm>(blocked));
-  form_fetcher_->OnGetPasswordStoreResultsFrom(account_mock_store_,
+  form_fetcher_->OnGetPasswordStoreResultsFrom(account_mock_store_.get(),
                                                std::move(results));
-  form_fetcher_->OnGetPasswordStoreResultsFrom(profile_mock_store_, {});
+  form_fetcher_->OnGetPasswordStoreResultsFrom(profile_mock_store_.get(), {});
 
   EXPECT_EQ(form_fetcher_->GetState(), FormFetcher::State::NOT_WAITING);
   EXPECT_TRUE(form_fetcher_->IsBlacklisted());
@@ -235,9 +235,9 @@ TEST_F(MultiStoreFormFetcherTest, CloningMultiStoreFetcherResumesFetch) {
   blocked.in_store = PasswordForm::Store::kAccountStore;
   std::vector<std::unique_ptr<PasswordForm>> results;
   results.push_back(std::make_unique<PasswordForm>(blocked));
-  form_fetcher_->OnGetPasswordStoreResultsFrom(account_mock_store_,
+  form_fetcher_->OnGetPasswordStoreResultsFrom(account_mock_store_.get(),
                                                std::move(results));
-  form_fetcher_->OnGetPasswordStoreResultsFrom(profile_mock_store_, {});
+  form_fetcher_->OnGetPasswordStoreResultsFrom(profile_mock_store_.get(), {});
 
   EXPECT_EQ(form_fetcher_->GetState(), FormFetcher::State::NOT_WAITING);
   EXPECT_TRUE(form_fetcher_->IsBlacklisted());
@@ -250,11 +250,11 @@ TEST_F(MultiStoreFormFetcherTest, Empty) {
   EXPECT_CALL(consumer_, OnFetchCompleted);
   // Both profile and account respond with empty results.
   form_fetcher_->OnGetPasswordStoreResultsFrom(
-      profile_mock_store_, std::vector<std::unique_ptr<PasswordForm>>());
+      profile_mock_store_.get(), std::vector<std::unique_ptr<PasswordForm>>());
   // We should be still waiting for the second store to respond.
   EXPECT_EQ(FormFetcher::State::WAITING, form_fetcher_->GetState());
   form_fetcher_->OnGetPasswordStoreResultsFrom(
-      account_mock_store_, std::vector<std::unique_ptr<PasswordForm>>());
+      account_mock_store_.get(), std::vector<std::unique_ptr<PasswordForm>>());
   EXPECT_EQ(FormFetcher::State::NOT_WAITING, form_fetcher_->GetState());
   EXPECT_THAT(form_fetcher_->GetNonFederatedMatches(), IsEmpty());
   EXPECT_THAT(form_fetcher_->GetFederatedMatches(), IsEmpty());
@@ -284,7 +284,7 @@ TEST_F(MultiStoreFormFetcherTest, MergeFromBothStores) {
   results.push_back(std::make_unique<PasswordForm>(federated2));
   results.push_back(std::make_unique<PasswordForm>(non_federated1));
   results.push_back(std::make_unique<PasswordForm>(blocked));
-  form_fetcher_->OnGetPasswordStoreResultsFrom(profile_mock_store_,
+  form_fetcher_->OnGetPasswordStoreResultsFrom(profile_mock_store_.get(),
                                                std::move(results));
 
   // We should be still waiting for the second store to respond.
@@ -297,7 +297,7 @@ TEST_F(MultiStoreFormFetcherTest, MergeFromBothStores) {
   results.push_back(std::make_unique<PasswordForm>(non_federated3));
 
   EXPECT_CALL(consumer_, OnFetchCompleted);
-  form_fetcher_->OnGetPasswordStoreResultsFrom(account_mock_store_,
+  form_fetcher_->OnGetPasswordStoreResultsFrom(account_mock_store_.get(),
                                                std::move(results));
 
   EXPECT_EQ(FormFetcher::State::NOT_WAITING, form_fetcher_->GetState());
@@ -322,10 +322,10 @@ TEST_F(MultiStoreFormFetcherTest, BlockedEntryInTheAccountStore) {
   // Pass response from the first store.
   std::vector<std::unique_ptr<PasswordForm>> results;
   results.push_back(std::make_unique<PasswordForm>(blocked));
-  form_fetcher_->OnGetPasswordStoreResultsFrom(account_mock_store_,
+  form_fetcher_->OnGetPasswordStoreResultsFrom(account_mock_store_.get(),
                                                std::move(results));
   // Pass empty response from the second store.
-  form_fetcher_->OnGetPasswordStoreResultsFrom(profile_mock_store_, {});
+  form_fetcher_->OnGetPasswordStoreResultsFrom(profile_mock_store_.get(), {});
 
   // Simulate a user in the account mode.
   ON_CALL(*client()->GetPasswordFeatureManager(), IsOptedInForAccountStorage())
@@ -362,10 +362,10 @@ TEST_F(MultiStoreFormFetcherTest, BlockedEntryInTheProfileStore) {
   // Pass response from the first store.
   std::vector<std::unique_ptr<PasswordForm>> results;
   results.push_back(std::make_unique<PasswordForm>(blocked));
-  form_fetcher_->OnGetPasswordStoreResultsFrom(profile_mock_store_,
+  form_fetcher_->OnGetPasswordStoreResultsFrom(profile_mock_store_.get(),
                                                std::move(results));
   // Pass empty response from the second store.
-  form_fetcher_->OnGetPasswordStoreResultsFrom(account_mock_store_, {});
+  form_fetcher_->OnGetPasswordStoreResultsFrom(account_mock_store_.get(), {});
 
   // Simulate a user in the account mode.
   ON_CALL(*client()->GetPasswordFeatureManager(), IsOptedInForAccountStorage())
@@ -421,10 +421,10 @@ TEST_F(MultiStoreFormFetcherTest, MovingToAccountStoreIsBlocked) {
   results.push_back(std::make_unique<PasswordForm>(blocked_form));
   results.push_back(std::make_unique<PasswordForm>(unblocked_form));
   results.push_back(std::make_unique<PasswordForm>(psl_form));
-  form_fetcher_->OnGetPasswordStoreResultsFrom(profile_mock_store_,
+  form_fetcher_->OnGetPasswordStoreResultsFrom(profile_mock_store_.get(),
                                                std::move(results));
   // Pass empty response from the account store.
-  form_fetcher_->OnGetPasswordStoreResultsFrom(account_mock_store_, {});
+  form_fetcher_->OnGetPasswordStoreResultsFrom(account_mock_store_.get(), {});
 
   // Moving should be blocked for |kUser| and |form1|.
   EXPECT_TRUE(

components/password_manager/core/browser/password_manager_unittest.cc

@@ -1913,7 +1913,7 @@ TEST_P(PasswordManagerTest, SaveFormFetchedAfterSubmit) {
   // before post-navigation load.
   ASSERT_TRUE(store_consumer);
   store_consumer->OnGetPasswordStoreResultsFrom(
-      store_, std::vector<std::unique_ptr<PasswordForm>>());
+      store_.get(), std::vector<std::unique_ptr<PasswordForm>>());
 
   std::unique_ptr<PasswordFormManagerForUI> form_manager_to_save;
   EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_))
@@ -2521,7 +2521,7 @@ TEST_P(PasswordManagerTest, ManualFallbackForSaving_SlowBackend) {
   // The storage responded. The fallback can be shown.
   ASSERT_TRUE(store_consumer);
   store_consumer->OnGetPasswordStoreResultsFrom(
-      store_, std::vector<std::unique_ptr<PasswordForm>>());
+      store_.get(), std::vector<std::unique_ptr<PasswordForm>>());
   std::unique_ptr<PasswordFormManagerForUI> form_manager_to_save;
   EXPECT_CALL(client_, ShowManualFallbackForSavingPtr(_, false, false))
       .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save)));

components/password_manager/core/browser/password_store.cc

@@ -693,7 +693,7 @@ bool PasswordStore::InitOnBackgroundSequence() {
   base::SequencedTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::BindOnce(&PasswordStoreConsumer::OnGetPasswordStoreResultsFrom,
-                     reuse_detector_->GetWeakPtr(), base::WrapRefCounted(this),
+                     reuse_detector_->GetWeakPtr(), base::RetainedRef(this),
                      GetAutofillableLoginsImpl()));
 #endif
   return true;
@@ -894,7 +894,7 @@ void PasswordStore::PostLoginsTaskAndReplyToConsumerWithResult(
   consumer->cancelable_task_tracker()->PostTaskAndReplyWithResult(
       background_task_runner_.get(), FROM_HERE, std::move(task),
       base::BindOnce(&PasswordStoreConsumer::OnGetPasswordStoreResultsFrom,
-                     consumer->GetWeakPtr(), base::WrapRefCounted(this)));
+                     consumer->GetWeakPtr(), base::RetainedRef(this)));
 }
 
 void PasswordStore::PostLoginsTaskAndReplyToConsumerWithProcessedResult(
@@ -905,7 +905,7 @@ void PasswordStore::PostLoginsTaskAndReplyToConsumerWithProcessedResult(
   auto call_consumer = base::BindOnce(
       CloseTraceAndCallBack, trace_name, consumer,
       base::BindOnce(&PasswordStoreConsumer::OnGetPasswordStoreResultsFrom,
-                     consumer->GetWeakPtr(), base::WrapRefCounted(this)));
+                     consumer->GetWeakPtr(), base::RetainedRef(this)));
   consumer->cancelable_task_tracker()->PostTaskAndReplyWithResult(
       background_task_runner_.get(), FROM_HERE, std::move(task),
       base::BindOnce(std::move(processor), std::move(call_consumer)));

components/password_manager/core/browser/password_store_consumer.cc

@@ -16,7 +16,7 @@ PasswordStoreConsumer::PasswordStoreConsumer() = default;
 PasswordStoreConsumer::~PasswordStoreConsumer() = default;
 
 void PasswordStoreConsumer::OnGetPasswordStoreResultsFrom(
-    scoped_refptr<PasswordStore> store,
+    PasswordStore* store,
     std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
   OnGetPasswordStoreResults(std::move(results));
 }

components/password_manager/core/browser/password_store_consumer.h

@@ -42,7 +42,7 @@ class PasswordStoreConsumer {
   // The default implementation simply calls OnGetPasswordStoreResults(), so
   // consumers that don't care about the store can just ignore this.
   virtual void OnGetPasswordStoreResultsFrom(
-      scoped_refptr<PasswordStore> store,
+      PasswordStore* store,
       std::vector<std::unique_ptr<autofill::PasswordForm>> results);
 
   // Called when the GetSiteStats() request is finished, with the associated