[Password Manager] Fix password saving on Macys registration page

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 ddf396fdaebc442c5582ca620e5d95d75a7679d9..e758260cd862fe773c54685b61f279a05d7a79ef 100644
--- a/components/password_manager/core/browser/password_form_manager_unittest.cc
+++ b/components/password_manager/core/browser/password_form_manager_unittest.cc
@@ -397,7 +397,6 @@ TEST_F(PasswordFormManagerTest, PSLMatchedCredentialsMetadataUpdated) {
   submitted_form.preferred = true;
   submitted_form.username_value = saved_match()->username_value;
   submitted_form.password_value = saved_match()->password_value;
-  submitted_form.origin = saved_match()->origin;
   form_manager.ProvisionallySave(
       submitted_form, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES);
 
@@ -405,6 +404,7 @@ TEST_F(PasswordFormManagerTest, PSLMatchedCredentialsMetadataUpdated) {
   expected_saved_form.times_used = 1;
   expected_saved_form.other_possible_usernames.clear();
   expected_saved_form.form_data = saved_match()->form_data;
+  expected_saved_form.origin = saved_match()->origin;

[Garrett Casto, 2015/02/25 01:10:17]

This change tickled this bug. This previously worked because the observed and
submitted forms matched their actions even though the origins were different. In
reality this should never happen, as the origin of these two should always be
the same for PasswordManager to think that this particular PasswordFormManager
is the correct one.

[vabr (Chromium), 2015/02/25 10:21:15]

Acknowledged.

   PasswordForm actual_saved_form;
 
   EXPECT_CALL(*(client_with_store.mock_driver()->mock_autofill_manager()),
@@ -1057,17 +1057,15 @@ TEST_F(PasswordFormManagerTest, NonHTMLFormsDoNotMatchHTMLForms) {
   ASSERT_EQ(PasswordForm::SCHEME_HTML, observed_form()->scheme);
   PasswordForm non_html_form(*observed_form());
   non_html_form.scheme = PasswordForm::SCHEME_DIGEST;
-  EXPECT_EQ(0,
-            manager.DoesManage(non_html_form) &
-                PasswordFormManager::RESULT_MANDATORY_ATTRIBUTES_MATCH);
+  EXPECT_EQ(0, manager.DoesManage(non_html_form) &
+                   PasswordFormManager::RESULT_HTML_ATTRIBUTES_MATCH);
 
   // The other way round: observing a non-HTML form, don't match a HTML form.
   PasswordForm html_form(*observed_form());
   PasswordFormManager non_html_manager(NULL, client(), kNoDriver, non_html_form,
                                        false);
-  EXPECT_EQ(0,
-            non_html_manager.DoesManage(html_form) &
-                PasswordFormManager::RESULT_MANDATORY_ATTRIBUTES_MATCH);
+  EXPECT_EQ(0, non_html_manager.DoesManage(html_form) &
+                   PasswordFormManager::RESULT_HTML_ATTRIBUTES_MATCH);
 }
 
 TEST_F(PasswordFormManagerTest, OriginCheck_HostsMatchExactly) {
@@ -1078,9 +1076,8 @@ TEST_F(PasswordFormManagerTest, OriginCheck_HostsMatchExactly) {
   PasswordForm form_longer_host(*observed_form());
   form_longer_host.origin = GURL("http://accounts.google.com.au/a/LoginAuth");
   // Check that accounts.google.com does not match accounts.google.com.au.
-  EXPECT_EQ(0,
-            manager.DoesManage(form_longer_host) &
-                PasswordFormManager::RESULT_MANDATORY_ATTRIBUTES_MATCH);
+  EXPECT_EQ(0, manager.DoesManage(form_longer_host) &
+                   PasswordFormManager::RESULT_HTML_ATTRIBUTES_MATCH);
 }
 
 TEST_F(PasswordFormManagerTest, OriginCheck_MoreSecureSchemePathsMatchPrefix) {
@@ -1091,9 +1088,8 @@ TEST_F(PasswordFormManagerTest, OriginCheck_MoreSecureSchemePathsMatchPrefix) {
 
   PasswordForm form_longer_path(*observed_form());
   form_longer_path.origin = GURL("https://accounts.google.com/a/LoginAuth/sec");
-  EXPECT_NE(0,
-            manager.DoesManage(form_longer_path) &
-                PasswordFormManager::RESULT_MANDATORY_ATTRIBUTES_MATCH);
+  EXPECT_NE(0, manager.DoesManage(form_longer_path) &
+                   PasswordFormManager::RESULT_HTML_ATTRIBUTES_MATCH);
 }
 
 TEST_F(PasswordFormManagerTest,
@@ -1106,9 +1102,8 @@ TEST_F(PasswordFormManagerTest,
   PasswordForm form_longer_path(*observed_form());
   form_longer_path.origin = GURL("http://accounts.google.com/a/LoginAuth/sec");
   // Check that /a/LoginAuth does not match /a/LoginAuth/more.
-  EXPECT_EQ(0,
-            manager.DoesManage(form_longer_path) &
-                PasswordFormManager::RESULT_MANDATORY_ATTRIBUTES_MATCH);
+  EXPECT_EQ(0, manager.DoesManage(form_longer_path) &
+                   PasswordFormManager::RESULT_HTML_ATTRIBUTES_MATCH);
 
   PasswordForm secure_observed_form(*observed_form());
   secure_observed_form.origin = GURL("https://accounts.google.com/a/LoginAuth");
@@ -1116,14 +1111,28 @@ TEST_F(PasswordFormManagerTest,
                                      secure_observed_form, true);
   // Also for HTTPS in the observed form, and HTTP in the compared form, an
   // exact path match is expected.
-  EXPECT_EQ(0,
-            secure_manager.DoesManage(form_longer_path) &
-                PasswordFormManager::RESULT_MANDATORY_ATTRIBUTES_MATCH);
+  EXPECT_EQ(0, secure_manager.DoesManage(form_longer_path) &
+                   PasswordFormManager::RESULT_HTML_ATTRIBUTES_MATCH);
   // Not even upgrade to HTTPS in the compared form should help.
   form_longer_path.origin = GURL("https://accounts.google.com/a/LoginAuth/sec");
-  EXPECT_EQ(0,
-            secure_manager.DoesManage(form_longer_path) &
-                PasswordFormManager::RESULT_MANDATORY_ATTRIBUTES_MATCH);
+  EXPECT_EQ(0, secure_manager.DoesManage(form_longer_path) &
+                   PasswordFormManager::RESULT_HTML_ATTRIBUTES_MATCH);
+}
+
+TEST_F(PasswordFormManagerTest, OriginCheck_OnlyOriginsMatch) {
+  // Make sure DoesManage() can distinguish when only origins match.
+  PasswordFormManager manager(NULL, client(), kNoDriver, *observed_form(),
+                              false);
+  PasswordForm different_html_attributes(*observed_form());
+  different_html_attributes.password_element = ASCIIToUTF16("random_pass");
+  different_html_attributes.username_element = ASCIIToUTF16("random_user");
+
+  EXPECT_EQ(0, manager.DoesManage(different_html_attributes) &
+                   PasswordFormManager::RESULT_HTML_ATTRIBUTES_MATCH);
+
+  EXPECT_EQ(PasswordFormManager::RESULT_ORIGINS_MATCH,

[vabr (Chromium), 2015/02/25 10:21:15]

optional: A shorter way to write this would be EXPECT_NE(0, ...), but I'm fine
with both.

[Garrett Casto, 2015/02/26 07:11:01]

Acknowledged.

+            manager.DoesManage(different_html_attributes) &
+                PasswordFormManager::RESULT_ORIGINS_MATCH);
 }
 
 TEST_F(PasswordFormManagerTest, CorrectlyUpdatePasswordsWithSameUsername) {