Fill on account select in the password manager behind a flag

components/autofill/content/renderer/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/autofill_agent.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string_split.h"
@@ skipping 224 matching lines @@
   }
 
   WebElement web_element = node.toConst<WebElement>();
 
   if (!web_element.document().frame())
     return;
 
   const WebInputElement* element = toWebInputElement(&web_element);
 
   if (!element || !element->isEnabled() || element->isReadOnly() ||
-      !element->isTextField() || element->isPasswordField())
+      !element->isTextField())
     return;
 
   element_ = *element;

[Ilya Sherman, 2014/12/11 23:28:55]

I honestly don't even remember why it's important to set the element here, given
that that's the last thing that happens in this method.  Could you please remind
me?

[jww, 2014/12/12 00:17:43]

I think it's as simple as the focused node has changed, and element_ tracks the
currently focused node. For example, AutofillAgent::AcceptDataListSuggestion
takes the currently focused element (element_) as the input element and fills it
with the suggested value, and thus after focus has changed, element_ needs to be
updated.

[Ilya Sherman, 2014/12/12 00:25:50]

AcceptDataListSuggestion should only be called after ShowSuggestions, which also
sets the element_.  So, I guess my question is: Are there any code paths that
rely on the element_ being set here, rather than in a different method?  My more
focused question is: Does your change really need the element_ to be set here?

[jww, 2014/12/12 00:54:55]

My change doesn't touch the assignment of element_, so no, it doesn't need it I
suppose, but I'd rather leave the removal of the assignment to a separate,
unrelated CL, if that's cool with you.

[Ilya Sherman, 2014/12/12 01:56:02]

I think you can still address my more narrow question, by reverting your change
to this method if it's not needed.

[jww, 2014/12/12 02:05:49]

Ah, got it. I misunderstood your request. Done.

 }
 
 void AutofillAgent::OrientationChangeEvent() {
   HidePopup();
 }
 
 void AutofillAgent::Resized() {
   HidePopup();
 }
 
@@ skipping 344 matching lines @@
 
 void AutofillAgent::ShowSuggestions(const WebFormControlElement& element,
                                     const ShowSuggestionsOptions& options) {
   if (!element.isEnabled() || element.isReadOnly())
     return;
   if (!options.datalist_only && !element.suggestedValue().isEmpty())
     return;
 
   const WebInputElement* input_element = toWebInputElement(&element);
   if (input_element) {
-    if (!input_element->isTextField() || input_element->isPasswordField())
+    if (!input_element->isTextField())

[Ilya Sherman, 2014/12/11 23:28:55]

I am concerned that without the isPasswordField() check, we might try to offer
regular, non-password Autofill suggestions for password fields.  Could you
please either restore the guard, but possibly a little later on, so that it only
affects non-password Autofill; or else explain to me why this concern doesn't
apply?

[jww, 2014/12/12 00:17:43]

Good point. I moved the guard further down, right after the potential call to
password_autofill_agent_->ShowSuggestions(), since password fields should *only*
go to the password autofill agent.

       return;
     if (!options.datalist_only && !input_element->suggestedValue().isEmpty())
       return;
   } else {
     DCHECK(IsTextAreaElement(element));
     if (!element.toConst<WebTextAreaElement>().suggestedValue().isEmpty())
       return;
   }
 
   // Don't attempt to autofill with values that are too large or if filling
@@ skipping 203 matching lines @@
 void AutofillAgent::LegacyAutofillAgent::Resized() {
   agent_->Resized();
 }
 
 void AutofillAgent::LegacyAutofillAgent::FrameWillClose(
     blink::WebFrame* frame) {
   agent_->LegacyFrameWillClose(frame);
 }
 
 }  // namespace autofill