Create a separate structure that tracks the current generation status for a

field in PasswordGenerationAgent.

Currently |generation_element_| has a double meaning. It's an element that
should trigger the automatic generation. However, if a manual generation is
triggered then it's overwritten by a password field. That's confusing.
The CL decouples the field that should trigger the automatic generation from
whatever is happening right now on a random password field.
In the future that will allow us to have multiple generation elements
simultaneously. For now it fixes 870220 where the manual generation on the
ambiguous field was suppressing the automatic generation on it.

Bug: 870220,852309
Change-Id: I6a3edd351e615289f6271fda5a45f6ff47e653af
Reviewed-on: https://chromium-review.googlesource.com/c/1310713Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605668}

chrome/browser/password_manager/password_generation_interactive_uitest.cc

@@ -254,23 +254,28 @@ IN_PROC_BROWSER_TEST_F(PasswordGenerationInteractiveTest,
                        PopupShownManuallyAndPasswordErased) {
   NavigateToFile("/password/password_form.html");
 
-  FocusPasswordField();
+  // Focus the field that is not the first password field. The first one is
+  // considered "automatic" generation field.
+  ASSERT_TRUE(content::ExecuteScript(
+      WebContents(), "document.getElementById('password_redirect').focus()"));
   // The same flow happens when user generates a password from the context menu.
   password_manager_util::UserTriggeredManualGenerationFromContextMenu(
       ChromePasswordManagerClient::FromWebContents(WebContents()));
+  WaitForPopupStatusChange();
   EXPECT_TRUE(GenerationPopupShowing());
   SendKeyToPopup(ui::VKEY_DOWN);
   SendKeyToPopup(ui::VKEY_RETURN);
 
   // Wait until the password is filled.
-  WaitForNonEmptyFieldValue("password_field");
+  WaitForNonEmptyFieldValue("password_redirect");
 
   // Re-focusing the password field should show the editing popup.
-  FocusPasswordField();
+  ASSERT_TRUE(content::ExecuteScript(
+      WebContents(), "document.getElementById('password_redirect').focus()"));
   EXPECT_TRUE(EditingPopupShowing());
 
   // Delete the password. The generation prompt should not be visible.
-  SimulateUserDeletingFieldContent("password_field");
+  SimulateUserDeletingFieldContent("password_redirect");
   WaitForPopupStatusChange();
   EXPECT_FALSE(EditingPopupShowing());
   EXPECT_FALSE(GenerationPopupShowing());

chrome/renderer/autofill/password_generation_agent_browsertest.cc

@@ -789,6 +789,23 @@ TEST_F(PasswordGenerationAgentTest, ManualGenerationNoFormTest) {
   ExpectManualGenerationAvailable("second_password", false);
 }
 
+TEST_F(PasswordGenerationAgentTest, ManualGenerationDoesntSuppressAutomatic) {
+  LoadHTMLWithUserGesture(kAccountCreationFormHTML);
+  SetNotBlacklistedMessage(password_generation_, kAccountCreationFormHTML);
+  SetAccountCreationFormsDetectedMessage(password_generation_,
+                                         GetMainFrame()->GetDocument(), 0, 1);
+  ExpectAutomaticGenerationAvailable("first_password", true);
+  // The browser may show a standard password dropdown with the "Generate"
+  // option. In this case manual generation is triggered.
+  password_generation_->UserTriggeredGeneratePassword();
+
+  // Move the focus away to somewhere.
+  FocusField("address");
+
+  // Moving the focus back should trigger the automatic generation again.
+  ExpectAutomaticGenerationAvailable("first_password", true);
+}
+
 TEST_F(PasswordGenerationAgentTest, PresavingGeneratedPassword) {
   const struct {
     const char* form;

components/autofill/content/renderer/password_generation_agent.h

@@ -13,7 +13,6 @@
 #include <vector>
 
 #include "base/macros.h"
-#include "base/memory/linked_ptr.h"
 #include "components/autofill/content/common/autofill_agent.mojom.h"
 #include "components/autofill/content/common/autofill_driver.mojom.h"
 #include "components/autofill/content/renderer/renderer_save_password_progress_logger.h"
@@ -95,16 +94,11 @@ class PasswordGenerationAgent : public content::RenderFrameObserver,
   void set_enabled(bool enabled) { enabled_ = enabled; }
 
  private:
-  struct AccountCreationFormData {
-    linked_ptr<PasswordForm> form;
-    std::vector<blink::WebInputElement> password_elements;
-
-    AccountCreationFormData(
-        linked_ptr<PasswordForm> form,
-        std::vector<blink::WebInputElement> password_elements);
-    AccountCreationFormData(const AccountCreationFormData& other);
-    ~AccountCreationFormData();
-  };
+  // Contains information about a form for which generation is possible.
+  struct AccountCreationFormData;
+  // Contains information about generation status for an element for the
+  // lifetime of the possible interaction.
+  struct GenerationItemInfo;
 
   typedef std::vector<AccountCreationFormData> AccountCreationFormDataList;
 
@@ -123,9 +117,10 @@ class PasswordGenerationAgent : public content::RenderFrameObserver,
   // |possible_account_creation_form_|.
   void FindPossibleGenerationForm();
 
-  // Helper function to decide if |passwords_| contains password fields for
-  // an account creation form. Sets |generation_element_| to the field that
-  // we want to trigger the generation UI on.
+  // Helper function to decide if |possible_account_creation_forms_| contains
+  // password fields for an account creation form. Sets
+  // |automatic_generation_element_| to the field that we want to trigger the
+  // generation UI on.
   void DetermineGenerationElement();
 
   // Helper function which takes care of the form processing and collecting the
@@ -186,48 +181,27 @@ class PasswordGenerationAgent : public content::RenderFrameObserver,
   // forms will not be sent if the feature is disabled.
   std::vector<autofill::PasswordFormGenerationData> generation_enabled_forms_;
 
-  // Data for form which generation is allowed on.
-  std::unique_ptr<AccountCreationFormData> generation_form_data_;
+  // Data for form which automatic generation is allowed on.
+  std::unique_ptr<AccountCreationFormData> automatic_generation_form_data_;
+
+  // Element where we want to trigger automatic password generation UI on.
+  blink::WebInputElement automatic_generation_element_;
 
-  // Element where we want to trigger password generation UI.
-  blink::WebInputElement generation_element_;
+  // Contains the current element where generation is offered at the moment. It
+  // can be either automatic or manual password generation.
+  std::unique_ptr<GenerationItemInfo> current_generation_item_;
 
   // Password element that had focus last. Since Javascript could change focused
   // element after the user triggered a generation request, it is better to save
   // the last focused password element.
   blink::WebInputElement last_focused_password_element_;
 
-  // If the password field at |generation_element_| contains a generated
-  // password.
-  bool password_is_generated_;
-
-  // True if the last password generation was manually triggered.
-  bool is_manually_triggered_;
-
-  // True if a password was generated and the user edited it. Used for UMA
-  // stats.
-  bool password_edited_;
-
-  // True if the generation popup was shown during this navigation. Used to
-  // track UMA stats per page visit rather than per display, since the former
-  // is more interesting.
-  // TODO(crbug.com/845458): Remove this or change the description of the
-  // logged event as calling AutomaticgenerationStatusChanged will no longer
-  // imply that a popup is shown. This could instead be logged with the
-  // metrics collected on the browser process.
-  bool generation_popup_shown_;
-
-  // True if the editing popup was shown during this navigation. Used to track
-  // UMA stats per page rather than per display, since the former is more
-  // interesting.
-  bool editing_popup_shown_;
-
   // If this feature is enabled. Controlled by Finch.
   bool enabled_;
 
   // True iff the generation element should be marked with special HTML
   // attribute (only for experimental purposes).
-  bool mark_generation_element_;
+  const bool mark_generation_element_;
 
   // Unowned pointer. Used to notify PassowrdAutofillAgent when values
   // in password fields are updated.