Add "previewing on hover" support for password field.

components/autofill/content/renderer/password_autofill_agent.cc

Index: components/autofill/content/renderer/password_autofill_agent.cc
diff --git a/components/autofill/content/renderer/password_autofill_agent.cc b/components/autofill/content/renderer/password_autofill_agent.cc
index 9e1e1db7025acd8685af9239015a95f397cd8bc2..fb8100fdd37b37ae4e033fd70eeb77fe8fe43c35 100644
--- a/components/autofill/content/renderer/password_autofill_agent.cc
+++ b/components/autofill/content/renderer/password_autofill_agent.cc
@@ -229,6 +229,8 @@ PasswordAutofillAgent::PasswordAutofillAgent(content::RenderView* render_view)
       usernames_usage_(NOTHING_TO_AUTOFILL),
       web_view_(render_view->GetWebView()),
       logging_state_active_(false),
+      was_password_autofilled_(false),
+      is_password_previewed_(false),
       weak_ptr_factory_(this) {
 }
 
@@ -380,7 +382,6 @@ bool PasswordAutofillAgent::AcceptSuggestion(
     return false;
   }
 
-  base::string16 current_username = username_element.value();
   username_element.setValue(username, true);
   username_element.setAutofilled(true);
   username_element.setSelectionRange(username.length(), username.length());

[Ilya Sherman, 2014/05/13 00:44:05]

If you rebase, you'll see that the line you removed was needed, and that this
line was updated.  Please make the same change below.

[ziran.sun, 2014/05/14 15:35:12]

I'm not able to see where this line is needed, even after rebasing code. Do
please let me know what I have missed. I've put this line back though.

[Ilya Sherman, 2014/05/14 23:10:58]

Oops, sorry, you're right -- I was thinking of a different case.  My bad!

@@ -391,11 +392,61 @@ bool PasswordAutofillAgent::AcceptSuggestion(
   return true;
 }
 
+bool PasswordAutofillAgent::SelectSuggestion(
+    const blink::WebNode& node,
+    const blink::WebString& username,
+    const blink::WebString& password) {
+  blink::WebInputElement username_element;
+  PasswordInfo password_info;
+
+  if (!FindLoginInfo(node, &username_element, &password_info) ||
+      !IsElementAutocompletable(username_element) ||
+      !IsElementAutocompletable(password_info.password_field)) {
+    return false;
+  }
+
+  username_element.setSuggestedValue(username);
+  username_element.setAutofilled(true);
+  // Password inline autocompletion auto-completes the username and password
+  // fields once user types part of the username string in. We compare the
+  // suggested value with the auto-completed value if there is any character
+  // by character here and set selection range starting from the end of matching
+  // characters.

[Ilya Sherman, 2014/05/13 00:44:05]

Hmm, does inline autocompletion actually call this (new) method?  If so, what's
the full codepath that triggers the call?

[ziran.sun, 2014/05/14 15:35:12]

I don't think inline autocompletion calls this. The use case is related to
inline autocompletion though: 

Say user has two usernames: ziran_su  & zi1234517. When type "zi" into username
field, the inline autocompletion auto-completes the field with "ziran_su". When
select "zi1234517" from the suggestion box, we will have "17" highlighted as the
selection range regardless that "zi12345" doesn't match "ziran_su". I feel that
in this case the selection range should be "1234517", starting from the end of
matching character between value and suggestedValue. 

[Ilya Sherman, 2014/05/14 23:10:58]

I agree with you there.  However, there's a simpler way to implement that,
because we only ever show suggestions that match the full value that's been
typed:

username_element.setSelectionRange(username_element.value().length(),
username_element.suggestedValue().length());

[ziran.sun, 2014/05/15 12:36:37]

I'm not sure this works for the use case I described above. When the inline
autocompletion is in place, the value of the field is not always the same as we
typed in, e.g. typing in "zi", the inline autocompletion auto-completes the
field with "ziran_su". However, the suggestion box still shows all the
suggestions that match "zi" rather than "ziran_su", e.g. "zi1234517" is listed
in the suggestion box as described above. If we take the length of the value of
the username field as the selection range starting point, we only have "17"
highlighted while hovering over "zi1234517", which might not be what we want.

Do let me know if I misses any points here though.

[Ilya Sherman, 2014/05/15 21:36:08]

Hmm, that's a fair point.  How about this, then?

username_element.setSelectionRange(username_element.selectionStart(),
                                   username_element.suggestedValue().length());

[ziran.sun, 2014/05/16 09:18:13]

This will have the beginning of the suggestedValue as selectionRange starting
point, which means the whole suggestedValue is selected. It sounds another
reasonable solution for me.

I'm not sure what's the requirement spec for this case - I had a search on w3c
drafts but couldn't find anything about this.

[Ilya Sherman, 2014/05/16 22:24:06]

So, after patching this in locally and testing, I realized that setting the
suggestedValue clears the selection, so what I really meant was:

int selection_start = username_element.selectionStart();
username_element.setSuggestedValue(username);
username_element.setSelectionRange(
    selection_start,
    username_element.suggestedValue().length());

Also, I realized from testing locally that we should save the original selection
and restore it in ClearPreview(), just as we do the autofill state.

In addition to making these changes to the code, please add test coverage for
them.  Thanks!

+  std::string value_string = username_element.value().utf8();
+  std::string suggested_string = username_element.suggestedValue().utf8();
+  size_t selection_start = 0;
+  bool done_compare = false;
+  while (done_compare == false) {
+    if (selection_start == value_string.length() ||
+        selection_start == suggested_string.length())
+      done_compare = true;
+
+    if (value_string[selection_start] == suggested_string[selection_start])
+      selection_start++;
+    else
+      done_compare = true;
+  }
+  username_element.setSelectionRange(
+      selection_start,
+      username_element.suggestedValue().length());
+
+  was_password_autofilled_ = password_info.password_field.isAutofilled();
+  password_info.password_field.setSuggestedValue(password);
+  password_info.password_field.setAutofilled(true);
+
+  is_password_previewed_ = true;
+
+  return true;
+}
+
 bool PasswordAutofillAgent::DidClearAutofillSelection(
     const blink::WebNode& node) {
   blink::WebInputElement input;

[Ilya Sherman, 2014/05/13 00:44:05]

nit: I'd rename this to |username|.

[ziran.sun, 2014/05/14 15:35:12]

Done.

   PasswordInfo password;
-  return FindLoginInfo(node, &input, &password);
+  if (is_password_previewed_ || !FindLoginInfo(node, &input, &password))
+    return false;
+
+  return true;

[Ilya Sherman, 2014/05/13 00:44:05]

This method should actually clear the form, not just return true, if there's
work to be done.

[ziran.sun, 2014/05/14 15:35:12]

Done.

 }
 
 bool PasswordAutofillAgent::ShowSuggestions(
@@ -416,6 +467,10 @@ bool PasswordAutofillAgent::ShowSuggestions(
   return ShowSuggestionPopup(iter->second.fill_data, element);
 }
 
+bool PasswordAutofillAgent::WasPasswordAutofilled() {
+  return was_password_autofilled_;
+}
+
 bool PasswordAutofillAgent::OriginCanAccessPasswordManager(
     const blink::WebSecurityOrigin& origin) {
   return origin.canAccessPasswordManager();