Autofill popup should not be presented when autocomplete='off', even if

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 236 matching lines @@
     // PasswordAutocompleteManager::TextDidChangeInTextField() for details.
     base::MessageLoop::current()->RunUntilIdle();
   }
 
   void SimulateUsernameChange(const std::string& username,
                               bool move_caret_to_end) {
     SimulateUsernameChangeForElement(username, move_caret_to_end,
                                      GetMainFrame(), username_element_);
   }
 
+  // Tests that no suggestion popup is generated when the username_element_ is
+  // edited.
+  void ExpectNoSuggestionsPopup() {
+    // This EXPECT_TRUE is interesting because of a quirk in how we handle the
+    // corner case of autocomplete='off' fields that also have a remembered
+    // password and username. Normally, if a unknown username is enterted, the

[Ilya Sherman, 2014/01/07 00:56:23]

nit: "a unknown" -> "an unknown"

[jww, 2014/01/07 20:00:25]

Done.

+    // autofill agent will return "false" from ShowSuggestions, but note that
+    // we're expecting "true" here.

[Ilya Sherman, 2014/01/07 00:56:23]

This sentence doesn't make sense anymore, since the tested username is known,
rather than unknown.  Please update this comment.  (Maybe it's worth testing
with an unknown username as well? Up to you.)

[jww, 2014/01/07 20:00:25]

Done.

+    //
+    // This is because of the specific case of an autocomplete='off' form that
+    // also has a remembered username and password (http://crbug.com/326679). To
+    // fix the DCHECK that this case used to hit, we return "true" from
+    // ShowSuggestions for all forms that are autocomplete='off', prentending
+    // that we've successfully shown a selection box to the user. Of course, we
+    // haven't so a message is never sent to the browser on acceptance, and the
+    // DCHECK isn't hit (and nothing is filled).
+    //
+    // However, because we do it for all usernames (known or unknown), we can
+    // differentiate this from before the patch, since it used to be that
+    // ShowSuggestions would return "false" on unknown usernames. However, we
+    // couldn't return "false" in the new case, because that would cause
+    // QueryAutofillSuggestions to be called, ultimately leading to the DCHECK
+    // being hit again.
+    //
+    // Thus, the regression test is to make sure that we are returing "true" for
+    // all calls to ShowSuggestions on autocomplete='off' forms, even for
+    // unknown usernames.
+    EXPECT_TRUE(autofill_agent_->password_autofill_agent_->ShowSuggestions(
+        username_element_));
+
+    // And as a final sanity check, we also make sure that no "show suggestions"

[Ilya Sherman, 2014/01/07 00:56:23]

nit: s/"we"//

[jww, 2014/01/07 20:00:25]

Done.

+    // IPC to the browser was generated.
+    EXPECT_TRUE(render_thread_->sink().GetFirstMessageMatching(
+                    AutofillHostMsg_ShowPasswordSuggestions::ID) == NULL);

[Ilya Sherman, 2014/01/07 00:56:23]

nit: I'd write this as either

EXPECT_FALSE(render_thread_->sink().GetFirstMessageMatching(
                AutofillHostMsg_ShowPasswordSuggestions::ID));

or as an EXPECT_EQ.  EXPECT_TRUE(a == b) is an unusual pattern.

[jww, 2014/01/07 20:00:25]

Done.

+  }
 
   void SimulateKeyDownEvent(const WebInputElement& element,
                             ui::KeyboardCode key_code) {
     blink::WebKeyboardEvent key_event;
     key_event.windowsKeyCode = key_code;
     autofill_agent_->textFieldDidReceiveKeyDown(element, key_event);
   }
 
   void CheckTextFieldsStateForElements(const WebInputElement& username_element,
                                        const std::string& username,
@@ skipping 532 matching lines @@
 
 // Verfies that a DOM-activated UI event will not cause an autofill.
 TEST_F(PasswordAutofillAgentTest, NoDOMActivationTest) {
   // Trigger the initial autocomplete.
   SimulateOnFillPasswordForm(fill_data_);
 
   ExecuteJavaScript(kJavaScriptClick);
   CheckTextFieldsDOMState(kAliceUsername, true, "", true);
 }
 
+// The following two tests are regression tests for http://crbug.com/326679

[Ilya Sherman, 2014/01/07 00:56:23]

nit: Please have one comment per test, in case tests move around or whatnot. 
It's fine if both comments are something very brief like "Regression test for
http://crbug.com/326679"

[jww, 2014/01/07 20:00:25]

Done.

+TEST_F(PasswordAutofillAgentTest, SelectUsernameWithUsernameAutofillOff) {
+  // Simulate the browser sending back the login info.
+  SimulateOnFillPasswordForm(fill_data_);
+
+  // Set the main password element to autocomplete='off'

[Ilya Sherman, 2014/01/07 00:56:23]

nit: Please update this comment.

[jww, 2014/01/07 20:00:25]

Done.

+  username_element_.setAttribute(WebString::fromUTF8("autocomplete"),
+                                 WebString::fromUTF8("off"));
+
+  // Simulate the user changing the username to some known username.
+  SimulateUsernameChange(kAliceUsername, true);
+
+  ExpectNoSuggestionsPopup();
+}
+
+TEST_F(PasswordAutofillAgentTest, SelectUsernameWithPasswordAutofillOff) {
+  // Simulate the browser sending back the login info.
+  SimulateOnFillPasswordForm(fill_data_);
+
+  // Set the main password element to autocomplete='off'
+  password_element_.setAttribute(WebString::fromUTF8("autocomplete"),
+                                 WebString::fromUTF8("off"));
+
+  // Simulate the user changing the username to some known username.
+  SimulateUsernameChange(kAliceUsername, true);
+
+  ExpectNoSuggestionsPopup();
+}
+
 }  // namespace autofill