Bug 1652715 - fix pmgr doorhanger only updating on password change & submit;r=MattN
authorSeverin <srudie@mozilla.com>
Thu, 23 Jul 2020 17:14:42 +0000
changeset 541788 5acd57b4187dd9dd3a2762f9f6f611b0d32267ed
parent 541787 6259301066022496a1072f1ac1b9a944a15511ae
child 541789 9168828ce9e9e74107bd8a64324981e6d5c70ea7
push id37633
push userccoroiu@mozilla.com
push dateFri, 24 Jul 2020 09:32:06 +0000
treeherdermozilla-central@141543043270 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1652715
milestone80.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1652715 - fix pmgr doorhanger only updating on password change & submit;r=MattN Differential Revision: https://phabricator.services.mozilla.com/D83853
toolkit/components/passwordmgr/LoginManagerChild.jsm
toolkit/components/passwordmgr/LoginManagerParent.jsm
toolkit/components/passwordmgr/test/browser/browser_doorhanger_autocomplete_values.js
--- a/toolkit/components/passwordmgr/LoginManagerChild.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerChild.jsm
@@ -250,17 +250,28 @@ const observer = {
         let formLikeRoot = FormLikeFactory.findRootForField(
           aEvent.composedTarget
         );
         if (!docState.fieldModificationsByRootElement.get(formLikeRoot)) {
           log("Ignoring change event on form that hasn't been user-modified");
           break;
         }
 
+        // _storeUserInput mutates docstate
         this._storeUserInput(docState, aEvent.composedTarget);
+        let detail = {
+          possibleValues: {
+            usernames: docState.possibleUsernames,
+            passwords: docState.possiblePasswords,
+          },
+        };
+        LoginManagerChild.forWindow(window).sendAsyncMessage(
+          "PasswordManager:updateDoorhangerSuggestions",
+          detail
+        );
 
         if (aEvent.composedTarget.hasBeenTypePassword) {
           let triggeredByFillingGenerated = docState.generatedPasswordFields.has(
             aEvent.composedTarget
           );
           // Autosave generated password initial fills and subsequent edits
           if (triggeredByFillingGenerated) {
             LoginManagerChild.forWindow(window)._passwordEditedOrGenerated(
--- a/toolkit/components/passwordmgr/LoginManagerParent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerParent.jsm
@@ -132,16 +132,25 @@ async function getImportableLogins(formO
     ? {
         browsers: await ChromeMigrationUtils.getImportableLogins(formOrigin),
         state,
       }
     : null;
 }
 
 class LoginManagerParent extends JSWindowActorParent {
+  possibleValues = {
+    // This is stored at the parent (i.e., frame) scope because the LoginManagerPrompter
+    // is shared across all frames.
+    //
+    // It is mutated to update values without forcing us to set a new doorhanger.
+    usernames: new Set(),
+    passwords: new Set(),
+  };
+
   // This is used by tests to listen to form submission.
   static setListenerForTests(listener) {
     gListenerForTests = listener;
   }
 
   // Used by tests to clean up recipes only when they were actually used.
   static get _recipeManager() {
     return gRecipeManager;
@@ -236,16 +245,22 @@ class LoginManagerParent extends JSWindo
         this.manager.documentPrincipal?.originNoSuffix
       );
       if (!origin) {
         throw new Error("An origin is required. Message name: " + msg.name);
       }
       return origin;
     });
     switch (msg.name) {
+      case "PasswordManager:updateDoorhangerSuggestions": {
+        this.possibleValues.usernames = data.possibleValues.usernames;
+        this.possibleValues.passwords = data.possibleValues.passwords;
+        break;
+      }
+
       case "PasswordManager:findLogins": {
         return this.sendLoginDataToChild(
           context.origin,
           data.actionOrigin,
           data.options
         );
       }
 
@@ -679,17 +694,16 @@ class LoginManagerParent extends JSWindo
     {
       browsingContextId,
       formActionOrigin,
       autoFilledLoginGuid,
       usernameField,
       newPasswordField,
       oldPasswordField,
       dismissedPrompt,
-      possibleValues,
     }
   ) {
     function recordLoginUse(login) {
       Services.logins.recordPasswordUse(
         login,
         browser && PrivateBrowsingUtils.isBrowserPrivate(browser),
         login.username ? "form_login" : "form_password",
         !!autoFilledLoginGuid
@@ -844,46 +858,46 @@ class LoginManagerParent extends JSWindo
         prompter.promptToChangePassword(
           promptBrowser,
           existingLogin,
           formLogin,
           dismissedPrompt,
           notifySaved,
           autoSavedStorageGUID,
           autoFilledLoginGuid,
-          possibleValues
+          this.possibleValues
         );
       } else if (!existingLogin.username && formLogin.username) {
         log("...empty username update, prompting to change.");
         let prompter = this._getPrompter(browser);
         prompter.promptToChangePassword(
           promptBrowser,
           existingLogin,
           formLogin,
           dismissedPrompt,
           notifySaved,
           autoSavedStorageGUID,
           autoFilledLoginGuid,
-          possibleValues
+          this.possibleValues
         );
       } else {
         recordLoginUse(existingLogin);
       }
 
       return;
     }
 
     // Prompt user to save login (via dialog or notification bar)
     prompter.promptToSavePassword(
       promptBrowser,
       formLogin,
       dismissedPrompt,
       notifySaved,
       autoFilledLoginGuid,
-      possibleValues
+      this.possibleValues
     );
   }
 
   /**
    * Performs validation of inputs against already-saved logins in order to determine whether and
    * how these inputs can be stored. Depending on validation, will either no-op or show a 'save'
    * or 'update' dialog to the user.
    *
@@ -895,33 +909,27 @@ class LoginManagerParent extends JSWindo
    * @param {Element} browser
    * @param {string} formOrigin
    * @param {string} options.formActionOrigin
    * @param {string?} options.autoFilledLoginGuid
    * @param {Object} options.newPasswordField
    * @param {Object?} options.usernameField
    * @param {Element?} options.oldPasswordField
    * @param {boolean} [options.triggeredByFillingGenerated = false]
-   * @param {Set<String>} possibleValues.usernames
-   * @param {Set<String>} possibleValues.passwords
    */
   async _onPasswordEditedOrGenerated(
     browser,
     formOrigin,
     {
       formActionOrigin,
       autoFilledLoginGuid,
       newPasswordField,
       usernameField = null,
       oldPasswordField,
       triggeredByFillingGenerated = false,
-      possibleValues = {
-        usernames: new Set(),
-        passwords: new Set(),
-      },
     }
   ) {
     log(
       "_onPasswordEditedOrGenerated, triggeredByFillingGenerated:",
       triggeredByFillingGenerated
     );
 
     // If password storage is disabled, bail out.
@@ -1220,29 +1228,29 @@ class LoginManagerParent extends JSWindo
         prompter.promptToChangePassword(
           promptBrowser,
           existingLogin,
           formLogin,
           true, // dismissed prompt
           notifySaved,
           autoSavedStorageGUID, // autoSavedLoginGuid
           autoFilledLoginGuid,
-          possibleValues
+          this.possibleValues
         );
       } else if (!existingLogin.username && formLogin.username) {
         log("...empty username update, prompting to change.");
         prompter.promptToChangePassword(
           promptBrowser,
           existingLogin,
           formLogin,
           true, // dismissed prompt
           notifySaved,
           autoSavedStorageGUID, // autoSavedLoginGuid
           autoFilledLoginGuid,
-          possibleValues
+          this.possibleValues
         );
       } else {
         log("_onPasswordEditedOrGenerated: No change to existing login");
         // is there a doorhanger we should update?
         let popupNotifications = promptBrowser.ownerGlobal.PopupNotifications;
         let notif = popupNotifications.getNotification("password", browser);
         log(
           "_onPasswordEditedOrGenerated: Has doorhanger?",
@@ -1252,30 +1260,30 @@ class LoginManagerParent extends JSWindo
           prompter.promptToChangePassword(
             promptBrowser,
             existingLogin,
             formLogin,
             true, // dismissed prompt
             notifySaved,
             autoSavedStorageGUID, // autoSavedLoginGuid
             autoFilledLoginGuid,
-            possibleValues
+            this.possibleValues
           );
         }
       }
       return;
     }
     log("_onPasswordEditedOrGenerated: no matching login to save/update");
     prompter.promptToSavePassword(
       promptBrowser,
       formLogin,
       true, // dismissed prompt
       notifySaved,
       autoFilledLoginGuid,
-      possibleValues
+      this.possibleValues
     );
   }
 
   static get recipeParentPromise() {
     if (!gRecipeManager) {
       const { LoginRecipesParent } = ChromeUtils.import(
         "resource://gre/modules/LoginRecipes.jsm"
       );
--- a/toolkit/components/passwordmgr/test/browser/browser_doorhanger_autocomplete_values.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_doorhanger_autocomplete_values.js
@@ -97,16 +97,27 @@ const TEST_CASES = [
       { [PASSWORD_SELECTOR]: "myPassword" },
       { [USERNAME_SELECTOR]: "new_username1" },
       { [USERNAME_SELECTOR]: "new_username2" },
       { [USERNAME_SELECTOR]: "new_username1" },
     ],
     expectUsernameDropmarker: true,
     expectedValues: ["new_username1", "new_username2"],
   },
+  {
+    description: "non-un/pw fields also prompt doorhanger updates",
+    modifiedFields: [
+      { [PASSWORD_SELECTOR]: "myPassword" },
+      { [USERNAME_SELECTOR]: "new_username1" },
+      { [SEARCH_SELECTOR]: "search" },
+      { [CAPTCHA_SELECTOR]: "captcha" },
+    ],
+    expectUsernameDropmarker: true,
+    expectedValues: ["new_username1", "search", "captcha"],
+  },
   // {
   //   description: "duplicated saved/page usernames should TODO https://mozilla.invisionapp.com/share/XGXL6WZVKFJ#/screens/420547613/comments",
   // },
 ];
 
 function _validateTestCase(tc) {
   if (tc.expectUsernameDropmarker) {
     ok(