Credential Manager: Introduce PendingRequestTask.

Currently, CredentialManagerDispatcher is a PasswordStoreObserver, and
handles responses from `request()` directly. We can't do that going
forward, as we also need to handle responses when dealing with (at
least) `notifySignedOut()`.

This patch splits the password store observer functionality out into
a new `PendingRequestTask` class, with the intent of introducing
`PendingSignedOutTask`, etc. classes in the future.

BUG=450581
R=vabr@chromium.org

Review URL: https://codereview.chromium.org/886793002

Cr-Commit-Position: refs/heads/master@{#314154}

components/password_manager/content/browser/credential_manager_dispatcher.cc

@@ -23,20 +23,83 @@
 
 namespace password_manager {
 
-struct CredentialManagerDispatcher::PendingRequestParameters {
-  PendingRequestParameters(int request_id,
+class CredentialManagerDispatcher::PendingRequestTask
+    : public PasswordStoreConsumer {
+ public:
+  PendingRequestTask(CredentialManagerDispatcher* const dispatcher,
+                     int request_id,
                      bool request_zero_click_only,
-                           GURL request_origin,
+                     const GURL& request_origin,
                      const std::vector<GURL>& request_federations)
-      : id(request_id),
-        zero_click_only(request_zero_click_only),
-        origin(request_origin),
-        federations(request_federations) {}
-
-  int id;
-  bool zero_click_only;
-  GURL origin;
-  std::vector<GURL> federations;
+      : dispatcher_(dispatcher),
+        id_(request_id),
+        zero_click_only_(request_zero_click_only),
+        origin_(request_origin) {
+    for (const GURL& origin : request_federations)
+      federations_.insert(origin.spec());
+  }
+
+  int id() const { return id_; }
+
+  // PasswordStoreConsumer implementation.
+  void OnGetPasswordStoreResults(
+      const std::vector<autofill::PasswordForm*>& results) override {
+    // We own the PasswordForm instances, so we're responsible for cleaning
+    // up the instances we don't add to |local_results| or |federated_results|.
+    //
+    // TODO(mkwst): Switch this and PromptUserToChooseCredentials() to use
+    // ScopedVector.
+    std::vector<autofill::PasswordForm*> local_results;
+    std::vector<autofill::PasswordForm*> federated_results;
+    for (autofill::PasswordForm* form : results) {
+      if (form->origin == origin_)
+        local_results.push_back(form);
+      else if (federations_.count(form->origin.spec()))
+        federated_results.push_back(form);
+      else
+        delete form;
+    }
+
+    if ((local_results.empty() && federated_results.empty()) ||
+        dispatcher_->web_contents()->GetLastCommittedURL().GetOrigin() !=
+            origin_) {
+      dispatcher_->SendCredential(id_, CredentialInfo());
+      return;
+    }
+    if (local_results.size() == 1 && dispatcher_->IsZeroClickAllowed()) {
+      // TODO(mkwst): Use the `one_time_disable_zero_click` flag on the result
+      // to prevent auto-sign-in, once that flag is implemented.
+      CredentialInfo info(*local_results[0],
+                          local_results[0]->federation_url.is_empty()
+                              ? CredentialType::CREDENTIAL_TYPE_LOCAL
+                              : CredentialType::CREDENTIAL_TYPE_FEDERATED);
+      STLDeleteElements(&local_results);
+      STLDeleteElements(&federated_results);
+      dispatcher_->SendCredential(id_, info);
+      return;
+    }
+
+    if (zero_click_only_ ||
+        !dispatcher_->client()->PromptUserToChooseCredentials(
+            local_results, federated_results,
+            base::Bind(&CredentialManagerDispatcher::SendCredential,
+                       base::Unretained(dispatcher_), id_))) {
+      STLDeleteElements(&local_results);
+      STLDeleteElements(&federated_results);
+      dispatcher_->SendCredential(id_, CredentialInfo());
+    }
+  }
+
+ private:
+  // Backlink to the CredentialManagerDispatcher that owns this object.
+  CredentialManagerDispatcher* const dispatcher_;
+
+  const int id_;
+  const bool zero_click_only_;
+  const GURL origin_;
+  std::set<std::string> federations_;
+
+  DISALLOW_COPY_AND_ASSIGN(PendingRequestTask);
 };
 
 CredentialManagerDispatcher::CredentialManagerDispatcher(
@@ -136,64 +199,13 @@ void CredentialManagerDispatcher::OnRequestCredential(
     return;
   }
 
-  pending_request_.reset(new PendingRequestParameters(
-      request_id, zero_click_only,
+  pending_request_.reset(new PendingRequestTask(
+      this, request_id, zero_click_only,
       web_contents()->GetLastCommittedURL().GetOrigin(), federations));
 
-  // This will result in a callback to ::OnGetPasswordStoreResults().
-  store->GetAutofillableLogins(this);
-}
-
-void CredentialManagerDispatcher::OnGetPasswordStoreResults(
-    const std::vector<autofill::PasswordForm*>& results) {
-  DCHECK(pending_request_);
-
-  std::set<std::string> federations;
-  for (const GURL& origin : pending_request_->federations)
-    federations.insert(origin.spec());
-
-  // We own the PasswordForm instances, so we're responsible for cleaning
-  // up the instances we don't add to |local_results| or |federated_results|.
-  std::vector<autofill::PasswordForm*> local_results;
-  std::vector<autofill::PasswordForm*> federated_results;
-  for (autofill::PasswordForm* form : results) {
-    if (form->origin == pending_request_->origin)
-      local_results.push_back(form);
-    else if (federations.count(form->origin.spec()) != 0)
-      federated_results.push_back(form);
-    else
-      delete form;
-  }
-
-  if ((local_results.empty() && federated_results.empty()) ||
-      web_contents()->GetLastCommittedURL().GetOrigin() !=
-          pending_request_->origin) {
-    SendCredential(pending_request_->id, CredentialInfo());
-    return;
-  }
-
-  if (local_results.size() == 1 && IsZeroClickAllowed()) {
-    // TODO(mkwst): Use the `one_time_disable_zero_click` flag on the result
-    // to prevent auto-sign-in, once that flag is implemented.
-    CredentialInfo info(*local_results[0],
-                        local_results[0]->federation_url.is_empty()
-                            ? CredentialType::CREDENTIAL_TYPE_LOCAL
-                            : CredentialType::CREDENTIAL_TYPE_FEDERATED);
-    STLDeleteElements(&local_results);
-    STLDeleteElements(&federated_results);
-    SendCredential(pending_request_->id, info);
-    return;
-  }
-
-  if (pending_request_->zero_click_only ||
-      !client_->PromptUserToChooseCredentials(
-          local_results, federated_results,
-          base::Bind(&CredentialManagerDispatcher::SendCredential,
-                     base::Unretained(this), pending_request_->id))) {
-    STLDeleteElements(&local_results);
-    STLDeleteElements(&federated_results);
-    SendCredential(pending_request_->id, CredentialInfo());
-  }
+  // This will result in a callback to
+  // PendingRequestTask::OnGetPasswordStoreResults().
+  store->GetAutofillableLogins(pending_request_.get());
 }
 
 PasswordStore* CredentialManagerDispatcher::GetPasswordStore() {
@@ -221,11 +233,11 @@ base::WeakPtr<PasswordManagerDriver> CredentialManagerDispatcher::GetDriver() {
 void CredentialManagerDispatcher::SendCredential(int request_id,
                                                  const CredentialInfo& info) {
   DCHECK(pending_request_);
-  DCHECK_EQ(pending_request_->id, request_id);
+  DCHECK_EQ(pending_request_->id(), request_id);
   web_contents()->GetRenderViewHost()->Send(
       new CredentialManagerMsg_SendCredential(
           web_contents()->GetRenderViewHost()->GetRoutingID(),
-          pending_request_->id, info));
+          pending_request_->id(), info));
   pending_request_.reset();
 }
 

components/password_manager/content/browser/credential_manager_dispatcher.h

@@ -31,8 +31,7 @@ class PasswordManagerDriver;
 class PasswordStore;
 struct CredentialInfo;
 
-class CredentialManagerDispatcher : public content::WebContentsObserver,
-                                    public PasswordStoreConsumer {
+class CredentialManagerDispatcher : public content::WebContentsObserver {
  public:
   CredentialManagerDispatcher(content::WebContents* web_contents,
                               PasswordManagerClient* client);
@@ -56,6 +55,9 @@ class CredentialManagerDispatcher : public content::WebContentsObserver,
 
   // Called in response to an IPC from the renderer, triggered by a page's call
   // to 'navigator.credentials.request'.
+  //
+  // TODO(vabr): Determine if we can drop the `const` here to save some copies
+  // while processing the request.
   virtual void OnRequestCredential(int request_id,
                                    bool zero_click_only,
                                    const std::vector<GURL>& federations);
@@ -63,15 +65,13 @@ class CredentialManagerDispatcher : public content::WebContentsObserver,
   // content::WebContentsObserver implementation.
   bool OnMessageReceived(const IPC::Message& message) override;
 
-  // PasswordStoreConsumer implementation.
-  void OnGetPasswordStoreResults(
-      const std::vector<autofill::PasswordForm*>& results) override;
-
   using CredentialCallback =
       base::Callback<void(const autofill::PasswordForm&)>;
 
+  PasswordManagerClient* client() const { return client_; }
+
  private:
-  struct PendingRequestParameters;
+  class PendingRequestTask;
 
   PasswordStore* GetPasswordStore();
 
@@ -91,9 +91,10 @@ class CredentialManagerDispatcher : public content::WebContentsObserver,
   BooleanPrefMember auto_signin_enabled_;
 
   // When 'OnRequestCredential' is called, it in turn calls out to the
-  // PasswordStore; we store request details here in order to properly
-  // respond to the request once the PasswordStore gives us data.
-  scoped_ptr<PendingRequestParameters> pending_request_;
+  // PasswordStore; we push enough data into Pending*Task objects so that
+  // they can properly respond to the request once the PasswordStore gives
+  // us data.
+  scoped_ptr<PendingRequestTask> pending_request_;
 
   DISALLOW_COPY_AND_ASSIGN(CredentialManagerDispatcher);
 };