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

components/autofill/content/renderer/password_autofill_agent.cc

 // Copyright 2013 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 "components/autofill/content/renderer/password_autofill_agent.h"
 
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/content/common/autofill_messages.h"
 #include "components/autofill/content/renderer/form_autofill_util.h"
 #include "components/autofill/content/renderer/password_form_conversion_utils.h"
 #include "components/autofill/core/common/form_field_data.h"
 #include "components/autofill/core/common/password_autofill_util.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/autofill/core/common/password_form_fill_data.h"
 #include "content/public/renderer/render_view.h"
 #include "third_party/WebKit/public/platform/WebVector.h"
 #include "third_party/WebKit/public/web/WebAutofillClient.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebElement.h"
 #include "third_party/WebKit/public/web/WebFormElement.h"
 #include "third_party/WebKit/public/web/WebFrame.h"
 #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"
 #include "ui/events/keycodes/keyboard_codes.h"
 
 namespace autofill {
 namespace {
 
 // The size above which we stop triggering autocomplete.
 static const size_t kMaximumTextSizeForAutocomplete = 1000;
@@ skipping 274 matching lines @@
 
 bool PasswordAutofillAgent::DidClearAutofillSelection(
     const blink::WebNode& node) {
   blink::WebInputElement input;
   PasswordInfo password;
   return FindLoginInfo(node, &input, &password);
 }
 
 bool PasswordAutofillAgent::ShowSuggestions(
     const blink::WebInputElement& element) {
+  blink::WebPasswordFormData form_data(element.form());

[Garrett Casto, 2013/12/27 22:09:48]

You also need to verify that element.form() is valid. It doesn't look like this
is checked previously in the call chain and it's possible to have input elements
that aren't in a form.

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

I believe this is irrelevant given my changes per your comment below. Please
correct me if I'm wrong.

+  scoped_ptr<PasswordForm> password_form = CreatePasswordForm(element.form());
+  if (!ShouldIgnoreAutocompleteOffForPasswordFields() && password_form.get() &&
+      (!element.autoComplete() || !password_form->password_autocomplete_set ||
+       !form_data.passwordShouldAutocomplete))

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

Why do you need both "!password_form->password_autocomplete_set" and
"!form_data.passwordShouldAutocomplete"?  It seems like all that matters is the
latter one -- whether autocomplete is *currently* disabled for the actual HTML
element.  If autocomplete had previously been disabled, but then got enabled by
the time we get here, I think it should be fine to show the popup.  Is there an
interesting case that I'm overlooking?

[Garrett Casto, 2013/12/27 22:09:48]

passwordShouldAutocomplete implies password_autocomplete_set, so there is no
reason to check both.

It seems like the right thing to do is move this seciton after the iterator
check. Then you check if both the password field (iter->second.password_field)
and the username field are fillable in the same manner that
FillUserNameAndPassword does. You might want to factor out the code that checks
that into a different function, as it does seem to get a lot of use
(IsElementEditable && ShouldIgnore...).

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

Done.

+    return true;
+
   LoginToPasswordInfoMap::const_iterator iter =
       login_to_password_info_.find(element);
   if (iter == login_to_password_info_.end())
     return false;
 
   return ShowSuggestionPopup(iter->second.fill_data, element);
 }
 
 bool PasswordAutofillAgent::OriginCanAccessPasswordManager(
     const blink::WebSecurityOrigin& origin) {
@@ skipping 507 matching lines @@
 }
 
 PasswordAutofillAgent::AutofillWebUserGestureHandler::
     AutofillWebUserGestureHandler(PasswordAutofillAgent* agent)
     : agent_(agent) {}
 
 PasswordAutofillAgent::AutofillWebUserGestureHandler::
     ~AutofillWebUserGestureHandler() {}
 
 }  // namespace autofill