Bug 1538460 - Only defer handling fields for login autofill when master password is not set. r=MattN
authorSam Foster <sfoster@mozilla.com>
Mon, 15 Apr 2019 18:48:03 +0000
changeset 469543 2f6e8916a498
parent 469542 a33df20e8c10
child 469544 03c29de4a29d
push id35874
push userccoroiu@mozilla.com
push dateTue, 16 Apr 2019 04:04:58 +0000
treeherdermozilla-central@be3f40425b52 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1538460
milestone68.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 1538460 - Only defer handling fields for login autofill when master password is not set. r=MattN * 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
@@ -82,21 +82,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
@@ -440,18 +440,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);
       });
     }
   },
@@ -472,17 +479,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);
 });