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_);
   }
 
+  bool SimulateShowSuggestions() {
+    return autofill_agent_->password_autofill_agent_->ShowSuggestions(
+        username_element_);
+  }
 
   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);
 }
 
+// Regression test for http://crbug.com/326679
+TEST_F(PasswordAutofillAgentTest, SelectUsernameOnAutofillOff) {

[Ilya Sherman, 2013/12/27 21:43:22]

nit: "On" -> "When" or "With" or something like that, so that it doesn't look
like it's contrasting with "Off"

[jww, 2013/12/27 23:04:39]

Done.

+  // Set the main password element to autocomplete='off'
+  password_element_.setAttribute(WebString::fromUTF8("autocomplete"),
+                                 WebString::fromUTF8("off"));

[Ilya Sherman, 2013/12/27 21:43:22]

It looks like the code refuses to show suggestions if autocomplete is off for
*either* the password field or the username field.  Right now, though, you're
only testing the case where the password field disables autocomplete.  Please
add test coverage for the username case as well.

[jww, 2013/12/27 23:04:39]

Done.

+
+  // Simulate the browser sending back the login info, it triggers the
+  // autocomplete.
+  SimulateOnFillPasswordForm(fill_data_);
+
+  // Simulate the user changing the username to some unknown username.
+  SimulateUsernameChange("foobar", true);
+
+  // 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
+  // autofill agent will return "false" from ShowSuggestions, but note that
+  // we're expecting "true" here.
+  //
+  // 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(SimulateShowSuggestions());

[Ilya Sherman, 2013/12/27 21:43:22]

Please also test that no AutofillHostMsg_ShowPasswordSuggestions message is
generated for a known username (and I guess make sure that it had been being
generated prior to this CL).  That seems like a more direct test of the property
you're trying to verify.

[jww, 2013/12/27 23:04:39]

That's a great idea. I didn't attempt it because I thought there was no way to
access the browser process from the browser test to check if an IPC handler
fired (or, even harder, if it didn't fire).

Your suggestion, though, of checking for the creation (or lack thereof) of the
message makes a lot of sense, though. Do you have any pointers on how to do this
(or perhaps an example browser test that does this)?
On 2013/12/27 21:43:22, Ilya Sherman (Away thru Jan 6) wrote:

[Ilya Sherman, 2013/12/27 23:17:09]

I think something like this ought to work:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/au...

If that doesn't quite help enough on its own, you might try a search for
something like "file:autofill file:renderer ipc sink" on
https://code.google.com/p/chromium/codesearch

On 2013/12/27 23:04:39, jww wrote:

[jww, 2013/12/31 00:33:05]

Done.

+}
+
 }  // namespace autofill