[Password manager] Recognise squashed login+sign-up forms

components/password_manager/core/browser/password_manager_unittest.cc

Index: components/password_manager/core/browser/password_manager_unittest.cc
diff --git a/components/password_manager/core/browser/password_manager_unittest.cc b/components/password_manager/core/browser/password_manager_unittest.cc
index a3cc8fc2cd5e185535fcbbe8030a4773d8b52119..5266521fa165fd61e8266494203c3c37e63ad2a9 100644
--- a/components/password_manager/core/browser/password_manager_unittest.cc
+++ b/components/password_manager/core/browser/password_manager_unittest.cc
@@ -42,6 +42,8 @@ class MockPasswordManagerClient : public StubPasswordManagerClient {
                      bool(const std::string&, const std::string&));
   MOCK_CONST_METHOD0(GetPasswordStore, PasswordStore*());
   MOCK_CONST_METHOD0(DidLastPageLoadEncounterSSLErrors, bool());
+  // The code inside EXPECT_CALL for PromptUserToSavePasswordPtr owns the
+  // PasswordFormManager* argument.
   MOCK_METHOD2(PromptUserToSavePasswordPtr,
                void(PasswordFormManager*, CredentialSourceType type));
   MOCK_METHOD1(AutomaticPasswordSavePtr, void(PasswordFormManager*));
@@ -823,10 +825,11 @@ TEST_F(PasswordManagerTest,
 }
 
 // Create a form with a new_password_element. Submit the form with the empty
-// new password value. It shouldn't overwrite the existing password.
+// new password value. It shouldn't attempt to save the empty password.
 TEST_F(PasswordManagerTest, DoNotUpdateWithEmptyPassword) {
   std::vector<PasswordForm> observed;
   PasswordForm form(MakeSimpleForm());
+  ASSERT_FALSE(form.password_value.empty());
   form.new_password_element = ASCIIToUTF16("new_password_element");
   form.new_password_value.clear();
   observed.push_back(form);
@@ -837,10 +840,11 @@ TEST_F(PasswordManagerTest, DoNotUpdateWithEmptyPassword) {
   // And the form submit contract is to call ProvisionallySavePassword.
   OnPasswordFormSubmitted(form);
 
+  scoped_ptr<PasswordFormManager> form_to_save;
   EXPECT_CALL(client_,
               PromptUserToSavePasswordPtr(
                   _, CredentialSourceType::CREDENTIAL_SOURCE_PASSWORD_MANAGER))
-      .Times(0);
+      .WillOnce(WithArg<0>(SaveToScopedPtr(&form_to_save)));
 
   // Now the password manager waits for the login to complete successfully.
   observed.clear();
@@ -848,6 +852,8 @@ TEST_F(PasswordManagerTest, DoNotUpdateWithEmptyPassword) {
                                    observed);  // The post-navigation load.
   manager()->OnPasswordFormsRendered(&driver_, observed,
                                      true);  // The post-navigation layout.
+  EXPECT_FALSE(PasswordFormManager::PasswordToSave(

[Garrett Casto, 2015/03/17 22:49:37]

What caused this change?

[vabr (Chromium), 2015/03/18 16:51:03]

Previously, if there was a "new password" field, it was unconditionally
considered as the only source of the password value. If empty, nothing was
offered for saving.

To support saving the login-part on a login+sign-up form, the code now is
willing to use the old password value, even in the presence of a new password
field, if the new password value is empty.

In this test that meant that there suddenly was a call to
PromptUserToSavePassword. The password to be saved was the old one, not the
(empty) new one (verified on this line, 855).

The bug checked by this test was about saving an empty password, which replaced
the old one. This does not happen after my CL -- instead of replacing the old
password with an empty password, the old one is "updated" with itself (no-op).
The important between-revisions-invariant to preserve is that we never save an
empty password.

(And of course, if there are both a non-empty old password value, and a
non-empty new password value, the non-empty new password value is saved.)

+                   form_to_save->pending_credentials()).empty());
 }
 
 TEST_F(PasswordManagerTest, FormSubmitWithOnlyPassowrdField) {