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

components/autofill/content/renderer/password_autofill_agent.cc

Index: components/autofill/content/renderer/password_autofill_agent.cc
diff --git a/components/autofill/content/renderer/password_autofill_agent.cc b/components/autofill/content/renderer/password_autofill_agent.cc
index 2e747a3f7c4bfe46bd66f7da3b5b5863253b8820..eff747b50cb646d3d2abf1f2e412acdf7ac1697e 100644
--- a/components/autofill/content/renderer/password_autofill_agent.cc
+++ b/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,12 @@ bool DoUsernamesMatch(const base::string16& username1,
   return StartsWith(username1, username2, true);
 }
 
+bool IsElementAutocompletable(const blink::WebInputElement& element) {

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

nit: Docs, please.

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

Done.

+  return IsElementEditable(element) &&
+         (ShouldIgnoreAutocompleteOffForPasswordFields() ||
+          element.autoComplete());
+}
+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -250,9 +257,7 @@ 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)) {
     return false;
   }
 
@@ -325,6 +330,12 @@ bool PasswordAutofillAgent::ShowSuggestions(
   if (iter == login_to_password_info_.end())
     return false;
 
+  // If autocomplete='off' is set on the form, we don't want to actually show a
+  // suggestion dialog, so we return "true," prentending we've done so.

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

nit: "prentending" -> "pretending"

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

nit: Please avoid the pronoun "we" in comments (actually, try to avoid pronouns
as much as possible), because the referent can be unclear.  In this case, I'd
suggest rephrasing the comment to something like

""" 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). """

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

Done.

+  if (!IsElementAutocompletable(element) ||
+      !IsElementAutocompletable(iter->second.password_field))
+    return true;
+
   return ShowSuggestionPopup(iter->second.fill_data, element);
 }
 
@@ -645,16 +656,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 +729,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);