Make FormFetcher a PasswordStoreConsumer

components/password_manager/core/browser/password_form_manager.h

Index: components/password_manager/core/browser/password_form_manager.h
diff --git a/components/password_manager/core/browser/password_form_manager.h b/components/password_manager/core/browser/password_form_manager.h
index 1718c9b474e79c975936ffc73cf13dc2834e65ef..f46d40128b41fc9d60a45ede1497b166a87a315e 100644
--- a/components/password_manager/core/browser/password_form_manager.h
+++ b/components/password_manager/core/browser/password_form_manager.h
@@ -25,7 +25,6 @@
 #include "components/password_manager/core/browser/form_fetcher_impl.h"
 #include "components/password_manager/core/browser/password_manager_driver.h"
 #include "components/password_manager/core/browser/password_store.h"
-#include "components/password_manager/core/browser/password_store_consumer.h"
 
 namespace password_manager {
 
@@ -33,18 +32,25 @@ class FormSaver;
 class PasswordManager;
 class PasswordManagerClient;
 
-// Per-password-form-{on-page, dialog} class responsible for interactions
-// between a given form, the per-tab PasswordManager, and the PasswordStore.
-class PasswordFormManager : public PasswordStoreConsumer,
-                            public FormFetcher::Consumer {
+// This class helps with filling the observed form (both HTML and from HTTP
+// auth) and with saving/updating the stored information about it.
+class PasswordFormManager : public FormFetcher::Consumer {
  public:
-  // |password_manager| owns this object
-  // |form_on_page| is the form that may be submitted and could need login data.
+  // |password_manager| owns |this|, |client| and |driver| serve to
+  // communicate with embedder, |observed_form| is the associated form |this|
+  // is managing, |form_saver| is used to save/update the form and
+  // |form_fetcher| to get saved data about the form.
+  //
+  // TODO(crbug.com/621355): So far, |form_fetcher| can be null. In that case
+  // |this| creates an instance of it itself (meant for production code). Once
+  // the fetcher is shared between PasswordFormManager instances, it will be
+  // required that |form_fetcher| is not null.
   PasswordFormManager(PasswordManager* password_manager,
                       PasswordManagerClient* client,
                       const base::WeakPtr<PasswordManagerDriver>& driver,
                       const autofill::PasswordForm& observed_form,
-                      std::unique_ptr<FormSaver> form_saver);
+                      std::unique_ptr<FormSaver> form_saver,
+                      FormFetcher* form_fetcher);
   ~PasswordFormManager() override;
 
   // Flags describing the result of comparing two forms as performed by
@@ -81,26 +87,6 @@ class PasswordFormManager : public PasswordStoreConsumer,
   // they match. The return value is a MatchResultMask bitmask.
   MatchResultMask DoesManage(const autofill::PasswordForm& form) const;
 
-  // Retrieves potential matching logins from the database. In addition the
-  // statistics is retrived on platforms with the password bubble. This is
-  // called automatically during construction and can be called manually later
-  // as well to cause an update of the cached credentials.
-  void FetchDataFromPasswordStore();
-
-  // Simple state-check to verify whether this object as received a callback
-  // from the PasswordStore and completed its matching phase. Note that the
-  // callback in question occurs on the same (and only) main thread from which
-  // instances of this class are ever used, but it is required since it is
-  // conceivable that a user (or ui test) could attempt to submit a login
-  // prompt before the callback has occured, which would InvokeLater a call to
-  // PasswordManager::ProvisionallySave, which would interact with this object
-  // before the db has had time to answer with matching password entries.
-  // This is intended to be a one-time check; if the return value is false the
-  // expectation is caller will give up. This clearly won't work if you put it
-  // in a loop and wait for matching to complete; you're (supposed to be) on
-  // the same thread!
-  bool HasCompletedMatching() const;
-
   // Update |this| with the |form| that was actually submitted. Used to
   // determine what type the submitted form is for
   // IsIgnorableChangePasswordForm() and UMA stats.
@@ -125,12 +111,6 @@ class PasswordFormManager : public PasswordStoreConsumer,
   // delayed until the data arrives.
   void ProcessFrame(const base::WeakPtr<PasswordManagerDriver>& driver);
 
-  // PasswordStoreConsumer:
-  void OnGetPasswordStoreResults(
-      std::vector<std::unique_ptr<autofill::PasswordForm>> results) override;
-  void OnGetSiteStatistics(
-      std::vector<std::unique_ptr<InteractionsStats>> stats) override;
-
   // A user opted to 'never remember' passwords for this form.
   // Blacklist it so that from now on when it is seen we ignore it.
   // TODO(vasilii): remove the 'virtual' specifier.
@@ -221,7 +201,7 @@ class PasswordFormManager : public PasswordStoreConsumer,
     return is_possible_change_password_form_without_username_;
   }
 
-  const FormFetcher* form_fetcher() { return form_fetcher_; }
+  FormFetcher* form_fetcher() { return form_fetcher_; }
 
   // Use this to wipe copies of |pending_credentials_| from the password store
   // (and |best_matches_| as well. It will only wipe if:
@@ -245,6 +225,12 @@ class PasswordFormManager : public PasswordStoreConsumer,
 
   FormSaver* form_saver() { return form_saver_.get(); }
 
+ protected:
+  // FormFetcher::Consumer:
+  void ProcessMatches(
+      const std::vector<const autofill::PasswordForm*>& non_federated,
+      size_t filtered_count) override;
+
  private:
   // ManagerAction - What does the manager do with this form? Either it
   // fills it, or it doesn't. If it doesn't fill it, that's either
@@ -321,11 +307,6 @@ class PasswordFormManager : public PasswordStoreConsumer,
   // |best_matches_|, |preferred_match_| and |non_best_matches_| accordingly.
   void ScoreMatches(const std::vector<const autofill::PasswordForm*>& matches);
 
-  // FormFetcher::Consumer:
-  void ProcessMatches(
-      const std::vector<const autofill::PasswordForm*>& non_federated,
-      size_t filtered_count) override;
-
   // Helper for Save in the case that best_matches.size() == 0, meaning
   // we have no prior record of this form/username/password and the user
   // has opted to 'Save Password'. The previously preferred login from
@@ -584,18 +565,14 @@ class PasswordFormManager : public PasswordStoreConsumer,
   // user has entered.
   FormType form_type_;
 
-  // False unless FetchMatchingLoginsFromPasswordStore has been called again
-  // without the password store returning results in the meantime.
-  bool need_to_refetch_;
-
   // FormSaver instance used by |this| to all tasks related to storing
   // credentials.
   std::unique_ptr<FormSaver> form_saver_;
 
   // TODO(crbug.com/621355) Remove this, ultimately the form fetcher will not be
   // owned by PasswordFormManager. Temporarily, this is the object which
-  // |form_fetcher_| points to.
-  FormFetcherImpl form_fetcher_impl_;
+  // |form_fetcher_| points to, unless set otherwise in the constructor.
+  std::unique_ptr<FormFetcherImpl> form_fetcher_impl_;
 
   // FormFetcher instance which owns the login data from PasswordStore.
   FormFetcher* const form_fetcher_;