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

there already is information in the autofill DB.

BUG=326679

Review URL: https://codereview.chromium.org/120343003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@243841 0039d316-1c4b-4281-b951-d872f2087c98

chrome/renderer/autofill/password_autofill_agent_browsertest.cc

@@ -254,6 +254,28 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {
                                      GetMainFrame(), username_element_);
   }
 
+  // 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.
+    //
+    // This is interesting in 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,
+    // |true| is returned from ShowSuggestions for all forms with valid
+    // usersnames that are autocomplete='off', prentending that a selection box
+    // has been shown to the user. Of course, it hasn't, so a message is never
+    // sent to the browser on acceptance, and the DCHECK isn't hit (and nothing
+    // is filled).
+    EXPECT_TRUE(autofill_agent_->password_autofill_agent_->ShowSuggestions(
+        username_element_));
+
+    EXPECT_FALSE(render_thread_->sink().GetFirstMessageMatching(
+        AutofillHostMsg_ShowPasswordSuggestions::ID));
+  }
 
   void SimulateKeyDownEvent(const WebInputElement& element,
                             ui::KeyboardCode key_code) {
@@ -806,4 +828,66 @@ TEST_F(PasswordAutofillAgentTest, NoDOMActivationTest) {
   CheckTextFieldsDOMState(kAliceUsername, true, "", true);
 }
 
+// Regression test for http://crbug.com/326679
+TEST_F(PasswordAutofillAgentTest, SelectUsernameWithUsernameAutofillOff) {
+  // Simulate the browser sending back the login info.
+  SimulateOnFillPasswordForm(fill_data_);
+
+  // Set the username element to autocomplete='off'
+  username_element_.setAttribute(WebString::fromUTF8("autocomplete"),
+                                 WebString::fromUTF8("off"));
+
+  // Simulate the user changing the username to some known username.
+  SimulateUsernameChange(kAliceUsername, true);
+
+  ExpectNoSuggestionsPopup();
+}
+
+// Regression test for http://crbug.com/326679
+TEST_F(PasswordAutofillAgentTest,
+       SelectUnknownUsernameWithUsernameAutofillOff) {
+  // Simulate the browser sending back the login info.
+  SimulateOnFillPasswordForm(fill_data_);
+
+  // Set the username element to autocomplete='off'
+  username_element_.setAttribute(WebString::fromUTF8("autocomplete"),
+                                 WebString::fromUTF8("off"));
+
+  // Simulate the user changing the username to some unknown username.
+  SimulateUsernameChange("foo", true);
+
+  ExpectNoSuggestionsPopup();
+}
+
+// Regression test for http://crbug.com/326679
+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();
+}
+
+// Regression test for http://crbug.com/326679
+TEST_F(PasswordAutofillAgentTest,
+       SelectUnknownUsernameWithPasswordAutofillOff) {
+  // 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 unknown username.
+  SimulateUsernameChange("foo", true);
+
+  ExpectNoSuggestionsPopup();
+}
+
 }  // namespace autofill

components/autofill/content/renderer/password_autofill_agent.cc

@@ -26,6 +26,7 @@
 #include "third_party/WebKit/public/web/WebInputEvent.h"
 #include "third_party/WebKit/public/web/WebNode.h"
 #include "third_party/WebKit/public/web/WebNodeList.h"
+#include "third_party/WebKit/public/web/WebPasswordFormData.h"
 #include "third_party/WebKit/public/web/WebSecurityOrigin.h"
 #include "third_party/WebKit/public/web/WebUserGestureIndicator.h"
 #include "third_party/WebKit/public/web/WebView.h"
@@ -181,6 +182,16 @@ bool DoUsernamesMatch(const base::string16& username1,
   return StartsWith(username1, username2, true);
 }
 
+// Returns |true| if the given element is both editable and has permission to be
+// autocompleted. The latter can be either because there is no
+// autocomplete='off' set for the element, or because the flag is set to ignore
+// autocomplete='off'. Otherwise, returns |false|.
+bool IsElementAutocompletable(const blink::WebInputElement& element) {
+  return IsElementEditable(element) &&
+         (ShouldIgnoreAutocompleteOffForPasswordFields() ||
+          element.autoComplete());
+}
+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -250,9 +261,8 @@ bool PasswordAutofillAgent::TextDidChangeInTextField(
   if (iter->second.fill_data.wait_for_username)
     return false;
 
-  if (!IsElementEditable(element) || !element.isText() ||
-      (!ShouldIgnoreAutocompleteOffForPasswordFields() &&
-       !element.autoComplete())) {
+  if (!element.isText() || !IsElementAutocompletable(element) ||
+      !IsElementAutocompletable(password)) {
     return false;
   }
 
@@ -325,6 +335,14 @@ bool PasswordAutofillAgent::ShowSuggestions(
   if (iter == login_to_password_info_.end())
     return false;
 
+  // If autocomplete='off' is set on the form elements, no suggestion dialog
+  // should be shown. However, return |true| to indicate that this is a known
+  // password form and that the request to show suggestions has been handled (as
+  // a no-op).
+  if (!IsElementAutocompletable(element) ||
+      !IsElementAutocompletable(iter->second.password_field))
+    return true;
+
   return ShowSuggestionPopup(iter->second.fill_data, element);
 }
 
@@ -645,16 +663,12 @@ void PasswordAutofillAgent::FillFormOnPasswordRecieved(
     return;
 
   // If we can't modify the password, don't try to set the username
-  if (!IsElementEditable(password_element) ||
-      (!ShouldIgnoreAutocompleteOffForPasswordFields() &&
-       !password_element.autoComplete()))
+  if (!IsElementAutocompletable(password_element))
     return;
 
   // Try to set the username to the preferred name, but only if the field
   // can be set and isn't prefilled.
-  if (IsElementEditable(username_element) &&
-      (ShouldIgnoreAutocompleteOffForPasswordFields() ||
-       username_element.autoComplete()) &&
+  if (IsElementAutocompletable(username_element) &&
       username_element.value().isEmpty()) {
     // TODO(tkent): Check maxlength and pattern.
     username_element.setValue(fill_data.basic_data.fields[0].value);
@@ -722,16 +736,12 @@ bool PasswordAutofillAgent::FillUserNameAndPassword(
   // fields.
 
   // Don't fill username if password can't be set.
-  if (!IsElementEditable(*password_element) ||
-      (!ShouldIgnoreAutocompleteOffForPasswordFields() &&
-       !password_element->autoComplete())) {
+  if (!IsElementAutocompletable(*password_element)) {
     return false;
   }
 
   // Input matches the username, fill in required values.
-  if (IsElementEditable(*username_element) &&
-      (ShouldIgnoreAutocompleteOffForPasswordFields() ||
-       username_element->autoComplete())) {
+  if (IsElementAutocompletable(*username_element)) {
     username_element->setValue(username);
     SetElementAutofilled(username_element, true);