Add password manager autocomplete suggestion when a username element in clicked.

chrome/renderer/autofill/password_autofill_agent_browsertest.cc

Index: chrome/renderer/autofill/password_autofill_agent_browsertest.cc
diff --git a/chrome/renderer/autofill/password_autofill_agent_browsertest.cc b/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
index 1723480a6b8fa2195a2f660fd0d43664434a0af0..754cdef5fa6d30d70d491316990476e17638ed3d 100644
--- a/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
+++ b/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
@@ -254,14 +254,17 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {
   void SimulateUsernameChangeForElement(const std::string& username,
                                         bool move_caret_to_end,
                                         WebFrame* input_frame,
-                                        WebInputElement& username_input) {
-    username_input.setValue(WebString::fromUTF8(username));
+                                        WebInputElement& username_input,
+                                        bool is_user_input) {
+    username_input.setValue(WebString::fromUTF8(username), is_user_input);
     // The field must have focus or AutofillAgent will think the
     // change should be ignored.
     while (!username_input.focused())
       input_frame->document().frame()->view()->advanceFocus(false);
     if (move_caret_to_end)
       username_input.setSelectionRange(username.length(), username.length());
+    if (is_user_input)
+      autofill_agent_->password_autofill_agent_->gatekeeper_.OnUserGesture();
     autofill_agent_->textFieldDidChange(username_input);
     // Processing is delayed because of a Blink bug:
     // https://bugs.webkit.org/show_bug.cgi?id=16976
@@ -276,14 +279,29 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {
     base::MessageLoop::current()->RunUntilIdle();
   }
 
+  void SimulateSuggestionChoice(const std::string& username,
+                                WebInputElement& username_input) {
+    blink::WebString blink_username = blink::WebString::fromUTF8(username);
+
+    // This call is necessary to setup the autofill agent appropriate for the
+    // user selection; simulates the menu actually popping up.
+    autofill_agent_->InputElementClicked(username_input, false, true);
+
+    autofill_agent_->OnAcceptPasswordAutofillSuggestion(blink_username);
+  }
+
   void LayoutMainFrame() {
     GetMainFrame()->view()->layout();
   }
 
   void SimulateUsernameChange(const std::string& username,
-                              bool move_caret_to_end) {
-    SimulateUsernameChangeForElement(username, move_caret_to_end,
-                                     GetMainFrame(), username_element_);
+                              bool move_caret_to_end,
+                              bool is_user_input = false) {
+    SimulateUsernameChangeForElement(username,
+                                     move_caret_to_end,
+                                     GetMainFrame(),
+                                     username_element_,
+                                     is_user_input);
   }
 
   // Tests that no suggestion popup is generated when the username_element_ is
@@ -310,7 +328,7 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {
     // permanently ignored.
     if (!ShouldIgnoreAutocompleteOffForPasswordFields()) {
       EXPECT_TRUE(autofill_agent_->password_autofill_agent_->ShowSuggestions(
-          username_element_));
+          username_element_, false));
 
       EXPECT_FALSE(render_thread_->sink().GetFirstMessageMatching(
           AutofillHostMsg_ShowPasswordSuggestions::ID));
@@ -330,7 +348,7 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {
                                        const WebInputElement& password_element,
                                        const std::string& password,
                                        bool password_autofilled,
-                                       bool checkSuggestedValue = true) {
+                                       bool checkSuggestedValue) {
     EXPECT_EQ(username,
               static_cast<std::string>(username_element.value().utf8()));
     EXPECT_EQ(username_autofilled, username_element.isAutofilled());
@@ -347,9 +365,13 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {
                             bool username_autofilled,
                             const std::string& password,
                             bool password_autofilled) {
-    CheckTextFieldsStateForElements(username_element_, username,
-                                    username_autofilled, password_element_,
-                                    password, password_autofilled);
+    CheckTextFieldsStateForElements(username_element_,
+                                    username,
+                                    username_autofilled,
+                                    password_element_,
+                                    password,
+                                    password_autofilled,
+                                    true);
   }
 
   // Checks the DOM-accessible value of the username element and the
@@ -372,6 +394,37 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {
     EXPECT_EQ(end, username_element_.selectionEnd());
   }
 
+  void ExpectAllCredentials() {
+    std::set<base::string16> usernames;
+    usernames.insert(username1_);
+    usernames.insert(username2_);
+    usernames.insert(username3_);
+    usernames.insert(alternate_username3_);
+
+    ASSERT_EQ(1u, password_autofill_->messages().size());
+    ASSERT_EQ(AutofillHostMsg_ShowPasswordSuggestions::ID,
+              password_autofill_->messages()[0]->type());
+    AutofillHostMsg_ShowPasswordSuggestions* msg =
+        reinterpret_cast<AutofillHostMsg_ShowPasswordSuggestions*>(
+            password_autofill_->messages()[0]);
+    Tuple4<autofill::FormFieldData,
+           gfx::RectF,
+           std::vector<base::string16>,
+           std::vector<base::string16> > args;
+    AutofillHostMsg_ShowPasswordSuggestions::Read(msg, &args);
+    ASSERT_EQ(4u, args.c.size());
+    std::set<base::string16>::iterator it;
+
+    for (int i = 0; i < 4; i++) {
+      it = usernames.find(args.c[i]);
+      EXPECT_TRUE(it != usernames.end());
+      if (it != usernames.end())
+        usernames.erase(it);
+    }
+
+    EXPECT_TRUE(usernames.empty());
+  }
+
   base::string16 username1_;
   base::string16 username2_;
   base::string16 username3_;
@@ -645,10 +698,10 @@ TEST_F(PasswordAutofillAgentTest, InlineAutocomplete) {
   // Simulate the browser sending back the login info.
   SimulateOnFillPasswordForm(fill_data_);
 
-  // Clear the text fields to start fresh.
   ClearUsernameAndPasswordFields();
 
-  // Simulate the user typing in the first letter of 'alice', a stored username.
+  // Simulate the user typing in the first letter of 'alice', a stored
+  // username.
   SimulateUsernameChange("a", true);
   // Both the username and password text fields should reflect selection of the
   // stored login.
@@ -822,16 +875,21 @@ TEST_F(PasswordAutofillAgentTest, IframeNoFillTest) {
   WebInputElement password_input = password_element.to<WebInputElement>();
   ASSERT_FALSE(username_element.isNull());
 
-  CheckTextFieldsStateForElements(username_input, "", false,
-                                  password_input, "", false);
+  CheckTextFieldsStateForElements(
+      username_input, "", false, password_input, "", false, false);
 
-  // Simulate the user typing in the username in the iframe, which should cause
+  // Simulate the user typing in the username in the iframe which should cause
   // an autofill.
-  SimulateUsernameChangeForElement(kAliceUsername, true,
-                                   iframe, username_input);
-
-  CheckTextFieldsStateForElements(username_input, kAliceUsername, true,
-                                  password_input, kAlicePassword, true);
+  SimulateUsernameChangeForElement(
+      kAliceUsername, true, iframe, username_input, true);
+
+  CheckTextFieldsStateForElements(username_input,
+                                  kAliceUsername,
+                                  true,
+                                  password_input,
+                                  kAlicePassword,
+                                  true,
+                                  false);
 }
 
 // Tests that a password will only be filled as a suggested and will not be
@@ -993,9 +1051,10 @@ TEST_F(PasswordAutofillAgentTest,
   // set directly.
   SimulateElementClick(kUsernameName);
 
-  // Simulate the user entering her username.
-  username_element_.setValue(ASCIIToUTF16(kAliceUsername), true);
-  autofill_agent_->textFieldDidEndEditing(username_element_);
+  // Simulate the user entering her username and selecting the matching autofill
+  // from the dropdown.
+  SimulateUsernameChange(kAliceUsername, true, true);
+  SimulateSuggestionChoice(kAliceUsername, username_element_);
 
   // The username and password should now have been autocompleted.
   CheckTextFieldsDOMState(kAliceUsername, true, kAlicePassword, true);
@@ -1016,4 +1075,54 @@ TEST_F(PasswordAutofillAgentTest,
   EXPECT_EQ(1, password_onchange_called);
 }
 
+// Tests that one user click on a username field is sufficient to bring up a
+// credential suggestion popup, and the user can autocomplete the password by
+// selecting the credential from the popup.
+TEST_F(PasswordAutofillAgentTest, ClickAndSelect) {
+  // Simulate the browser sending back the login info.
+  SimulateOnFillPasswordForm(fill_data_);
+
+  // Clear the text fields to start fresh.
+  ClearUsernameAndPasswordFields();
+
+  // SimulateElementClick() is called so that a user gesture is actually made
+  // and the password can be filled. However, SimulateElementClick() does not
+  // actually lead to the AutofillAgent's InputElementClicked() method being
+  // called, so SimulateSuggestionChoice has to manually call
+  // InputElementClicked().
+  SimulateElementClick(kUsernameName);
+  SimulateSuggestionChoice(kAliceUsername, username_element_);
+
+  CheckTextFieldsDOMState(kAliceUsername, true, kAlicePassword, true);
+}
+
+TEST_F(PasswordAutofillAgentTest, AllCredentialsOnClick) {
+  // Simulate the browser sending back the login info.
+  SimulateOnFillPasswordForm(fill_data_);
+
+  // Clear the text fields to start fresh.
+  ClearUsernameAndPasswordFields();
+
+  // Call SimulateElementClick() to produce a user gesture on the page so
+  // autofill will actually fill.
+  SimulateElementClick(kUsernameName);
+
+  // Simulate a user clicking on the username element. This should produce a
+  // message with all the usernames.
+  password_autofill_->pause_messages();

[Garrett Casto, 2014/03/06 01:23:46]

I'd probably remove pause_messages() and resume_message() and just check the
last sent message. For renderer browser tests, there is no one listening on the
other side of the IPC anyway, so it doesn't actually matter if they are sent or
not. I'm not sure if it's worth adding them to the API of
TestPasswordAutofillAgent just to verify that only one IPC was sent (since
really we don't care that only one was send).

[jww, 2014/03/07 02:06:06]

Yeah, I was overthinking it. I've modified the test to just use the
render_thread_->sink() message methods.

+  autofill_agent_->InputElementClicked(username_element_, false, true);
+  ExpectAllCredentials();
+  password_autofill_->resume_messages();
+
+  // Now simulate a user typing in the first letter of the username and then
+  // clicking on the username element. While the typing of the first letter will
+  // inline autocomplete, clicking on the element should still produce a message
+  // with all the usernames.

[Garrett Casto, 2014/03/06 01:23:46]

I think that we have a misunderstanding somewhere, because this is the opposite
of what I was arguing for earlier :). I think that you shouldn't show all
usernames if you click while in the middle of typing. Stomping on inline
autocomplete seems like a bad user experience.

[jww, 2014/03/07 02:06:06]

No misunderstanding; just a screw-up on my part.

+  SimulateUsernameChange("a", true);
+  password_autofill_->pause_messages();

[Garrett Casto, 2014/03/06 01:23:46]

I'd probably remove pause_messages() and resume_message() and just check the
last sent message. For renderer browser tests, there is no one listening on the
other side of the IPC anyway, so it doesn't actually matter if they are sent or
not. I'm not sure if it's worth adding them to the API of
TestPasswordAutofillAgent just to verify that only one IPC was sent, since we
don't care about the total number of IPCs anyway.

[jww, 2014/03/07 02:06:06]

Done.

+  autofill_agent_->InputElementClicked(username_element_, false, true);
+  ExpectAllCredentials();
+  password_autofill_->resume_messages();
+}
+
 }  // namespace autofill