Add a text/password field count metric for empty username forms.

components/autofill/content/renderer/password_form_conversion_utils.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_form_conversion_utils.h"
 
 #include <vector>
 
 #include "base/lazy_instance.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/metrics/histogram.h"
 #include "base/strings/string_util.h"
 #include "components/autofill/content/renderer/form_autofill_util.h"
 #include "components/autofill/core/common/form_data_predictions.h"
 #include "components/autofill/core/common/password_form.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebFormControlElement.h"
 #include "third_party/WebKit/public/web/WebInputElement.h"
 #include "third_party/icu/source/i18n/unicode/regex.h"
 
@@ skipping 211 matching lines @@
     PasswordForm* password_form,
     const std::map<const blink::WebInputElement, blink::WebString>*
         nonscript_modified_values,
     const std::map<autofill::FormData, autofill::FormFieldData>*
         form_predictions) {
   WebInputElement latest_input_element;
   WebInputElement username_element;
   password_form->username_marked_by_site = false;
   std::vector<WebInputElement> passwords;
   std::vector<base::string16> other_possible_usernames;
+  int num_text_and_password_fields = 0;
 
   WebVector<WebFormControlElement> control_elements;
   form.getFormControlElements(control_elements);
 
   std::string layout_sequence;
   layout_sequence.reserve(control_elements.size());
   for (size_t i = 0; i < control_elements.size(); ++i) {
     WebFormControlElement control_element = control_elements[i];
     if (control_element.isActivatedSubmit())
       password_form->submit_element = control_element.formControlName();
 
     WebInputElement* input_element = toWebInputElement(&control_element);
     if (!input_element || !input_element->isEnabled())
       continue;
 
     if (input_element->isTextField()) {
       if (input_element->isPasswordField())
         layout_sequence.push_back('P');
       else
         layout_sequence.push_back('N');
+
+      // Count the total number of text and password fields. Note that

[vabr (Chromium), 2015/05/12 10:26:02]

nit: The comment is clear and correct, but probably not needed. The name of the
variable is good enough to convey the needed meaning.

[msramek, 2015/05/12 10:50:44]

Acknowledged.

+      // in renderer, |PasswordInputType| is a subclass of |TextFieldInputType|,
+      // but for consistency with the rest of the code we use the term "text"
+      // to refer specifically to a non-password text field.
+      ++num_text_and_password_fields;
     }
 
     // If the password field is readonly, the page is likely using a virtual
     // keyboard and bypassing the password field value (see
     // http://crbug.com/475488). There is nothing Chrome can do to fill
     // passwords for now.
     if (input_element->isPasswordField() &&
         (!input_element->isReadOnly() ||
          HasAutocompleteAttributeValue(*input_element, "current_password") ||
          HasAutocompleteAttributeValue(*input_element, "new-password"))) {
@@ skipping 89 matching lines @@
         base::string16 typed_username_value = username_iterator->second;
         if (!StartsWith(username_value, typed_username_value, false)) {
           // We check that |username_value| was not obtained by autofilling
           // |typed_username_value|. In case when it was, |typed_username_value|
           // is incomplete, so we should leave autofilled value.
           username_value = typed_username_value;
         }
       }
     }
     password_form->username_value = username_value;
+  } else {

[vabr (Chromium), 2015/05/12 10:26:02]

Do you want to report this even if there are not passwords found?

[msramek, 2015/05/12 10:50:44]

Do you mean no password fields found? No. But if I understand correctly, this
method is only called for forms that we already know to contain a password
field, right?

(at least my quick testing confirms that; I didn't check for all call chains)

If you mean for forms where we have no password saved, then yes. Currently, if
you encounter a form with no username and successfully submit it, we will always
offer to save. Therefore, any such form could have resulted in saved credentials
with no username, and thus I want to get a statistic on all the forms in the
wild.

[vabr (Chromium), 2015/05/12 11:06:11]

Yes, sorry for the confusion, I meant password fields, not values.
I don't think so, at least I could not find any evidence for it in the code.

If you look just below the block you added, there is a specific function to
locate the password fields, which also handles the case when |passwords| is
empty. It might be a good idea to check for that here as well.

[msramek, 2015/05/12 11:27:03]

Interesting. I would expect none of this code to trigger on such fields, as
confirmed by my quick test. Nevertheless, you're right that 
LocateSpecificPasswords() checks this case.

I moved this block to the end of the method. LocateSpecificPasswords() only
returns true if either |password| or |new_password| has been found, so no
additional conditions are needed.

[vabr (Chromium), 2015/05/12 11:33:58]

This has the caveat that you won't get this count for password forms with 3+
password fields, where the first 3 are non-empty and equal, or all different,
etc. (see when LocateSpecificPasswords returns false).

[msramek, 2015/05/12 12:13:47]

I know. But in this case the password manager will also bail, not offering to
save the form. So for the investigation of saved empty usernames, that is ok.

+    // To get a better idea on how forms without a username field look like,
+    // report the total number of text and password fields.
+    UMA_HISTOGRAM_COUNTS_100(
+        "PasswordManager.EmptyUsernames.TextAndPasswordFieldCount",
+        num_text_and_password_fields);

[vabr (Chromium), 2015/05/12 10:26:02]

Alternatively, you can use layout_sequence.size().
But if you prefer the dedicated counter for readability, that's fine.

[msramek, 2015/05/12 10:50:44]

No, let's use layout_sequence.size(). I just really didn't notice. The advantage
here is that I'll avoid the confusion with semantics of "text field".

   }
 
   WebInputElement password;
   WebInputElement new_password;
   if (!LocateSpecificPasswords(passwords, &password, &new_password))
     return;
 
   password_form->action = GetCanonicalActionForForm(form);
   if (!password_form->action.is_valid())
     return;
@@ skipping 75 matching lines @@
   WebFormElementToFormData(web_form,
                            blink::WebFormControlElement(),
                            EXTRACT_NONE,
                            &password_form->form_data,
                            NULL /* FormFieldData */);
 
   return password_form.Pass();
 }
 
 }  // namespace autofill