Bug 1538460 - Only defer handling fields for login autofill when master password is not set. r=MattN a=pascalc
authorSam Foster <sfoster@mozilla.com>
Mon, 15 Apr 2019 18:48:03 +0000
changeset 523248 d2668e1efcfd00856dd3c49eec1aa52b6ed7c095
parent 523247 f350dc70a86486f45ee7e2414332d9c058445617
child 523249 1cf183a8be97f72bc3d61a866ba76a41ffcd4289
push id11115
push useraiakab@mozilla.com
push dateFri, 19 Apr 2019 16:14:22 +0000
treeherdermozilla-beta@86f6ec90b34b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN, pascalc
bugs1538460
milestone67.0
Bug 1538460 - Only defer handling fields for login autofill when master password is not set. r=MattN a=pascalc * Use Services.(ppmm|cpmm).sharedData to make isMasterPasswordSet value available to the content process * We don't handle runtime enable/disable of MP Differential Revision: https://phabricator.services.mozilla.com/D26939
toolkit/components/passwordmgr/LoginManager.jsm
toolkit/components/passwordmgr/LoginManagerContent.jsm
toolkit/components/passwordmgr/test/browser/browser.ini
toolkit/components/passwordmgr/test/browser/browser_hidden_document_autofill.js
--- a/toolkit/components/passwordmgr/LoginManager.jsm
+++ b/toolkit/components/passwordmgr/LoginManager.jsm
@@ -90,21 +90,24 @@ LoginManager.prototype = {
 
       // Initialize storage so that asynchronous data loading can start.
       this._initStorage();
     }
 
     Services.obs.addObserver(this._observer, "gather-telemetry");
   },
 
-
   _initStorage() {
     this._storage = Cc["@mozilla.org/login-manager/storage/default;1"]
                     .createInstance(Ci.nsILoginManagerStorage);
     this.initializationPromise = this._storage.initialize();
+    this.initializationPromise.then(() => {
+      log.debug("initializationPromise is resolved, updating isMasterPasswordSet in sharedData");
+      Services.ppmm.sharedData.set("isMasterPasswordSet", LoginHelper.isMasterPasswordSet());
+    });
   },
 
 
   /* ---------- Utility objects ---------- */
 
 
   /**
    * Internal utility object, implements the nsIObserver interface.
--- a/toolkit/components/passwordmgr/LoginManagerContent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ -424,18 +424,25 @@ var LoginManagerContent = {
     }
     onVisibleTasks.push(fn);
   },
 
   onDOMFormHasPassword(event) {
     if (!event.isTrusted) {
       return;
     }
+    let isMasterPasswordSet = Services.cpmm.sharedData.get("isMasterPasswordSet");
     let document = event.target.ownerDocument;
-    if (document.visibilityState == "visible") {
+
+    // don't attempt to defer handling when a master password is set
+    // Showing the MP modal as soon as possible minimizes its interference with tab interactions
+    // See bug 1539091 and bug 1538460.
+    log("onDOMFormHasPassword, visibilityState:", document.visibilityState,
+        "isMasterPasswordSet:", isMasterPasswordSet);
+    if (document.visibilityState == "visible" || isMasterPasswordSet) {
       this._processDOMFormHasPasswordEvent(event);
     } else {
       // wait until the document becomes visible before handling this event
       this._deferHandlingEventUntilDocumentVisible(event, document, () => {
         this._processDOMFormHasPasswordEvent(event);
       });
     }
   },
@@ -454,17 +461,24 @@ var LoginManagerContent = {
 
     let pwField = event.originalTarget;
     if (pwField.form) {
       // Fill is handled by onDOMFormHasPassword which is already throttled.
       return;
     }
 
     let document = pwField.ownerDocument;
-    if (document.visibilityState == "visible") {
+    let isMasterPasswordSet = Services.cpmm.sharedData.get("isMasterPasswordSet");
+    log("onDOMInputPasswordAdded, visibilityState:", document.visibilityState,
+        "isMasterPasswordSet:", isMasterPasswordSet);
+
+    // don't attempt to defer handling when a master password is set
+    // Showing the MP modal as soon as possible minimizes its interference with tab interactions
+    // See bug 1539091 and bug 1538460.
+    if (document.visibilityState == "visible" || isMasterPasswordSet) {
       this._processDOMInputPasswordAddedEvent(event, topWindow);
     } else {
       // wait until the document becomes visible before handling this event
       this._deferHandlingEventUntilDocumentVisible(event, document, () => {
         this._processDOMInputPasswordAddedEvent(event, topWindow);
       });
     }
   },
--- a/toolkit/components/passwordmgr/test/browser/browser.ini
+++ b/toolkit/components/passwordmgr/test/browser/browser.ini
@@ -58,16 +58,17 @@ support-files =
   file_focus_before_DOMContentLoaded.sjs
 [browser_formless_submit_chrome.js]
 [browser_hasInsecureLoginForms.js]
 skip-if = verify
 [browser_hasInsecureLoginForms_streamConverter.js]
 [browser_http_autofill.js]
 skip-if = verify
 [browser_hidden_document_autofill.js]
+skip-if = os == "win" && os_version == "10.0" && debug # bug 1530935
 [browser_insecurePasswordConsoleWarning.js]
 skip-if = verify
 [browser_master_password_autocomplete.js]
 [browser_notifications.js]
 [browser_notifications_username.js]
 [browser_notifications_password.js]
 [browser_notifications_2.js]
 skip-if = os == "linux" # Bug 1272849 Main action button disabled state intermittent
--- a/toolkit/components/passwordmgr/test/browser/browser_hidden_document_autofill.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_hidden_document_autofill.js
@@ -124,61 +124,52 @@ testUrls.forEach(testUrl => {
     is(fieldValues.username, "user1", "Checking filled username");
     is(fieldValues.password, "pass1", "Checking filled password");
 
     gBrowser.removeTab(tab1);
     gBrowser.removeTab(tab2);
   });
 });
 
-add_task(async function test_defer_autofill_with_masterpassword() {
+add_task(async function test_immediate_autofill_with_masterpassword() {
   // Set master password prompt timeout to 3s.
   // If this test goes intermittent, you likely have to increase this value.
   await SpecialPowers.pushPrefEnv({set: [["signon.masterPasswordReprompt.timeout_ms", 3000]]});
-  LoginTestUtils.masterPassword.enable();
 
-  registerCleanupFunction(function() {
+  LoginTestUtils.masterPassword.enable();
+  await LoginTestUtils.reloadData();
+  info(`Have enabled masterPassword, now isLoggedIn? ${Services.logins.isLoggedIn}`);
+
+  registerCleanupFunction(async function() {
     LoginTestUtils.masterPassword.disable();
+    await LoginTestUtils.reloadData();
   });
 
-  let result, tab1Visibility, dialogObserved;
+  let dialogResult, tab1Visibility, dialogObserved;
+
   // open 2 tabs
   const tab1 = await BrowserTestUtils.openNewForegroundTab(gBrowser, INITIAL_URL);
   const tab2 = await BrowserTestUtils.openNewForegroundTab(gBrowser, INITIAL_URL);
 
-  info("sanity check by first loading a form into the visible tab");
+  info("load a background login form tab with a matching saved login " +
+       "and wait to see if the master password dialog is shown");
   is(await getDocumentVisibilityState(tab2.linkedBrowser),
      "visible", "The second tab should be visible");
-  result = { wasShown: false };
 
-  dialogObserved = observeMasterPasswordDialog(tab2.ownerGlobal, result);
-  await BrowserTestUtils.loadURI(tab2.linkedBrowser, FORM_URL);
-  await dialogObserved;
-  ok(result.wasShown, "Dialog should be shown when form is loaded into a visible document");
-
-  info("load a background login form tab with a matching saved login " +
-       "and wait to see if the master password dialog is shown");
-  // confirm document is hidden
   tab1Visibility = await getDocumentVisibilityState(tab1.linkedBrowser);
   is(tab1Visibility, "hidden", "The first tab should be backgrounded");
-  result = { wasShown: false };
 
-  dialogObserved = observeMasterPasswordDialog(tab1.ownerGlobal, result);
-  await BrowserTestUtils.loadURI(tab1.linkedBrowser, FORM_URL);
-  await dialogObserved;
-  ok(!result.wasShown, "Dialog should not be shown when form is loaded into a hidden document");
+  dialogResult = { wasShown: false };
+  dialogObserved = observeMasterPasswordDialog(tab1.ownerGlobal, dialogResult);
 
-  info("switch to the form tab " +
-       "and confirm the master password dialog is then shown");
-  tab1Visibility = await getDocumentVisibilityState(tab1.linkedBrowser);
-  is(tab1Visibility, "hidden", "The first tab should be backgrounded");
-  result = { wasShown: false };
+  // In this case we will try to autofill while hidden, so look for the passwordmgr-processed-form
+  // to be observed
+  await addContentObserver(tab1.linkedBrowser, "passwordmgr-processed-form");
+  await BrowserTestUtils.loadURI(tab1.linkedBrowser, FORM_URL);
+  let wasProcessed = getContentObserverResult(tab1.linkedBrowser, "passwordmgr-processed-form");
+  await Promise.all([dialogObserved, wasProcessed]);
 
-  dialogObserved = observeMasterPasswordDialog(tab1.ownerGlobal, result);
-  await BrowserTestUtils.switchTab(gBrowser, tab1);
-  await dialogObserved;
-  tab1Visibility = await getDocumentVisibilityState(tab1.linkedBrowser);
-  is(tab1Visibility, "visible", "The first tab should be foreground");
-  ok(result.wasShown, "Dialog should be shown when input's document becomes visible");
+  ok(wasProcessed, "Observer should be notified when form is loaded into a hidden document");
+  ok(dialogResult && dialogResult.wasShown, "MP Dialog should be shown when form is loaded into a hidden document");
 
   gBrowser.removeTab(tab1);
   gBrowser.removeTab(tab2);
 });