From c03f29af597f6e9b38a6e0264124ca6f172942a2 Mon Sep 17 00:00:00 2001
From: engedy <engedy@chromium.org>
Date: Mon, 13 Apr 2015 09:03:22 -0700
Subject: [PATCH] Do not wait for username select when filling
 affiliation-based matches.

BUG=476519

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

Cr-Commit-Position: refs/heads/master@{#324856}
---
 .../core/browser/password_form_manager.cc          | 14 ++++++---
 .../core/browser/password_form_manager_unittest.cc | 34 +++++++++++++++++++++-
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/components/password_manager/core/browser/password_form_manager.cc b/components/password_manager/core/browser/password_form_manager.cc
index 64ade29c67b8..88fdc57496a2 100644
--- a/components/password_manager/core/browser/password_form_manager.cc
+++ b/components/password_manager/core/browser/password_form_manager.cc
@@ -16,6 +16,7 @@
 #include "components/autofill/core/browser/form_structure.h"
 #include "components/autofill/core/browser/validation.h"
 #include "components/autofill/core/common/password_form.h"
+#include "components/password_manager/core/browser/affiliation_utils.h"
 #include "components/password_manager/core/browser/browser_save_password_progress_logger.h"
 #include "components/password_manager/core/browser/password_manager.h"
 #include "components/password_manager/core/browser/password_manager_client.h"
@@ -492,10 +493,15 @@ void PasswordFormManager::ProcessFrame(
   // Note that we provide the choices but don't actually prefill a value if:
   // (1) we are in Incognito mode, (2) the ACTION paths don't match,
   // or (3) if it matched using public suffix domain matching.
-  bool wait_for_username = client_->IsOffTheRecord() ||
-                           observed_form_.action.GetWithEmptyPath() !=
-                               preferred_match_->action.GetWithEmptyPath() ||
-                           preferred_match_->IsPublicSuffixMatch();
+  // However, 2 and 3 should not apply to Android-based credentials found via
+  // affiliation-based matching (we want to autofill them).
+  // TODO(engedy): Clean this up. See: https://crbug.com/476519.
+  bool wait_for_username =
+      client_->IsOffTheRecord() ||
+      (!IsValidAndroidFacetURI(preferred_match_->original_signon_realm) &&
+       (observed_form_.action.GetWithEmptyPath() !=
+            preferred_match_->action.GetWithEmptyPath() ||
+        preferred_match_->IsPublicSuffixMatch()));
   if (wait_for_username)
     manager_action_ = kManagerActionNone;
   else
diff --git a/components/password_manager/core/browser/password_form_manager_unittest.cc b/components/password_manager/core/browser/password_form_manager_unittest.cc
index 6d332dbc6070..7a209082af3a 100644
--- a/components/password_manager/core/browser/password_form_manager_unittest.cc
+++ b/components/password_manager/core/browser/password_form_manager_unittest.cc
@@ -129,7 +129,7 @@ class TestPasswordManagerClient : public StubPasswordManagerClient {
 class TestPasswordManager : public PasswordManager {
  public:
   explicit TestPasswordManager(TestPasswordManagerClient* client)
-      : PasswordManager(client) {}
+      : PasswordManager(client), wait_for_username_(false) {}
 
   void Autofill(password_manager::PasswordManagerDriver* driver,
                 const autofill::PasswordForm& form_for_autofill,
@@ -137,15 +137,21 @@ class TestPasswordManager : public PasswordManager {
                 const autofill::PasswordForm& preferred_match,
                 bool wait_for_username) const override {
     best_matches_ = best_matches;
+    wait_for_username_ = wait_for_username;
   }
 
   const autofill::PasswordFormMap& GetLatestBestMatches() {
     return best_matches_;
   }
 
+  bool GetLatestWaitForUsername() { return wait_for_username_; }
+
  private:
   // Marked mutable to get around constness of Autofill().
+  // TODO(vabr): This should be rewritten as a mock of PasswordManager, and the
+  // interesting arguments should be saved via GMock actions instead.
   mutable autofill::PasswordFormMap best_matches_;
+  mutable bool wait_for_username_;
 };
 
 }  // namespace
@@ -1007,6 +1013,32 @@ TEST_F(PasswordFormManagerTest, TestScoringPublicSuffixMatch) {
                     ->second->original_signon_realm);
 }
 
+TEST_F(PasswordFormManagerTest, AndroidCredentialsAreAutofilled) {
+  EXPECT_CALL(*(client()->mock_driver()), AllowPasswordGenerationForForm(_));
+
+  TestPasswordManager password_manager(client());
+  PasswordFormManager manager(&password_manager, client(), client()->driver(),
+                              *observed_form(), false);
+
+  // Although Android-based credentials are treated similarly to PSL-matched
+  // credentials in most respects, they should be autofilled as opposed to be
+  // filled on username-select.
+  ScopedVector<PasswordForm> simulated_results;
+  simulated_results.push_back(new PasswordForm());
+  simulated_results[0]->signon_realm = observed_form()->signon_realm;
+  simulated_results[0]->original_signon_realm =
+      "android://hash@com.google.android";
+  simulated_results[0]->origin = observed_form()->origin;
+  simulated_results[0]->username_value = saved_match()->username_value;
+  simulated_results[0]->password_value = saved_match()->password_value;
+
+  manager.SimulateFetchMatchingLoginsFromPasswordStore();
+  manager.OnGetPasswordStoreResults(simulated_results.Pass());
+
+  EXPECT_EQ(1u, password_manager.GetLatestBestMatches().size());
+  EXPECT_FALSE(password_manager.GetLatestWaitForUsername());
+}
+
 TEST_F(PasswordFormManagerTest, InvalidActionURLsDoNotMatch) {
   PasswordFormManager manager(nullptr, client(), kNoDriver, *observed_form(),
                               false);
-- 
2.11.4.GIT