Fill on account select in the password manager behind a flag

components/autofill/content/renderer/autofill_agent.cc

Index: components/autofill/content/renderer/autofill_agent.cc
diff --git a/components/autofill/content/renderer/autofill_agent.cc b/components/autofill/content/renderer/autofill_agent.cc
index 8a9350e9304bc369045479377850335096a0421e..ade2d10f6bcea106c87f3e512d0e82b1a21a3973 100644
--- a/components/autofill/content/renderer/autofill_agent.cc
+++ b/components/autofill/content/renderer/autofill_agent.cc
@@ -242,7 +242,7 @@ void AutofillAgent::FocusedNodeChanged(const WebNode& node) {
   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.

@@ -610,7 +610,7 @@ void AutofillAgent::ShowSuggestions(const WebFormControlElement& element,
 
   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;