Processing USERNAME reply from Autofill server in Password Manager

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..bf3efe7e6b22d7e559ed846189ed90bb32fec175 100644
--- a/components/password_manager/core/browser/password_manager_unittest.cc
+++ b/components/password_manager/core/browser/password_manager_unittest.cc
@@ -1037,4 +1037,47 @@ TEST_F(PasswordManagerTest, SavingSignupForms_NoActionMatch) {
   form_to_save->Save();
 }
 
+TEST_F(PasswordManagerTest, FormSubmittedChangedWithAutofillResponse) {
+  // Test that observing that a newly submitted form differs from provisionally

[vabr (Chromium), 2015/03/26 09:43:55]

I still struggle to understand, partly because the sentence spans 3 lines. This
seems to tell me that observing a difference in the forms should influence
username identification. But I doubt that is true, and it is not what is being
tested.

This verifies that if the provisionally saved and observed forms differ in the
choice of the username, the saving still succeeds, as long as the changed form
is marked "parsed using autofill predictions".

What about using the above explanation?

Also, why would the server-side predictions apply to the provisionally saved,
but not the observed form?

[dvadym, 2015/03/26 12:29:13]

Thanks, comment for test changed to yours.

+  // saved one due to autoflll predictions caused another field to be identified
+  // as username.
+  EXPECT_CALL(driver_, FillPasswordForm(_)).Times(Exactly(0));
+  std::vector<PasswordForm> observed;
+  PasswordForm form(MakeSimpleForm());
+  observed.push_back(form);
+  // The initial load.
+  manager()->OnPasswordFormsParsed(&driver_, observed);
+  // The initial layout.
+  manager()->OnPasswordFormsRendered(&driver_, observed, true);
+
+  // Simulate that based on autofill server username prediction, the username
+  // of the form changed from the default candidate("Email") to something else.
+  // Set the parsed_using_autofill_predictions bit to true to make sure that
+  // choice of username is accepted by PasswordManager, otherwise the form
+  // will rejected as not equal to provionally saved one.

[vabr (Chromium), 2015/03/26 09:43:55]

Aren't we telling the password manager, that |form| is _the provisionally saved
one_ on line 1061? How can it differ from itself? Did you mean "the observed
one"?

[vabr (Chromium), 2015/03/26 09:43:55]

will rejected -> will be rejected
provionally -> the provisionally

[dvadym, 2015/03/26 12:29:13]

Added comment 
  // Note that during initial parsing we don't have yet autofill server
predictions, that's why observed form and submitted form may be different.

[dvadym, 2015/03/26 12:29:13]

Done.

+  form.username_element = ASCIIToUTF16("Username");
+  form.parsed_using_autofill_predictions = true;
+  // And the form submit contract is to call ProvisionallySavePassword.
+  manager()->ProvisionallySavePassword(form);
+
+  scoped_ptr<PasswordFormManager> form_to_save;
+  EXPECT_CALL(client_,
+              PromptUserToSavePasswordPtr(
+                  _, CredentialSourceType::CREDENTIAL_SOURCE_PASSWORD_MANAGER))
+      .WillOnce(WithArg<0>(SaveToScopedPtr(&form_to_save)));
+
+  // Now the password manager waits for the navigation to complete.
+  observed.clear();
+  // The post-navigation load.
+  manager()->OnPasswordFormsParsed(&driver_, observed);
+  // The post-navigation layout.
+  manager()->OnPasswordFormsRendered(&driver_, observed, true);
+
+  ASSERT_TRUE(form_to_save);
+  EXPECT_CALL(*store_, AddLogin(FormMatches(form)));
+
+  // Simulate saving the form, as if the info bar was accepted.
+  form_to_save->Save();
+}
+
 }  // namespace password_manager