Credential Manager: Switch 'request()' to GetAutofillableLogins().

This patch changes the CredentialManagerDispatcher implementation of
'request()' to grab all the credentials from the PasswordStore, rather
than just those that match the origin of the currently committed
WebContents. This change is the first step towards supporting federations.

Rather than doing multiple requests to the PasswordStore for the current
origin, and each of the federation origins, we'll grab everything, and
filter the result list before handing it back to the UI for display.

BUG=400674

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

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

components/password_manager/content/browser/content_credential_manager_dispatcher.cc

@@ -5,6 +5,7 @@
 #include "components/password_manager/content/browser/content_credential_manager_dispatcher.h"
 
 #include "base/bind.h"
+#include "base/memory/scoped_vector.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/core/common/password_form.h"
@@ -21,12 +22,26 @@
 
 namespace password_manager {
 
+struct ContentCredentialManagerDispatcher::PendingRequestParameters {
+  PendingRequestParameters(int request_id,
+                           bool request_zero_click_only,
+                           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;
+};
+
 ContentCredentialManagerDispatcher::ContentCredentialManagerDispatcher(
     content::WebContents* web_contents,
     PasswordManagerClient* client)
-    : WebContentsObserver(web_contents),
-      client_(client),
-      pending_request_id_(0) {
+    : WebContentsObserver(web_contents), client_(client) {
   DCHECK(web_contents);
 }
 
@@ -93,43 +108,59 @@ void ContentCredentialManagerDispatcher::OnNotifySignedOut(int request_id) {
 
 void ContentCredentialManagerDispatcher::OnRequestCredential(
     int request_id,
-    bool /* zero_click_only */,
-    const std::vector<GURL>& /* federations */) {
+    bool zero_click_only,
+    const std::vector<GURL>& federations) {
   DCHECK(request_id);
   PasswordStore* store = GetPasswordStore();
-  if (pending_request_id_ || !store) {
+  if (pending_request_ || !store) {
     web_contents()->GetRenderViewHost()->Send(
         new CredentialManagerMsg_RejectCredentialRequest(
-            web_contents()->GetRenderViewHost()->GetRoutingID(),
-            request_id,
-            pending_request_id_
+            web_contents()->GetRenderViewHost()->GetRoutingID(), request_id,
+            pending_request_
                 ? blink::WebCredentialManagerError::ErrorTypePendingRequest
                 : blink::WebCredentialManagerError::
                       ErrorTypePasswordStoreUnavailable));
     return;
   }
 
-  pending_request_id_ = request_id;
-
-  // TODO(mkwst): we should deal with federated login types.
-  autofill::PasswordForm form;
-  form.scheme = autofill::PasswordForm::SCHEME_HTML;
-  form.origin = web_contents()->GetLastCommittedURL().GetOrigin();
-  form.signon_realm = form.origin.spec();
+  pending_request_.reset(new PendingRequestParameters(
+      request_id, zero_click_only,
+      web_contents()->GetLastCommittedURL().GetOrigin(), federations));
 
-  store->GetLogins(form, PasswordStore::DISALLOW_PROMPT, this);
+  store->GetAutofillableLogins(this);
 }
 
 void ContentCredentialManagerDispatcher::OnGetPasswordStoreResults(
     const std::vector<autofill::PasswordForm*>& results) {
-  DCHECK(pending_request_id_);
-
-  if (results.empty() ||
-      !client_->PromptUserToChooseCredentials(results, base::Bind(
-          &ContentCredentialManagerDispatcher::SendCredential,
-          base::Unretained(this),
-          pending_request_id_))) {
-    SendCredential(pending_request_id_, CredentialInfo());
+  DCHECK(pending_request_);
+
+  // We own the PasswordForm instances, so we're responsible for cleaning
+  // up the instances we don't add to |filtered_results|. We'll dump them
+  // into a ScopedVector and allow it to delete the PasswordForms upon
+  // destruction.
+  std::vector<autofill::PasswordForm*> filtered_results;
+  ScopedVector<autofill::PasswordForm> discarded_results;
+  for (autofill::PasswordForm* form : results) {
+    // TODO(mkwst): Extend this filter to include federations.
+    if (form->origin == pending_request_->origin) {
+      filtered_results.push_back(form);
+    } else {
+      discarded_results.push_back(form);
+    }
+  }
+
+  if (filtered_results.empty() ||
+      web_contents()->GetLastCommittedURL().GetOrigin() !=
+          pending_request_->origin) {
+    SendCredential(pending_request_->id, CredentialInfo());
+    return;
+  }
+
+  if (!client_->PromptUserToChooseCredentials(
+          filtered_results,
+          base::Bind(&ContentCredentialManagerDispatcher::SendCredential,
+                     base::Unretained(this), pending_request_->id))) {
+    SendCredential(pending_request_->id, CredentialInfo());
   }
 }
 
@@ -149,14 +180,13 @@ ContentCredentialManagerDispatcher::GetDriver() {
 
 void ContentCredentialManagerDispatcher::SendCredential(
     int request_id, const CredentialInfo& info) {
-  DCHECK(pending_request_id_);
-  DCHECK_EQ(pending_request_id_, request_id);
+  DCHECK(pending_request_);
+  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_ = 0;
+          pending_request_->id, info));
+  pending_request_.reset();
 }
 
 }  // namespace password_manager

components/password_manager/content/browser/content_credential_manager_dispatcher.h

@@ -7,6 +7,7 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "components/password_manager/core/browser/credential_manager_dispatcher.h"
 #include "components/password_manager/core/browser/password_store_consumer.h"
@@ -61,6 +62,8 @@ class ContentCredentialManagerDispatcher : public CredentialManagerDispatcher,
       base::Callback<void(const autofill::PasswordForm&)>;
 
  private:
+  struct PendingRequestParameters;
+
   PasswordStore* GetPasswordStore();
 
   // Returns the driver for the current main frame.
@@ -73,9 +76,9 @@ class ContentCredentialManagerDispatcher : public CredentialManagerDispatcher,
   scoped_ptr<CredentialManagerPasswordFormManager> form_manager_;
 
   // When 'OnRequestCredential' is called, it in turn calls out to the
-  // PasswordStore; we store the request ID here in order to properly respond
-  // to the request once the PasswordStore gives us data.
-  int pending_request_id_;
+  // 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_;
 
   DISALLOW_COPY_AND_ASSIGN(ContentCredentialManagerDispatcher);
 };

components/password_manager/content/browser/content_credential_manager_dispatcher_unittest.cc

@@ -138,6 +138,13 @@ class ContentCredentialManagerDispatcherTest
     form_.signon_realm = form_.origin.spec();
     form_.scheme = autofill::PasswordForm::SCHEME_HTML;
 
+    cross_origin_form_.username_value = base::ASCIIToUTF16("Username");
+    cross_origin_form_.display_name = base::ASCIIToUTF16("Display Name");
+    cross_origin_form_.password_value = base::ASCIIToUTF16("Password");
+    cross_origin_form_.origin = GURL("https://example.net/");
+    cross_origin_form_.signon_realm = cross_origin_form_.origin.spec();
+    cross_origin_form_.scheme = autofill::PasswordForm::SCHEME_HTML;
+
     store_->Clear();
     EXPECT_TRUE(store_->IsEmpty());
   }
@@ -151,6 +158,7 @@ class ContentCredentialManagerDispatcherTest
 
  protected:
   autofill::PasswordForm form_;
+  autofill::PasswordForm cross_origin_form_;
   scoped_refptr<TestPasswordStore> store_;
   scoped_ptr<ContentCredentialManagerDispatcher> dispatcher_;
   scoped_ptr<TestPasswordManagerClient> client_;
@@ -227,6 +235,26 @@ TEST_F(ContentCredentialManagerDispatcherTest,
   EXPECT_FALSE(client_->did_prompt_user_to_choose());
 }
 
+TEST_F(ContentCredentialManagerDispatcherTest,
+       CredentialManagerOnRequestCredentialWithCrossOriginPasswordStore) {
+  store_->AddLogin(cross_origin_form_);
+
+  std::vector<GURL> federations;
+  dispatcher()->OnRequestCredential(kRequestId, false, federations);
+
+  RunAllPendingTasks();
+
+  const uint32 kMsgID = CredentialManagerMsg_SendCredential::ID;
+  const IPC::Message* message =
+      process()->sink().GetFirstMessageMatching(kMsgID);
+  EXPECT_TRUE(message);
+  CredentialManagerMsg_SendCredential::Param param;
+  CredentialManagerMsg_SendCredential::Read(message, &param);
+  EXPECT_EQ(CREDENTIAL_TYPE_EMPTY, param.b.type);
+  process()->sink().ClearMessages();
+  EXPECT_FALSE(client_->did_prompt_user_to_choose());
+}
+
 TEST_F(ContentCredentialManagerDispatcherTest,
        CredentialManagerOnRequestCredentialWithFullPasswordStore) {
   store_->AddLogin(form_);

components/password_manager/core/browser/test_password_store.cc

@@ -49,6 +49,15 @@ void TestPasswordStore::WrapModificationTask(ModificationTask task) {
   task.Run();
 }
 
+void TestPasswordStore::GetAutofillableLoginsImpl(
+    PasswordStore::GetLoginsRequest* request) {
+  for (auto& forms_for_realm : stored_passwords_) {
+    for (const autofill::PasswordForm& form : forms_for_realm.second)
+      request->result()->push_back(new autofill::PasswordForm(form));
+  }
+  ForwardLoginsResult(request);
+}
+
 PasswordStoreChangeList TestPasswordStore::AddLoginImpl(
     const autofill::PasswordForm& form) {
   PasswordStoreChangeList changes;

components/password_manager/core/browser/test_password_store.h

@@ -51,6 +51,8 @@ class TestPasswordStore : public PasswordStore {
                      PasswordStore::AuthorizationPromptPolicy prompt_policy,
                      const ConsumerCallbackRunner& runner) override;
   void WrapModificationTask(ModificationTask task) override;
+  void GetAutofillableLoginsImpl(
+      PasswordStore::GetLoginsRequest* request) override;
 
   // Unused portions of PasswordStore interface
   void ReportMetricsImpl(const std::string& sync_username,
@@ -61,8 +63,6 @@ class TestPasswordStore : public PasswordStore {
   PasswordStoreChangeList RemoveLoginsSyncedBetweenImpl(
       base::Time delete_begin,
       base::Time delete_end) override;
-  void GetAutofillableLoginsImpl(
-      PasswordStore::GetLoginsRequest* request) override {}
   void GetBlacklistLoginsImpl(
       PasswordStore::GetLoginsRequest* request) override {}
   bool FillAutofillableLogins(