Fill on account select in the password manager

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/command_line.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/content/renderer/renderer_save_password_progress_logger.h"
+#include "components/autofill/core/common/autofill_switches.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/common/page_transition_types.h"
 #include "content/public/renderer/document_state.h"
 #include "content/public/renderer/navigation_state.h"
 #include "content/public/renderer/render_view.h"
 #include "third_party/WebKit/public/platform/WebVector.h"
 #include "third_party/WebKit/public/web/WebAutofillClient.h"
@@ skipping 268 matching lines @@
   if (!IsElementEditable(password))
     return false;
 
   blink::WebInputElement username = element;  // We need a non-const.
 
   // Do not set selection when ending an editing session, otherwise it can
   // mess with focus.
   FillUserNameAndPassword(&username,
                           &password,
                           fill_data,
-                          true /* exact_username_match */,
-                          false /* set_selection */);
+                          EXACT_USERNAME_MATCH);
   return true;
 }
 
 bool PasswordAutofillAgent::TextDidChangeInTextField(
     const blink::WebInputElement& element) {
   if (element.isPasswordField()) {
     // Some login forms have event handlers that put a hash of the password into
     // a hidden field and then clear the password (http://crbug.com/28910,
     // http://crbug.com/391693). This method gets called before any of those
     // handlers run, so save away a copy of the password in case it gets lost.
@@ skipping 594 matching lines @@
     return;
 
   if (!ShouldIgnoreAutocompleteOffForPasswordFields() &&
       !username_element.form().autoComplete())
     return;
 
   // If we can't modify the password, don't try to set the username
   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 (IsElementAutocompletable(username_element) &&
-      username_element.value().isEmpty()) {
-    // TODO(tkent): Check maxlength and pattern.
-    username_element.setValue(fill_data.basic_data.fields[0].value, true);
-  }
-
   // Fill if we have an exact match for the username. Note that this sets
   // username to autofilled.
   FillUserNameAndPassword(&username_element,
                           &password_element,
                           fill_data,
-                          true /* exact_username_match */,
-                          false /* set_selection */);
+                          EXACT_USERNAME_MATCH | FILL_PREFERRED_USERNAME);
 }
 
 bool PasswordAutofillAgent::FillUserNameAndPassword(
     blink::WebInputElement* username_element,
     blink::WebInputElement* password_element,
     const PasswordFormFillData& fill_data,
-    bool exact_username_match,
-    bool set_selection) {
+    const int options) {

[Garrett Casto, 2014/09/08 21:27:20]

So I think that the behavior if the username if already filled out is slightly
strange. If the username is one that we have data for, the user will see the
username with autofill highlighting but not password. If they click on the
username field the dropdown should appear and if they select that they should
get a password. If we don't have data for the particular username it's also less
likely that the user will understand what is going on, as it looks like the
username field was just autofilled, but at least the password will fill if they
try to change the username.

I would probably just avoid these cases for now. If the user sees a field that
is highlighted with Autofill but doesn't have any data, I'm not sure what they
are going to think, but if they see a field with data that is highlighted they
will probably just think that it was autofilled. If we decide to go down this
route and have a better indicator we should show it in all cases, but for now I
would just fill normally if the username if prefilled. I also know that this
comes up on accounts.google.com where the username field is not only prefilled
but hidden, so if we don't do this the password manager will just break.

This also makes the logic much simpler, just add an additional if statement in
FillFormOnPasswordRecieved()

if (fill_on_account_select) {
  username_element.setAutofilled(true);
  return;
} else {
  username_element.setValue(...);
}

[jww, 2014/09/09 01:40:34]

I was admittedly a bit confused by your comment, but I think I got the gist of
it and have addressed the issues, but you'll definitely want to double check my
latest CL :-)
On 2014/09/08 21:27:20, Garrett Casto wrote:

+  bool exact_username_match = (options & EXACT_USERNAME_MATCH) != 0;
+  bool set_selection = (options & SET_SELECTION) != 0;
+  bool fill_preferred_username = (options & FILL_PREFERRED_USERNAME) != 0;
+  bool fill_on_account_select = CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableFillOnAccountSelect);

[Garrett Casto, 2014/09/08 21:27:20]

You should pull this out into a function so that we can easily add a finch
experiment for this. Also you are currently ignoring the disable flag that you
added.

[jww, 2014/09/09 01:40:34]

Done.

   base::string16 current_username = username_element->value();
   // username and password will contain the match found if any.
   base::string16 username;
   base::string16 password;
 
+  // Try to set the username to the preferred name, but only if the field
+  // can be set and isn't prefilled. If FILL_PREFERRED_USERNAME
+  // is set, set |current_username| to the preferred value, but don't set the
+  // element's value. This is done so that even if values aren't actually
+  // filled, if there is a valid value to fill with, the elements will be
+  // properly marked as autofilled later.
+  if (fill_preferred_username &&
+      IsElementAutocompletable(*username_element) &&
+      username_element->value().isEmpty()) {
+    current_username = fill_data.basic_data.fields[0].value;
+    if (!fill_on_account_select) {
+      // TODO(tkent): Check maxlength and pattern.
+      username_element->setValue(current_username, true);
+    }
+  }
+
   // Look for any suitable matches to current field text.
   if (DoUsernamesMatch(fill_data.basic_data.fields[0].value,
                        current_username,
                        exact_username_match)) {
     username = fill_data.basic_data.fields[0].value;
     password = fill_data.basic_data.fields[1].value;
   } else {
     // Scan additional logins for a match.
     PasswordFormFillData::LoginCollection::const_iterator iter;
     for (iter = fill_data.additional_logins.begin();
@@ skipping 33 matching lines @@
   // TODO(tkent): Check maxlength and pattern for both username and password
   // fields.
 
   // Don't fill username if password can't be set.
   if (!IsElementAutocompletable(*password_element)) {
     return false;
   }
 
   // Input matches the username, fill in required values.
   if (IsElementAutocompletable(*username_element)) {
-    username_element->setValue(username, true);
     username_element->setAutofilled(true);
 
-    if (set_selection) {
-      username_element->setSelectionRange(current_username.length(),
-                                          username.length());
+    if (!fill_on_account_select) {
+      username_element->setValue(username, true);
+
+      if (set_selection) {
+        username_element->setSelectionRange(current_username.length(),
+                                            username.length());
+      }
     }
   } else if (current_username != username) {
     // If the username can't be filled and it doesn't match a saved password
     // as is, don't autofill a password.
     return false;
   }
 
   // Wait to fill in the password until a user gesture occurs. This is to make
   // sure that we do not fill in the DOM with a password until we believe the
   // user is intentionally interacting with the page.
-  password_element->setSuggestedValue(password);
+  if (!fill_on_account_select) {
+    password_element->setSuggestedValue(password);
+  }
   gatekeeper_.RegisterElement(password_element);
 
   password_element->setAutofilled(true);
   return true;
 }
 
 void PasswordAutofillAgent::PerformInlineAutocomplete(
     const blink::WebInputElement& username_input,
     const blink::WebInputElement& password_input,
     const PasswordFormFillData& fill_data) {
@@ skipping 12 matching lines @@
 
   // Show the popup with the list of available usernames.
   ShowSuggestionPopup(fill_data, username, false);
 
 #if !defined(OS_ANDROID)
   // Fill the user and password field with the most relevant match. Android
   // only fills in the fields after the user clicks on the suggestion popup.
   FillUserNameAndPassword(&username,
                           &password,
                           fill_data,
-                          false /* exact_username_match */,
-                          true /* set_selection */);
+                          SET_SELECTION);
 #endif
 }
 
 void PasswordAutofillAgent::FrameClosing(const blink::WebFrame* frame) {
   for (LoginToPasswordInfoMap::iterator iter = login_to_password_info_.begin();
        iter != login_to_password_info_.end();) {
     if (iter->first.document().frame() == frame)
       login_to_password_info_.erase(iter++);
     else
       ++iter;
@@ skipping 51 matching lines @@
   scoped_ptr<PasswordForm> password_form(CreatePasswordForm(form));
   if (!password_form || (restriction == RESTRICTION_NON_EMPTY_PASSWORD &&
                          password_form->password_value.empty() &&
                          password_form->new_password_value.empty())) {
     return;
   }
   provisionally_saved_forms_[frame].reset(password_form.release());
 }
 
 }  // namespace autofill