[Password Manager] Add UMA stats for submitted form type

components/password_manager/core/browser/password_form_manager.h

Index: components/password_manager/core/browser/password_form_manager.h
diff --git a/components/password_manager/core/browser/password_form_manager.h b/components/password_manager/core/browser/password_form_manager.h
index 7e3a5e9e4a3e5e1da4ac32d2153a46fc7e1e6a7a..b210e49fe3461cc1ee190f69a389de1ceac8d5c3 100644
--- a/components/password_manager/core/browser/password_form_manager.h
+++ b/components/password_manager/core/browser/password_form_manager.h
@@ -93,7 +93,12 @@ class PasswordFormManager : public PasswordStoreConsumer {
   // the same thread!
   bool HasCompletedMatching() const;
 
-  // When attempting to provisionally save |form|, call this to check if it is
+  // Update with the |form| that was actually submitted. Used to determine

[vabr (Chromium), 2015/03/31 10:08:17]

nit: Update what? The PasswordFormManager / |this| ?

[Garrett Casto, 2015/04/01 22:34:48]

Yeah, done.

+  // what type the submitted form is for IsIgnorableChangePasswordForm() and
+  // UMA stats.
+  void SetSubmittedForm(const autofill::PasswordForm& form);
+
+  // When attempting to provisionally save |form|, check this to check if it is

[vabr (Chromium), 2015/03/31 10:08:17]

optional nit: You are the native speaker here :), but does "check this to check"
really sounds better than "call this to check"?

[Garrett Casto, 2015/04/01 22:34:48]

My fault, leftover from a different change.

   // actually a change-password form which should be ignored, i.e., whether:
   // * the username was not explicitly marked with "autocomplete=username", and
   // * both the current and new password fields are non-empty, and
@@ -102,7 +107,9 @@ class PasswordFormManager : public PasswordStoreConsumer {
   // even be none), and the user should not be bothered with saving a
   // potentially malformed credential. Once we handle change password forms
   // correctly, this method should be replaced accordingly.
-  bool IsIgnorableChangePasswordForm(const autofill::PasswordForm& form) const;
+  //
+  // Must be called after SetSubmittedForm().
+  bool IsIgnorableChangePasswordForm();

[vabr (Chromium), 2015/03/31 10:08:17]

nit: Let's change it to a simple is_ignorable_change_password_form() accessor
for is_ignorable_change_password_form_?

[Garrett Casto, 2015/04/01 22:34:48]

Yeah, I went back and forth about this. While this actually is an accessor right
now, it wasn't clear that this needed to be communicated to callers. Since you
seem to prefer it, I'll switch.

 
   // Determines if the user opted to 'never remember' passwords for this form.
   bool IsBlacklisted() const;
@@ -235,6 +242,19 @@ class PasswordFormManager : public PasswordStoreConsumer {
     kSubmitResultMax
   };
 
+  // What the form is used for.
+  enum FormType {
+    kFormTypeLogin = 0,

[vabr (Chromium), 2015/03/31 10:08:17]

Currently you do not assign this value anywhere. I suggest against making this
the default value, and instead suggest assigning it in SetSubmittedForm
explicitly (when appropriate).

For the default value you might want to create another value, to carry the
meaning: "no submitted form seen yet".

[Garrett Casto, 2015/04/01 22:34:48]

Done.
Yeah, I sort of didn't want to do that because it added a value that we wouldn't
actually upload, but the clarity it gives probably outweighs any possible
confusion about it not being uploaded. I added a comment to clarify.

[vabr (Chromium), 2015/04/02 08:05:53]

Acknowledged.

+    kFormTypeLoginNoUsername,
+    kFormTypeChangePasswordEnabled,
+    kFormTypeChangePasswordDisabled,
+    kFormTypeChangePasswordNoUsername,
+    kFormTypeSignup,
+    kFormTypeSignupNoUsername,
+    kFormTypeLoginAndSignup,
+    kFormTypeMax,

[vabr (Chromium), 2015/03/31 10:08:17]

optional nit:
Not putting the "," after kFormTypeMax might help prevent the case when somebody
adds a value past this one by mistake.

[Garrett Casto, 2015/04/01 22:34:48]

Sure, done.

+  };
+
   // The maximum number of combinations of the three preceding enums.
   // This is used when recording the actions taken by the form in UMA.
   static const int kMaxNumActionsTaken =
@@ -341,6 +361,10 @@ class PasswordFormManager : public PasswordStoreConsumer {
   // multiple matches (when first saved, a login is marked preferred).
   const autofill::PasswordForm* preferred_match_;
 
+  // If the submitted form is for a change password form and the username
+  // doesn't match saved credentials.
+  bool is_ignorable_change_password_form_;
+
   typedef enum {
     PRE_MATCHING_PHASE,  // Have not yet invoked a GetLogins query to find
                          // matching login information from password store.
@@ -374,6 +398,8 @@ class PasswordFormManager : public PasswordStoreConsumer {
   UserAction user_action_;
   SubmitResult submit_result_;
 
+  FormType form_type_;
+
   DISALLOW_COPY_AND_ASSIGN(PasswordFormManager);
 };