[PasswordManager] Improve detection of ignorable change password forms.

components/password_manager/core/browser/password_form_manager_unittest.cc

Index: components/password_manager/core/browser/password_form_manager_unittest.cc
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 5545a04ab89b0802ead8a72cc7342c194a89a479..c6ea5539a3dfde4990bb72ea37d4e8608b9c0af7 100644
--- a/components/password_manager/core/browser/password_form_manager_unittest.cc
+++ b/components/password_manager/core/browser/password_form_manager_unittest.cc
@@ -381,6 +381,7 @@ TEST_F(PasswordFormManagerTest, TestNewLoginFromNewPasswordElement) {
   // Add a new password field to the test form. The PasswordFormManager should
   // save the password from this field, instead of the current password field.
   observed_form()->new_password_element = ASCIIToUTF16("NewPasswd");
+  observed_form()->username_marked_by_site = true;
 
   PasswordFormManager manager(NULL, client(), kNoDriver, *observed_form(),
                               false);
@@ -1346,4 +1347,80 @@ TEST_F(PasswordFormManagerTest, PreferredMatchIsUpToDate) {
   PasswordForm dummy(*form_manager.preferred_match());
 }
 
+TEST_F(PasswordFormManagerTest,
+       SubmitIngnorableChangePasswordForm_MatchingUsernameAndPassword) {

[vabr (Chromium), 2015/02/23 10:15:26]

nit: Please rename the new tests by replacing "Submit" with "Is" to match the
tested method name.

[Pritam Nikam, 2015/02/23 11:27:52]

Done.

+  observed_form()->new_password_element =
+      base::ASCIIToUTF16("new_password_field");
+
+  TestPasswordManagerClient client_with_store(mock_store());
+  PasswordFormManager manager(nullptr, &client_with_store,
+                              client_with_store.driver(), *observed_form(),
+                              false);
+  SimulateMatchingPhase(&manager, RESULT_MATCH_FOUND);
+
+  // The user submits a password on a change-password form, which does not use
+  // the "autocomplete=username" mark-up (therefore Chrome had to guess what is
+  // the username), but the user-typed credentials match something already
+  // stored (which confirms that the guess was right).
+  PasswordForm credentials(*observed_form());
+  credentials.username_value = saved_match()->username_value;
+  credentials.password_value = saved_match()->password_value;
+  credentials.new_password_value = ASCIIToUTF16("NewPassword");
+
+  EXPECT_FALSE(manager.IsIgnorableChangePasswordForm(
+      credentials.username_value, credentials.password_value));
+  manager.ProvisionallySave(

[vabr (Chromium), 2015/02/23 10:15:26]

I suggest to drop the lines 1372-1383. They don't involve any code using
IsIgnorableChangePasswordForm. The test should test just one thing
(IsIgnorableChangePasswordForm), not more (e.g., whether ProvisionallySave
works), unless they are inseparable.

[Pritam Nikam, 2015/02/23 11:27:52]

Done.

+      credentials, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES);
+
+  EXPECT_EQ(credentials.signon_realm,
+            GetPendingCredentials(&manager)->signon_realm);
+  EXPECT_EQ(credentials.username_value,
+            GetPendingCredentials(&manager)->username_value);
+
+  // By this point, the PasswordFormManager should have overwritten the new
+  // password value to be the current password.
+  EXPECT_EQ(credentials.new_password_value,
+            GetPendingCredentials(&manager)->password_value);
+}
+
+TEST_F(PasswordFormManagerTest,
+       SubmitIngnorableChangePasswordForm_NotMatchingPassword) {
+  observed_form()->new_password_element =
+      base::ASCIIToUTF16("new_password_field");
+
+  TestPasswordManagerClient client_with_store(mock_store());
+  PasswordFormManager manager(nullptr, &client_with_store,
+                              client_with_store.driver(), *observed_form(),
+                              false);
+  SimulateMatchingPhase(&manager, RESULT_MATCH_FOUND);
+
+  // The user submits a password on a change-password form, which does not use
+  // the "autocomplete=username" mark-up (therefore Chrome had to guess what is
+  // the username), and the user-typed password do not match anything already
+  // stored. There is not much confidence in the guess being right, so the
+  // password should not be stored.
+  EXPECT_TRUE(manager.IsIgnorableChangePasswordForm(
+      saved_match()->username_value, ASCIIToUTF16("DifferentPassword")));
+}
+
+TEST_F(PasswordFormManagerTest,
+       SubmitIngnorableChangePasswordForm_NotMatchingUsername) {
+  observed_form()->new_password_element =
+      base::ASCIIToUTF16("new_password_field");
+
+  TestPasswordManagerClient client_with_store(mock_store());
+  PasswordFormManager manager(nullptr, &client_with_store,
+                              client_with_store.driver(), *observed_form(),
+                              false);
+  SimulateMatchingPhase(&manager, RESULT_MATCH_FOUND);
+
+  // The user submits a password on a change-password form, which does not use
+  // the "autocomplete=username" mark-up (therefore Chrome had to guess what is
+  // the username), and the user-typed username does not match anything already
+  // stored. There is not much confidence in the guess being right, so the
+  // password should not be stored.
+  EXPECT_TRUE(manager.IsIgnorableChangePasswordForm(
+      ASCIIToUTF16("DifferentUsername"), saved_match()->password_value));
+}
+
 }  // namespace password_manager