Add metrics for PasswordForm submission types

components/autofill/core/common/password_form.h

Index: components/autofill/core/common/password_form.h
diff --git a/components/autofill/core/common/password_form.h b/components/autofill/core/common/password_form.h
index 8bcf44850a8d6f537694bcedfcddb219b24931a7..e3cbe6b03fd3e5941bae51c6a8d3e13806333f74 100644
--- a/components/autofill/core/common/password_form.h
+++ b/components/autofill/core/common/password_form.h
@@ -84,6 +84,18 @@ struct PasswordForm {
     LAYOUT_LAST = LAYOUT_LOGIN_AND_SIGNUP
   };
 
+  // Events that Password Manager uses as triggers for checking a success of a
+  // password form submission.
+  enum class SubmissionEvent {

[engedy, 2017/05/09 22:02:59]

nit: To avoid using overloaded words in this context, have you considered naming
this something along the lines of:
 -- SubmissionDetectionTechnique, or
 -- SubmissionIndicatorEvent, or
 -- ProvisionalSaveTrigger, etc.

[dvadym, 2017/05/10 12:10:12]

Thanks, it makes sense. Renamed to SubmissionIndicatorEvent.

[engedy, 2017/05/10 13:07:42]

Acknowledged.

+    NONE,
+    HTML_FORM_SUBMISSION,
+    INPAGE_NAVIGATION,
+    XHR_SUCCEEDED,
+    FRAME_DETACHED,
+    BY_CONTEXT_MENU_CLICK,
+    SUBMISSION_EVENT_COUNT
+  };
+
   // The "Realm" for the sign-on. This is scheme, host, port for SCHEME_HTML.
   // Dialog based forms also contain the HTTP realm. Android based forms will
   // contain a string of the form "android://<hash of cert>@<package name>"
@@ -278,6 +290,9 @@ struct PasswordForm {
   // If true, this form looks like SignUp form according to local heuristics.
   bool does_look_like_signup_form;
 
+  // Type of event which was used for this form submission.
+  SubmissionEvent submission_event;

[engedy, 2017/05/09 22:02:59]

I wonder if Vasilii and Vaclav are going to be really sad once they are back and
see that PasswordForm grew even larger during their absence.

While I can certainly see the appeal of making this value part of PasswordForm
to facilitate plumbing it through all the layers, have you considered the
following?

 -- Getting this value through Mojo is quite painless: We would need to add this
as an argument to PasswordManagerDriver::InPageNavigation, but in all other
scenarios the PasswordManager can already tell the submission detection type
based on what method was called on it.

 -- The better question is how to store this on the browser side. We could store
this entirely in the PasswordManager (after all, the histogram is recorded
there), or add it as an additional field to PasswordFormManager. The latter
seems nicer, but there are quite a few call sites in unittests to
PFM::ProvisionallySave :-/.

[dvadym, 2017/05/10 12:10:12]

Don't worry, with the refactoring PasswordForm will not be on renderer anymore. 
And I think that this is the right place, since submission event type is a
concept of password form, why it should be in PasswordFormManager, which is
actually also overloaded with information :).

[engedy, 2017/05/10 13:07:42]

Fair enough, and glad to hear that this abomination is ultimately going away.
;-)

[dvadym, 2017/05/10 14:09:25]

Here many fields in PasswordForm is the smallest problem :). But of course it's
very visible.

+
   // Return true if we consider this form to be a change password form.
   // We use only client heuristics, so it could include signup forms.
   bool IsPossibleChangePasswordForm() const;
@@ -315,6 +330,8 @@ base::string16 OtherPossibleUsernamesToString(
 std::ostream& operator<<(std::ostream& os, PasswordForm::Layout layout);
 std::ostream& operator<<(std::ostream& os, const PasswordForm& form);
 std::ostream& operator<<(std::ostream& os, PasswordForm* form);
+std::ostream& operator<<(std::ostream& os,
+                         PasswordForm::SubmissionEvent submission_event);
 
 }  // namespace autofill