Save password without an associated username.

chrome/renderer/autofill/password_autofill_manager.cc

Index: chrome/renderer/autofill/password_autofill_manager.cc
===================================================================
--- chrome/renderer/autofill/password_autofill_manager.cc	(revision 127980)
+++ chrome/renderer/autofill/password_autofill_manager.cc	(working copy)
@@ -194,6 +194,38 @@
   return StartsWith(username1, username2, true);
 }
 
+// Get the username field from fill_data.
+// If it hasn't any username, calls NOTREACHED().
+const webkit::forms::FormField& GetUsernameField(
+  const webkit::forms::PasswordFormFillData& fill_data) {
+  if (fill_data.basic_data.fields.size() == 2) {
+    return fill_data.basic_data.fields[1];
+  }
+  NOTREACHED();
+  // This return expression is to make this function returns a value
+  // in all paths.
+  // If there is a path which doesn't return any values,
+  // most of compiler output a warning or error.
+  // (ex. Visual C++ compiler outputs C4715 warning.)
+  return fill_data.basic_data.fields[1];
+}

[Ilya Sherman, 2012/03/27 00:15:28]

nit: I would write this method simply as

DCHECK_GE(fill_data.basic_data.fields.size(), 2U);
return fill_data.basic_data.fields[1];

[Yumikiyo Osanai, 2012/03/27 22:59:28]

Thanks! I didn't know the DCHECK_** macro. It seems very useful. I'll use
DCHECK_EQ macro in this function. For now, if fill_data.basic_data.fields.size()
> 2, it's probably wrong data.

+
+// Get the password field from fill_data.
+const webkit::forms::FormField& GetPasswordField(
+  const webkit::forms::PasswordFormFillData& fill_data) {
+  if (fill_data.basic_data.fields.size() == 2 ||
+      fill_data.basic_data.fields.size() == 1) {
+    return fill_data.basic_data.fields[0];
+  }
+  NOTREACHED();
+  // This return expression is to make this function returns a value
+  // in all paths.
+  // If there is a path which doesn't return any values,
+  // most of compiler output a warning or error.
+  // (ex. Visual C++ compiler outputs C4715 warning.)
+  return fill_data.basic_data.fields[0];

[Ilya Sherman, 2012/03/27 00:15:28]

nit: I would write this method simply as

DCHECK(!fill_data.basic_data.fields.empty());
return fill_data.basic_data.fields[0];

[Yumikiyo Osanai, 2012/03/27 22:59:28]

Done.

+}
+
 }  // namespace
 
 namespace autofill {
@@ -423,26 +455,27 @@
     if (!form_data.wait_for_username)
       FillForm(form_elements.get(), form_data.basic_data);
 

[Ilya Sherman, 2012/03/27 00:15:28]

nit: There's no need to do anything else on this iteration if wait_for_username
is true.  I would add an if-stmt here along the lines of

if (!HasUsernameField(form_data))
  continue;

(where HasUsernameField() is a simple helper function that returns """
form_data.basic_data.fields.size() == 2"""), and reduce indentation for the rest
of the code (i.e. lines 465--478 in this file in this patch set).

[Yumikiyo Osanai, 2012/03/27 22:59:28]

I've defined the HasUsernameField() and changed to use the function.

But I haven't changed anything about below your comment.
I'm a bit concerned that it changes the behavior of FillUserNameAndPassword()
function. Because the original version of this function is same as my patch when
wait_for_username is true.
For now, I'm not confidence this behavior is false.

[Ilya Sherman, 2012/03/27 23:28:57]

Sorry, I think my comment was a little unclear.  What I meant was: There's no
need to do anything else on this iteration if wait_for_username is true *and
there is no username*.  So, exactly the change you made is what I meant, except
also moving the HasUsernameField() check to be immediately after the
|wait_for_username| check.

[Yumikiyo Osanai, 2012/03/28 00:11:00]

Thank you for writing the detail:)
I've moved the HasUsernameField() check immediately after the wait_for_username
check.

-    // Attach autocomplete listener to enable selecting alternate logins.
-    // First, get pointers to username element.
-    WebKit::WebInputElement username_element =
-        form_elements->input_elements[form_data.basic_data.fields[0].name];
-
     // Get pointer to password element. (We currently only support single
     // password forms).
     WebKit::WebInputElement password_element =
-        form_elements->input_elements[form_data.basic_data.fields[1].name];
+      form_elements->input_elements[GetPasswordField(form_data).name];
 
-    // We might have already filled this form if there are two <form> elements
-    // with identical markup.
-    if (login_to_password_info_.find(username_element) !=
-        login_to_password_info_.end())
-      continue;
-
-    PasswordInfo password_info;
-    password_info.fill_data = form_data;
-    password_info.password_field = password_element;
-    login_to_password_info_[username_element] = password_info;
+    // If there is a username, set the username and login_to_password_info_.
+    if (form_data.basic_data.fields.size() == 2) {
+      // Attach autocomplete listener to enable selecting alternate logins.
+      // First, get pointers to username element.
+      WebKit::WebInputElement username_element =
+        form_elements->input_elements[GetUsernameField(form_data).name];
+      // We might have already filled this form if there are two <form> elements
+      // with identical markup.
+      if (login_to_password_info_.find(username_element) !=
+          login_to_password_info_.end())
+        continue;
+      PasswordInfo password_info;
+      password_info.fill_data = form_data;
+      password_info.password_field = password_element;
+      login_to_password_info_[username_element] = password_info;
+    }
   }
 }
 
@@ -453,9 +486,11 @@
     const webkit::forms::PasswordFormFillData& fill_data,
     const string16& input,
     std::vector<string16>* suggestions) {
-  if (StartsWith(fill_data.basic_data.fields[0].value, input, false))
-    suggestions->push_back(fill_data.basic_data.fields[0].value);
-
+  // There are both a password and a username.
+  if (fill_data.basic_data.fields.size() == 2) {
+    if (StartsWith(GetUsernameField(fill_data).value, input, false))

[Ilya Sherman, 2012/03/27 00:15:28]

nit: Please use "&&" rather than nested if-stmts

[Yumikiyo Osanai, 2012/03/27 22:59:28]

Done.

+      suggestions->push_back(GetUsernameField(fill_data).value);
+  }
   webkit::forms::PasswordFormFillData::LoginCollection::const_iterator iter;
   for (iter = fill_data.additional_logins.begin();
        iter != fill_data.additional_logins.end(); ++iter) {
@@ -501,11 +536,18 @@
   string16 username;
   string16 password;
 
+  // If there isn't any username form, just exit.
+  // Because this function is used for the case
+  // that the username form and the possword form exist.
+  if (fill_data.basic_data.fields.size() == 1) {
+    return false;

[Ilya Sherman, 2012/03/27 00:15:28]

nit: No need for curly braces, since this if-stmt's body is just one line.

[Yumikiyo Osanai, 2012/03/27 22:59:28]

Done. And I've changed to use the HasUsernameField() to check there isn't any
username.

+  }
+
   // Look for any suitable matches to current field text.
-  if (DoUsernamesMatch(fill_data.basic_data.fields[0].value, current_username,
+  if (DoUsernamesMatch(GetUsernameField(fill_data).value, current_username,
                        exact_username_match)) {
-    username = fill_data.basic_data.fields[0].value;
-    password = fill_data.basic_data.fields[1].value;
+    username = GetUsernameField(fill_data).value;
+    password = GetPasswordField(fill_data).value;
   } else {
     // Scan additional logins for a match.
     webkit::forms::PasswordFormFillData::LoginCollection::const_iterator iter;