[Password Generation] Fixes sending votes about the usage of the password generation popup

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 f46d40128b41fc9d60a45ede1497b166a87a315e..66c3946731c5ffeff7f0318852123aae7620a2b8 100644
--- a/components/password_manager/core/browser/password_form_manager.h
+++ b/components/password_manager/core/browser/password_form_manager.h
@@ -26,6 +26,8 @@
 #include "components/password_manager/core/browser/password_manager_driver.h"
 #include "components/password_manager/core/browser/password_store.h"
 
+using autofill::FormStructure;
+
 namespace password_manager {
 
 class FormSaver;
@@ -216,8 +218,12 @@ class PasswordFormManager : public FormFetcher::Consumer {
   // Called when the user chose not to update password.
   void OnNopeUpdateClicked();
 
-  // Called when the user didn't interact with Update UI.
-  void OnNoInteractionOnUpdate();
+  // Called when the user clicked "Never" button.

[vasilii, 2016/12/08 16:58:14]

... in the "save password" prompt?

[kolos1, 2016/12/09 12:16:08]

Done.

+  void OnNeverClicked();
+
+  // Called when the user didn't interact with UI. |is_update| is true iff
+  // it was the update UI.
+  void OnNoInteraction(bool is_update);
 
   // Saves the outcome of HTML parsing based form classifier to upload proto.
   void SaveGenerationFieldDetectedByClassifier(
@@ -357,36 +363,17 @@ class PasswordFormManager : public FormFetcher::Consumer {
   // UMA.
   int GetActionsTaken() const;
 
-  // Try to label password fields and upload |form_data|. This differs from
-  // AutofillManager::OnFormSubmitted() in a few ways.
-  //   - This function will only label the first <input type="password"> field
-  //     as |password_type|. Other fields will stay unlabeled, as they
-  //     should have been labeled during the upload for OnFormSubmitted().
-  //   - If the |username_field| attribute is nonempty, we will additionally
-  //     label the field with that name as the username field.
-  //   - This function does not assume that |form| is being uploaded during
-  //     the same browsing session as it was originally submitted (as we may
-  //     not have the necessary information to classify the form at that time)
-  //     so it bypasses the cache and doesn't log the same quality UMA metrics.
-  // |login_form_signature| may be empty.  It is non-empty when the user fills
-  // and submits a login form using a generated password. In this case,
-  // |login_form_signature| should be set to the submitted form's signature.
-  // Note that in this case, |form.FormSignature()| gives the signature for the
-  // registration form on which the password was generated, rather than the
-  // submitted form's signature.
-  bool UploadPasswordForm(const autofill::FormData& form_data,
-                          const base::string16& username_field,
+  // Tries to label password fields and upload |form_data|. Returns true on

[vasilii, 2016/12/08 16:58:14]

There is no |form_data|.

[kolos1, 2016/12/09 12:16:08]

Done.

+  // success.
+  // This differs from AutofillManager::OnFormSubmitted(). This function does
+  // not assume that |form| is being uploaded during the same browsing session

[vasilii, 2016/12/08 16:58:14]

There is no |form| parameter either.

[kolos1, 2016/12/09 12:16:08]

Done.

+  // as it was originally submitted (as we may not have the necessary
+  // information to classify the form at that time) so it bypasses the cache and
+  // doesn't log the same quality UMA metrics.

[vasilii, 2016/12/08 16:58:14]

As a newbie I don't understand the comment at all. It's good to know that it's
different from AutofillManager::OnFormSubmitted which I also don't know. It
would be more helpful to describe what it really does instead of doesn't ;-)

[kolos1, 2016/12/09 12:16:08]

This is a part of very old comment to UploadPasswordForm. But actually, nobody
is interested in the difference between UploadPasswordVote and OnFormSubmitted,
I believe. So, I deleted that. 

Now the description of the method is brief and describe only high-level purpose
of the method. 

+  bool UploadPasswordVote(const base::string16& username_field,
                           const autofill::ServerFieldType& password_type,
                           const std::string& login_form_signature);
 
-  // Try to label username, password and new password fields of |observed_form_|
-  // which is considered to be change password forms. Returns true on success.
-  // |password_type| should be equal to NEW_PASSWORD, PROBABLY_NEW_PASSWORD or
-  // NOT_NEW_PASSWORD. These values correspond to cases when the user conrirmed
-  // password update, did nothing or declined to update password respectively.
-  bool UploadChangePasswordForm(const autofill::ServerFieldType& password_type,
-                                const std::string& login_form_signature);
-
   // Adds a vote on password generation usage to |form_structure|.
   void AddGeneratedVote(autofill::FormStructure* form_structure);
 
@@ -436,6 +423,23 @@ class PasswordFormManager : public FormFetcher::Consumer {
   base::Optional<autofill::PasswordForm> UpdatePendingAndGetOldKey(
       std::vector<autofill::PasswordForm>* credentials_to_update);
 
+  // Labels username, password and new password fields in |form_structure| which
+  // is considered to be change password form. |password_type| should be equal
+  // to NEW_PASSWORD, PROBABLY_NEW_PASSWORD or NOT_NEW_PASSWORD. These values
+  // correspond to cases when the user confirmed password update, did nothing or
+  // declined to update password respectively.
+  void LabelFieldsForChangePasswordForm(
+      const autofill::ServerFieldType& password_type,
+      FormStructure* form_structure,
+      autofill::ServerFieldTypeSet* available_field_types);
+
+  // Labels username and password fields in |form_structure|.
+  void LabelFieldsForNonChangePasswordForm(
+      const base::string16& username_field,
+      const autofill::ServerFieldType& password_type,
+      FormStructure* form_structure,
+      autofill::ServerFieldTypeSet* available_field_types);
+
   // Set of nonblacklisted PasswordForms from the DB that best match the form
   // being managed by |this|, indexed by username. They are owned by
   // |form_fetcher_|.