Password autofill should not override explicitly typed password

chrome/renderer/autofill/password_autofill_agent_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/test/base/chrome_render_view_test.h"
 #include "components/autofill/content/common/autofill_messages.h"
 #include "components/autofill/content/renderer/autofill_agent.h"
 #include "components/autofill/content/renderer/form_autofill_util.h"
@@ skipping 243 matching lines @@
     password_element_ = element.to<blink::WebInputElement>();
   }
 
   void ClearUsernameAndPasswordFields() {
     username_element_.setValue("");
     username_element_.setAutofilled(false);
     password_element_.setValue("");
     password_element_.setAutofilled(false);
   }
 
-  void SimulateUsernameChangeForElement(const std::string& username,
-                                        bool move_caret_to_end,
-                                        WebFrame* input_frame,
-                                        WebInputElement& username_input,
-                                        bool is_user_input) {
-    username_input.setValue(WebString::fromUTF8(username), is_user_input);
+  void SimulateDidEndEditing(WebFrame* input_frame, WebInputElement& input) {
+    autofill_agent_->textFieldDidEndEditing(input);
+  }
+
+  void SimulateInputChangeForElement(const std::string& new_value,
+                                     bool move_caret_to_end,
+                                     WebFrame* input_frame,
+                                     WebInputElement& input,
+                                     bool is_user_input) {
+    input.setValue(WebString::fromUTF8(new_value), is_user_input);
     // The field must have focus or AutofillAgent will think the
     // change should be ignored.
-    while (!username_input.focused())
+    while (!input.focused())
       input_frame->document().frame()->view()->advanceFocus(false);
     if (move_caret_to_end)
-      username_input.setSelectionRange(username.length(), username.length());
+      input.setSelectionRange(new_value.length(), new_value.length());
     if (is_user_input)
       password_autofill_agent()->FirstUserGestureObserved();
-    autofill_agent_->textFieldDidChange(username_input);
+    autofill_agent_->textFieldDidChange(input);
     // Processing is delayed because of a Blink bug:
     // https://bugs.webkit.org/show_bug.cgi?id=16976
     // See PasswordAutofillAgent::TextDidChangeInTextField() for details.
 
     // Autocomplete will trigger a style recalculation when we put up the next
     // frame, but we don't want to wait that long. Instead, trigger a style
     // recalcuation manually after TextFieldDidChangeImpl runs.
     base::MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
         &PasswordAutofillAgentTest::LayoutMainFrame, base::Unretained(this)));
 
@@ skipping 14 matching lines @@
     autofill_agent_->OnFillPasswordSuggestion(blink_username, blink_password);
   }
 
   void LayoutMainFrame() {
     GetMainFrame()->view()->layout();
   }
 
   void SimulateUsernameChange(const std::string& username,
                               bool move_caret_to_end,
                               bool is_user_input = false) {
-    SimulateUsernameChangeForElement(username,
-                                     move_caret_to_end,
-                                     GetMainFrame(),
-                                     username_element_,
-                                     is_user_input);
+    SimulateInputChangeForElement(username,
+                                  move_caret_to_end,
+                                  GetMainFrame(),
+                                  username_element_,
+                                  is_user_input);
   }
 
   // Tests that no suggestion popup is generated when the username_element_ is
   // edited.
   void ExpectNoSuggestionsPopup() {
     // The first test below ensures that the suggestions have been handled by
     // the password_autofill_agent, even though autocomplete='off' is set. The
     // second check ensures that, although handled, no "show suggestions" IPC to
     // the browser was generated.
     //
@@ skipping 33 matching lines @@
                                        const WebInputElement& password_element,
                                        const std::string& password,
                                        bool password_autofilled,
                                        bool checkSuggestedValue) {
     EXPECT_EQ(username,
               static_cast<std::string>(username_element.value().utf8()));
     EXPECT_EQ(username_autofilled, username_element.isAutofilled());
     EXPECT_EQ(password,
               static_cast<std::string>(
                   checkSuggestedValue ? password_element.suggestedValue().utf8()
-                                      : password_element.value().utf8()));
+                                      : password_element.value().utf8()))
+        << "checkSuggestedValue == " << checkSuggestedValue;
     EXPECT_EQ(password_autofilled, password_element.isAutofilled());
   }
 
   // Checks the DOM-accessible value of the username element and the
   // *suggested* value of the password element.
   void CheckTextFieldsState(const std::string& username,
                             bool username_autofilled,
                             const std::string& password,
                             bool password_autofilled) {
     CheckTextFieldsStateForElements(username_element_,
@@ skipping 531 matching lines @@
 
   WebInputElement username_input = username_element.to<WebInputElement>();
   WebInputElement password_input = password_element.to<WebInputElement>();
   ASSERT_FALSE(username_element.isNull());
 
   CheckTextFieldsStateForElements(
       username_input, "", false, password_input, "", false, false);
 
   // Simulate the user typing in the username in the iframe which should cause
   // an autofill.
-  SimulateUsernameChangeForElement(
+  SimulateInputChangeForElement(
       kAliceUsername, true, iframe, username_input, true);
 
   CheckTextFieldsStateForElements(username_input,
                                   kAliceUsername,
                                   true,
                                   password_input,
                                   kAlicePassword,
                                   true,
                                   false);
 }
@@ skipping 554 matching lines @@
   render_thread_->sink().ClearMessages();
   autofill_agent_->FormControlElementClicked(username_element_, true);
   ExpectAllCredentials();
 }
 
 // The user types in a password, but then just before sending the form off, a
 // script clears that password. This test checks that PasswordAutofillAgent can
 // still remember the password typed by the user.
 TEST_F(PasswordAutofillAgentTest,
        RememberLastNonEmptyPasswordOnSubmit_ScriptCleared) {
-  SimulateUsernameChangeForElement(
+  SimulateInputChangeForElement(
       "temp", true, GetMainFrame(), username_element_, true);
-  SimulateUsernameChangeForElement(
+  SimulateInputChangeForElement(
       "random", true, GetMainFrame(), password_element_, true);
 
   // Simulate that the password value was cleared by the site's JavaScript
   // before submit.
   password_element_.setValue(WebString());
   static_cast<content::RenderViewObserver*>(password_autofill_agent())
       ->WillSubmitForm(GetMainFrame(), username_element_.form());
 
   // Observe that the PasswordAutofillAgent still remembered the last non-empty
   // password and sent that to the browser.
   ExpectFormSubmittedWithPasswords("random", "");
 }
 
 // Similar to RememberLastNonEmptyPasswordOnSubmit_ScriptCleared, but this time
 // it's the user who clears the password. This test checks that in that case,
 // the last non-empty password is not remembered.
 TEST_F(PasswordAutofillAgentTest,
        RememberLastNonEmptyPasswordOnSubmit_UserCleared) {
-  SimulateUsernameChangeForElement(
+  SimulateInputChangeForElement(
       "temp", true, GetMainFrame(), username_element_, true);
-  SimulateUsernameChangeForElement(
+  SimulateInputChangeForElement(
       "random", true, GetMainFrame(), password_element_, true);
 
   // Simulate that the user actually cleared the password again.
-  SimulateUsernameChangeForElement(
+  SimulateInputChangeForElement(
       "", true, GetMainFrame(), password_element_, true);
   static_cast<content::RenderViewObserver*>(password_autofill_agent())
       ->WillSubmitForm(GetMainFrame(), username_element_.form());
 
   // Observe that the PasswordAutofillAgent respects the user having cleared the
   // password.
   ExpectFormSubmittedWithPasswords("", "");
 }
 
 // Similar to RememberLastNonEmptyPasswordOnSubmit_ScriptCleared, but uses the
 // new password instead of the current password.
 TEST_F(PasswordAutofillAgentTest,
        RememberLastNonEmptyPasswordOnSubmit_NewPassword) {
   const char kNewPasswordFormHTML[] =
       "<FORM name='LoginTestForm'>"
       "  <INPUT type='text' id='username' autocomplete='username'/>"
       "  <INPUT type='password' id='password' autocomplete='new-password'/>"
       "  <INPUT type='submit' value='Login'/>"
       "</FORM>";
   LoadHTML(kNewPasswordFormHTML);
   UpdateUsernameAndPasswordElements();
 
-  SimulateUsernameChangeForElement(
+  SimulateInputChangeForElement(
       "temp", true, GetMainFrame(), username_element_, true);
-  SimulateUsernameChangeForElement(
+  SimulateInputChangeForElement(
       "random", true, GetMainFrame(), password_element_, true);
 
   // Simulate that the password value was cleared by the site's JavaScript
   // before submit.
   password_element_.setValue(WebString());
   static_cast<content::RenderViewObserver*>(password_autofill_agent())
       ->WillSubmitForm(GetMainFrame(), username_element_.form());
 
   // Observe that the PasswordAutofillAgent still remembered the last non-empty
   // password and sent that to the browser.
   ExpectFormSubmittedWithPasswords("", "random");
 }
 
+// The user first accepts a suggestion, but then overwrites the password. This
+// test checks that the overwritten password is not reverted back by the user

[engedy, 2014/08/06 10:18:17]

nit: s/by/if/

[vabr (Chromium), 2014/08/25 14:53:29]

Done.

+// triggering autofill through focusing (but not changing) the username again.

[engedy, 2014/08/06 10:18:17]

nit: s/triggering/triggers/

[vabr (Chromium), 2014/08/25 14:53:29]

Done.

+TEST_F(PasswordAutofillAgentTest, PasswordNotOverwritten) {

[engedy, 2014/08/06 10:18:17]

nit: How about NoopEditingDoesNotOverwriteManuallyEditedPassword?

[vabr (Chromium), 2014/08/25 14:53:29]

Done.

+  // Simulate having credentials which needed to wait until the user starts
+  // typing the username to be filled (e.g., PSL-matched credentials). Those are
+  // the ones which can be filled as a result of TextFieldDidEndEditing.
+  fill_data_.wait_for_username = true;
+  SimulateOnFillPasswordForm(fill_data_);
+  // Simulate that the user typed her name to make the autofill work.
+  SimulateInputChangeForElement(kAliceUsername,
+                                /*move_caret_to_end=*/true,
+                                GetMainFrame(),
+                                username_element_,
+                                /*is_user_input=*/true);
+  SimulateDidEndEditing(GetMainFrame(), username_element_);
+  const std::string old_username(username_element_.value().utf8());
+  const std::string old_password(password_element_.value().utf8());
+  const std::string new_password(old_password + "modify");
+
+  // The user changes the password.
+  SimulateInputChangeForElement(new_password,
+                                /*move_caret_to_end=*/true,
+                                GetMainFrame(),
+                                password_element_,
+                                /*is_user_input=*/true);
+
+  // The user switches back into the username field, but leaves that without
+  // changes.
+  SimulateDidEndEditing(GetMainFrame(), username_element_);
+
+  // The password should have stayed as the user changed it.
+  CheckTextFieldsDOMState(old_username, true, new_password, false);
+  // The password should not have a suggested value.
+  CheckTextFieldsState(old_username, true, std::string(), false);
+}
+
+// Tests that inline autocompletion overwrites the previous password value when

[engedy, 2014/08/06 10:18:17]

nit: I would personally remove this comment, I do not think it adds to much
value.

[vabr (Chromium), 2014/08/25 14:53:29]

Done.

+// it should.
+TEST_F(PasswordAutofillAgentTest, InlineAutocompleteOverwritesValue) {

[engedy, 2014/08/06 10:18:17]

nit: How about InlineAutocompleteOverwritesManuallyEditedPassword?

[vabr (Chromium), 2014/08/25 14:53:29]

Done.

+  // Simulate the browser sending back the login info.
+  SimulateOnFillPasswordForm(fill_data_);
+
+  ClearUsernameAndPasswordFields();
+
+  // The user enters a password
+  SimulateInputChangeForElement("someOtherPassword",
+                                /*move_caret_to_end=*/true,
+                                GetMainFrame(),
+                                password_element_,
+                                /*is_user_input=*/true);
+
+  // Simulate the user typing a stored username.
+  SimulateUsernameChange(kAliceUsername, true);
+  // The autofileld password should replace the typed one.
+  CheckTextFieldsDOMState(kAliceUsername, true, kAlicePassword, true);
+}
+
 }  // namespace autofill