diff --git a/components/password_manager/core/browser/password_form_manager.cc b/components/password_manager/core/browser/password_form_manager.cc index 9ff8533e788f5..e1fee32d2f502 100644 --- a/components/password_manager/core/browser/password_form_manager.cc +++ b/components/password_manager/core/browser/password_form_manager.cc @@ -735,6 +735,7 @@ void PasswordFormManager::UpdateLogin() { password_store->UpdateLoginWithPrimaryKey(pending_credentials_, old_primary_key); } else if (observed_form_.new_password_element.empty() && + !IsValidAndroidFacetURI(pending_credentials_.signon_realm) && (pending_credentials_.password_element.empty() || pending_credentials_.username_element.empty() || pending_credentials_.submit_element.empty())) { @@ -991,8 +992,7 @@ void PasswordFormManager::CreatePendingCredentials() { pending_credentials_ = *saved_form; password_overridden_ = pending_credentials_.password_value != password_to_save; - if (IsPendingCredentialsPublicSuffixMatch() || - IsValidAndroidFacetURI(preferred_match_->signon_realm)) { + if (IsPendingCredentialsPublicSuffixMatch()) { // If the autofilled credentials were a PSL match or credentials stored // from Android apps store a copy with the current origin and signon // realm. This ensures that on the next visit, a precise match is found. @@ -1089,13 +1089,14 @@ void PasswordFormManager::CreatePendingCredentials() { CreatePendingCredentialsForNewCredentials(); } - pending_credentials_.action = provisionally_saved_form_->action; - - // If the user selected credentials we autofilled from a PasswordForm - // that contained no action URL (IE6/7 imported passwords, for example), - // bless it with the action URL from the observed form. See bug 1107719. - if (pending_credentials_.action.is_empty()) - pending_credentials_.action = observed_form_.action; + if (!IsValidAndroidFacetURI(pending_credentials_.signon_realm)) { + pending_credentials_.action = provisionally_saved_form_->action; + // If the user selected credentials we autofilled from a PasswordForm + // that contained no action URL (IE6/7 imported passwords, for example), + // bless it with the action URL from the observed form. See bug 1107719. + if (pending_credentials_.action.is_empty()) + pending_credentials_.action = observed_form_.action; + } pending_credentials_.password_value = password_to_save; pending_credentials_.preferred = provisionally_saved_form_->preferred; 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 e9e1571c00028..56af53edb53cf 100644 --- a/components/password_manager/core/browser/password_form_manager_unittest.cc +++ b/components/password_manager/core/browser/password_form_manager_unittest.cc @@ -1321,25 +1321,56 @@ TEST_F(PasswordFormManagerTest, AndroidCredentialsAreAutofilled) { EXPECT_CALL(*(client()->mock_driver()), AllowPasswordGenerationForForm(_)); // Although Android-based credentials are treated similarly to PSL-matched - // credentials in most respects, they should be autofilled as opposed to be + // credentials in some respects, they should be autofilled as opposed to be // filled on username-select. - ScopedVector simulated_results; - simulated_results.push_back(new PasswordForm()); - simulated_results[0]->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; - - form_manager()->SimulateFetchMatchingLoginsFromPasswordStore(); + PasswordForm android_login; + android_login.signon_realm = "android://hash@com.google.android"; + android_login.origin = GURL("android://hash@com.google.android/"); + android_login.is_affiliation_based_match = true; + android_login.username_value = saved_match()->username_value; + android_login.password_value = saved_match()->password_value; + android_login.preferred = false; + android_login.times_used = 42; autofill::PasswordFormFillData fill_data; EXPECT_CALL(*client()->mock_driver(), FillPasswordForm(_)) .WillOnce(SaveArg<0>(&fill_data)); + ScopedVector simulated_results; + simulated_results.push_back(new PasswordForm(android_login)); + form_manager()->SimulateFetchMatchingLoginsFromPasswordStore(); form_manager()->OnGetPasswordStoreResults(std::move(simulated_results)); EXPECT_TRUE(fill_data.additional_logins.empty()); EXPECT_FALSE(fill_data.wait_for_username); EXPECT_EQ(1u, form_manager()->best_matches().size()); + + // When the user submits the filled form, no copy of the credential should be + // created, instead the usage counter of the original credential should be + // incremented in-place, as if it were a regular credential for that website. + PasswordForm credential(*observed_form()); + credential.username_value = android_login.username_value; + credential.password_value = android_login.password_value; + credential.preferred = true; + form_manager()->ProvisionallySave( + credential, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); + EXPECT_FALSE(form_manager()->IsNewLogin()); + + PasswordForm updated_credential; + EXPECT_CALL(*mock_store(), UpdateLogin(_)) + .WillOnce(testing::SaveArg<0>(&updated_credential)); + EXPECT_CALL(*mock_store(), UpdateLoginWithPrimaryKey(_, _)).Times(0); + EXPECT_CALL(*mock_store(), AddLogin(_)).Times(0); + form_manager()->Save(); + Mock::VerifyAndClearExpectations(mock_store()); + + EXPECT_EQ(android_login.username_value, updated_credential.username_value); + EXPECT_EQ(android_login.password_value, updated_credential.password_value); + EXPECT_EQ(android_login.times_used + 1, updated_credential.times_used); + EXPECT_TRUE(updated_credential.preferred); + EXPECT_EQ(GURL(), updated_credential.action); + EXPECT_EQ(base::string16(), updated_credential.username_element); + EXPECT_EQ(base::string16(), updated_credential.password_element); + EXPECT_EQ(base::string16(), updated_credential.submit_element); } // Credentials saved through Android apps should always be shown in the drop- diff --git a/components/password_manager/core/browser/password_manager_unittest.cc b/components/password_manager/core/browser/password_manager_unittest.cc index 734b15a06249e..31b3f6c6cda87 100644 --- a/components/password_manager/core/browser/password_manager_unittest.cc +++ b/components/password_manager/core/browser/password_manager_unittest.cc @@ -1313,6 +1313,22 @@ TEST_F(PasswordManagerTest, AutofillingOfAffiliatedCredentials) { EXPECT_EQ(android_form.password_value, form_data.password_field.value); EXPECT_FALSE(form_data.wait_for_username); EXPECT_EQ(android_form.signon_realm, form_data.preferred_realm); + + EXPECT_CALL(client_, IsSavingAndFillingEnabledForCurrentPage()) + .WillRepeatedly(Return(true)); + + PasswordForm filled_form(form); + filled_form.username_value = android_form.username_value; + filled_form.password_value = android_form.password_value; + OnPasswordFormSubmitted(filled_form); + + observed.clear(); + EXPECT_CALL(*store_, UpdateLogin(_)); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_, _)).Times(0); + EXPECT_CALL(*store_, AddLogin(_)).Times(0); + EXPECT_CALL(*store_, UpdateLoginWithPrimaryKey(_, _)).Times(0); + manager()->OnPasswordFormsParsed(&driver_, observed); + manager()->OnPasswordFormsRendered(&driver_, observed, true); } } // namespace password_manager