Autofill: Refactoring to support fetching password after a username is selected

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"
@@ skipping 333 matching lines @@
   LoginToPasswordInfoMap::iterator iter = login_to_password_info_.find(element);
   if (iter == login_to_password_info_.end())
     return false;
 
   int win_key_code = event.windowsKeyCode;
   iter->second.backspace_pressed_last =
       (win_key_code == ui::VKEY_BACK || win_key_code == ui::VKEY_DELETE);
   return true;
 }
 
-bool PasswordAutofillAgent::DidAcceptAutofillSuggestion(
+bool PasswordAutofillAgent::AcceptAutofillSuggestion(
     const blink::WebNode& node,
-    const blink::WebString& username) {
-  blink::WebInputElement input;
-  PasswordInfo password;
-  if (!FindLoginInfo(node, &input, &password))
+    const blink::WebString& username,
+    const blink::WebString& password) {
+  blink::WebInputElement username_element;
+  PasswordInfo password_info;
+  if (!FindLoginInfo(node, &username_element, &password_info))
     return false;
 
-  // Set the incoming |username| in the text field and |FillUserNameAndPassword|
-  // will do the rest.
-  input.setValue(username, true);
-  return FillUserNameAndPassword(&input, &password.password_field,
-                                 password.fill_data,
-                                 true /* exact_username_match */,
-                                 true /* set_selection */);
+  if (IsElementAutocompletable(username_element) &&
+      IsElementAutocompletable(password_info.password_field)) {

[Ilya Sherman, 2014/03/28 21:33:34]

nit: I would invert this condition and include an early return stmt, so that the
body of the method can be less indented.

[Patrick Dubroy, 2014/04/01 16:08:48]

Done.

+    base::string16 current_username = username_element.value();
+    username_element.setValue(username, true);
+    SetElementAutofilled(&username_element, true);
+    username_element.setSelectionRange(current_username.length(),
+                                       username.length());
+
+    password_info.password_field.setValue(password, true);
+    SetElementAutofilled(&password_info.password_field, true);
+  }
+
+  return true;
 }

[Ilya Sherman, 2014/03/28 21:33:34]

Could you please add test coverage for this method?  Ideally the tests should
have been able to catch the lack of IsElementAutocompletable() calls or the
forgotten setSelectionRange() call.  (I apologize that there wasn't already test
coverage in place for this.)

[Patrick Dubroy, 2014/04/01 16:08:48]

Done. In doing so, I did noticed some weird behavior of the old code though:

- The username is always filled, even if the username or password is not
autocompletable or the login doesn't match.
- In those situations, the username is not marked as autofilled.

I'm assuming those behaviors weren't intentional, so I've fixed them in my code.
Do you know of a reason that I shouldn't fix them?

[Garrett Casto, 2014/04/01 17:32:21]

I think that this was an oversight, but note that if this happens you are in a
pretty strange situation to begin with. The user has selected their credentials
from the UI, so not filling them is going to be confusing as well.  I'm not sure
if it matters much what we plan on doing at this point as this shouldn't happen
on well behaving sites unless we have a bug.

 
 bool PasswordAutofillAgent::DidClearAutofillSelection(
     const blink::WebNode& node) {
   blink::WebInputElement input;
   PasswordInfo password;
   return FindLoginInfo(node, &input, &password);
 }
 
 bool PasswordAutofillAgent::ShowSuggestions(
     const blink::WebInputElement& element) {
@@ skipping 525 matching lines @@
   LoginToPasswordInfoMap::iterator iter = login_to_password_info_.find(input);
   if (iter == login_to_password_info_.end())
     return false;
 
   *found_input = input;
   *found_password = iter->second;
   return true;
 }
 
 }  // namespace autofill