Backed out 2 changesets (bug 1547693, bug 1545167) for xpcshell failures on test_handlerService.js . CLOSED TREE

Backed out changeset 0e6e758f47b5 (bug 1545167)
Backed out changeset f74a32d5753a (bug 1547693)

browser/base/content/test/performance/browser_startup_mainthreadio.js

@@ -289,6 +289,13 @@ const startupPhases = {
       read: 22,
       close: 11,
     },
+    {
+      // bug 1545167
+      path: "/etc/mime.types",
+      condition: LINUX,
+      read: 3,
+      close: 3,
+    },
     {
       // bug 1541246
       path: "ProfD:extensions",
@@ -350,6 +357,20 @@ const startupPhases = {
       condition: WIN,
       stat: 1,
     },
+    // Bug 1547693
+    {
+      path: "*WindowsApps/microsoft.windowscommunicationsapps*",
+      condition: WIN,
+      ignoreIfUnused: true,
+      stat: 3,
+    },
+    // Bug 1545167
+    {
+      path: "*Microsoft.MicrosoftEdge*",
+      condition: WIN,
+      ignoreIfUnused: true,
+      stat: 3,
+    },
     {
       // bug 1545139
       path: "*Fonts/StaticCache.dat",

browser/base/content/test/performance/io/browser.ini

@@ -9,6 +9,9 @@ skip-if = debug || (os == "linux" && asan) || (os == "win" && processor == "aarc
 # set during early startup to have an impact as a canvas will be used by
 # startupRecorder.js
 prefs =
+  # Skip migration work in BG__migrateUI for browser_startup.js since it isn't
+  # representative of common startup, and triggers Places I/O.
+  browser.migration.version=9999999
   browser.startup.record=true
   gfx.canvas.willReadFrequently.enable=true
   # The Screenshots extension is disabled by default in Mochitests. We re-enable

browser/components/BrowserGlue.jsm

@@ -989,7 +989,7 @@ BrowserGlue.prototype = {
         // parent only: configure default prefs, set up pref observers, register
         // pdf content handler, and initializes parent side message manager
         // shim for privileged api access.
-        PdfJs.init(this._isNewProfile);
+        PdfJs.init();
         break;
       case "shield-init-complete":
         this._shieldInitComplete = true;
@@ -1123,6 +1123,15 @@ BrowserGlue.prototype = {
   _beforeUIStartup: function BG__beforeUIStartup() {
     SessionStartup.init();
 
+    if (Services.prefs.prefHasUserValue(PREF_PDFJS_ENABLED_CACHE_STATE)) {
+      Services.ppmm.sharedData.set(
+        "pdfjs.enabled",
+        Services.prefs.getBoolPref(PREF_PDFJS_ENABLED_CACHE_STATE)
+      );
+    } else {
+      PdfJs.earlyInit();
+    }
+
     // check if we're in safe mode
     if (Services.appinfo.inSafeMode) {
       Services.ww.openWindow(
@@ -1140,15 +1149,6 @@ BrowserGlue.prototype = {
     // handle any UI migration
     this._migrateUI();
 
-    if (Services.prefs.prefHasUserValue(PREF_PDFJS_ENABLED_CACHE_STATE)) {
-      Services.ppmm.sharedData.set(
-        "pdfjs.enabled",
-        Services.prefs.getBoolPref(PREF_PDFJS_ENABLED_CACHE_STATE)
-      );
-    } else {
-      PdfJs.earlyInit(this._isNewProfile);
-    }
-
     listeners.init();
 
     SessionStore.init();

browser/extensions/pdfjs/content/PdfJs.jsm

@@ -75,31 +75,11 @@ function initializeDefaultPreferences() {
   }
 }
 
-// We're supposed to get this type of thing from the OS, and generally we do.
-// But doing so is expensive, so on startup paths we can use this to make the
-// handler service get and store the Right Thing (it just goes into a JSON
-// file) and avoid the performance issues.
-const gPdfFakeHandlerInfo = {
-  QueryInterface: ChromeUtils.generateQI([Ci.nsIMIMEInfo]),
-  getFileExtensions() {
-    return ["pdf"];
-  },
-  possibleApplicationHandlers: Cc["@mozilla.org/array;1"].createInstance(
-    Ci.nsIMutableArray
-  ),
-  extensionExists(ext) {
-    return ext == "pdf";
-  },
-  alwaysAskBeforeHandling: false,
-  preferredAction: Ci.nsIHandlerInfo.handleInternally,
-  type: PDF_CONTENT_TYPE,
-};
-
 var PdfJs = {
   QueryInterface: ChromeUtils.generateQI([Ci.nsIObserver]),
   _initialized: false,
 
-  init: function init(isNewProfile) {
+  init: function init() {
     if (
       Services.appinfo.processType !== Services.appinfo.PROCESS_TYPE_DEFAULT
     ) {
@@ -110,19 +90,13 @@ var PdfJs = {
     PdfjsChromeUtils.init();
     this.initPrefs();
 
-    Services.ppmm.sharedData.set(
-      "pdfjs.enabled",
-      this.checkEnabled(isNewProfile)
-    );
+    Services.ppmm.sharedData.set("pdfjs.enabled", this.checkEnabled());
   },
 
-  earlyInit(isNewProfile) {
+  earlyInit() {
     // Note: Please keep this in sync with the duplicated logic in
     // BrowserGlue.jsm.
-    Services.ppmm.sharedData.set(
-      "pdfjs.enabled",
-      this.checkEnabled(isNewProfile)
-    );
+    Services.ppmm.sharedData.set("pdfjs.enabled", this.checkEnabled());
   },
 
   initPrefs: function initPrefs() {
@@ -171,38 +145,23 @@ var PdfJs = {
   },
 
   _becomeHandler: function _becomeHandler() {
-    // Normally, this happens in the first run at some point, where the
-    // handler service doesn't have any info on user preferences yet.
-    // Don't bother storing old defaults in this case, as they're
-    // meaningless anyway.
-    if (!Svc.handlerService.exists(gPdfFakeHandlerInfo)) {
-      // Store the requisite info into the DB, and nothing else:
-      Svc.handlerService.store(gPdfFakeHandlerInfo);
-    } else {
-      let handlerInfo = Svc.mime.getFromTypeAndExtension(
-        PDF_CONTENT_TYPE,
-        "pdf"
-      );
-      let prefs = Services.prefs;
-      if (
-        handlerInfo.preferredAction !== Ci.nsIHandlerInfo.handleInternally &&
-        handlerInfo.alwaysAskBeforeHandling !== false
-      ) {
-        // Store the previous settings of preferredAction and
-        // alwaysAskBeforeHandling in case we need to revert them in a hotfix that
-        // would turn pdf.js off.
-        prefs.setIntPref(PREF_PREVIOUS_ACTION, handlerInfo.preferredAction);
-        prefs.setBoolPref(
-          PREF_PREVIOUS_ASK,
-          handlerInfo.alwaysAskBeforeHandling
-        );
-      }
-
-      // Change and save mime handler settings.
-      handlerInfo.alwaysAskBeforeHandling = false;
-      handlerInfo.preferredAction = Ci.nsIHandlerInfo.handleInternally;
-      Svc.handlerService.store(handlerInfo);
+    let handlerInfo = Svc.mime.getFromTypeAndExtension(PDF_CONTENT_TYPE, "pdf");
+    let prefs = Services.prefs;
+    if (
+      handlerInfo.preferredAction !== Ci.nsIHandlerInfo.handleInternally &&
+      handlerInfo.alwaysAskBeforeHandling !== false
+    ) {
+      // Store the previous settings of preferredAction and
+      // alwaysAskBeforeHandling in case we need to revert them in a hotfix that
+      // would turn pdf.js off.
+      prefs.setIntPref(PREF_PREVIOUS_ACTION, handlerInfo.preferredAction);
+      prefs.setBoolPref(PREF_PREVIOUS_ASK, handlerInfo.alwaysAskBeforeHandling);
     }
+
+    // Change and save mime handler settings.
+    handlerInfo.alwaysAskBeforeHandling = false;
+    handlerInfo.preferredAction = Ci.nsIHandlerInfo.handleInternally;
+    Svc.handlerService.store(handlerInfo);
   },
 
   _unbecomeHandler: function _unbecomeHandler() {
@@ -226,13 +185,7 @@ var PdfJs = {
     }
   },
 
-  /**
-   * @param isNewProfile used to decide whether we need to check the
-   *                     handler service to see if the user configured
-   *                     pdfs differently. If we're on a new profile,
-   *                     there's no need to check.
-   */
-  _isEnabled(isNewProfile) {
+  _isEnabled: function _isEnabled() {
     let { processType, PROCESS_TYPE_DEFAULT } = Services.appinfo;
     if (processType !== PROCESS_TYPE_DEFAULT) {
       throw new Error(
@@ -244,20 +197,12 @@ var PdfJs = {
       return false;
     }
 
-    // Don't bother with the handler service on a new profile:
-    if (isNewProfile) {
-      return true;
-    }
     // Check if the 'application/pdf' preview handler is configured properly.
-    let handlerInfo = Svc.mime.getFromTypeAndExtension(PDF_CONTENT_TYPE, "pdf");
-    return (
-      !handlerInfo.alwaysAskBeforeHandling &&
-      handlerInfo.preferredAction === Ci.nsIHandlerInfo.handleInternally
-    );
+    return PdfjsChromeUtils.isDefaultHandlerApp();
   },
 
-  checkEnabled(isNewProfile) {
-    let isEnabled = this._isEnabled(isNewProfile);
+  checkEnabled: function checkEnabled() {
+    let isEnabled = this._isEnabled();
     // This will be updated any time we observe a dependency changing, since
     // updateRegistration internally calls enabled.
     Services.prefs.setBoolPref(PREF_ENABLED_CACHE_STATE, isEnabled);

browser/extensions/pdfjs/content/PdfjsChromeUtils.jsm

@@ -18,6 +18,7 @@
 var EXPORTED_SYMBOLS = ["PdfjsChromeUtils"];
 
 const PREF_PREFIX = "pdfjs";
+const PDF_CONTENT_TYPE = "application/pdf";
 
 const { XPCOMUtils } = ChromeUtils.import(
   "resource://gre/modules/XPCOMUtils.jsm"
@@ -65,6 +66,7 @@ var PdfjsChromeUtils = {
       this._ppmm.addMessageListener("PDFJS:Parent:setBoolPref", this);
       this._ppmm.addMessageListener("PDFJS:Parent:setCharPref", this);
       this._ppmm.addMessageListener("PDFJS:Parent:setStringPref", this);
+      this._ppmm.addMessageListener("PDFJS:Parent:isDefaultHandlerApp", this);
 
       // global dom message manager (MMg)
       this._mmg = Services.mm;
@@ -87,6 +89,10 @@ var PdfjsChromeUtils = {
       this._ppmm.removeMessageListener("PDFJS:Parent:setBoolPref", this);
       this._ppmm.removeMessageListener("PDFJS:Parent:setCharPref", this);
       this._ppmm.removeMessageListener("PDFJS:Parent:setStringPref", this);
+      this._ppmm.removeMessageListener(
+        "PDFJS:Parent:isDefaultHandlerApp",
+        this
+      );
 
       this._mmg.removeMessageListener("PDFJS:Parent:displayWarning", this);
 
@@ -129,6 +135,8 @@ var PdfjsChromeUtils = {
       case "PDFJS:Parent:setStringPref":
         this._setStringPref(aMsg.data.name, aMsg.data.value);
         break;
+      case "PDFJS:Parent:isDefaultHandlerApp":
+        return this.isDefaultHandlerApp();
       case "PDFJS:Parent:displayWarning":
         this._displayWarning(aMsg);
         break;
@@ -336,6 +344,19 @@ var PdfjsChromeUtils = {
     Services.prefs.setStringPref(aPrefName, aPrefValue);
   },
 
+  /*
+   * Svc.mime doesn't have profile information in the child, so
+   * we bounce this pdfjs enabled configuration check over to the
+   * parent.
+   */
+  isDefaultHandlerApp() {
+    var handlerInfo = Svc.mime.getFromTypeAndExtension(PDF_CONTENT_TYPE, "pdf");
+    return (
+      !handlerInfo.alwaysAskBeforeHandling &&
+      handlerInfo.preferredAction === Ci.nsIHandlerInfo.handleInternally
+    );
+  },
+
   /*
    * Display a notification warning when the renderer isn't sure
    * a pdf displayed correctly.

uriloader/exthandler/HandlerService.js

@@ -116,8 +116,6 @@ HandlerService.prototype = {
         this._store.data.defaultHandlersVersion[
           locale
         ] = prefsDefaultHandlersVersion;
-        // Now save the result:
-        this._store.saveSoon();
       }
     } catch (ex) {
       Cu.reportError(ex);
@@ -153,41 +151,22 @@ HandlerService.prototype = {
       } catch (ex) {}
     }
 
-    // Now, we're going to cheat. Terribly. The idiologically correct way
-    // of implementing the following bit of code would be to fetch the
-    // handler info objects from the protocol service, manipulate those,
-    // and then store each of them.
-    // However, that's expensive. It causes us to talk to the OS about
-    // default apps, which causes the OS to go hit the disk.
-    // All we're trying to do is insert some web apps into the list. We
-    // don't care what's already in the file, we just want to do the
-    // equivalent of appending into the database. So let's just go do that:
     for (let scheme of Object.keys(schemes)) {
-      let existingSchemeInfo = this._store.data.schemes[scheme];
-      if (!this._store.data.schemes[scheme]) {
-        // Haven't seen this scheme before. Default to asking which app the
-        // user wants to use:
-        existingSchemeInfo = {
-          // Signal to future readers that we didn't ask the OS anything.
-          // When the entry is first used, get the info from the OS.
-          stubEntry: true,
-          // The first item in the list is the preferred handler, and
-          // there isn't one, so we fill in null:
-          handlers: [null],
-        };
-        this._store.data.schemes[scheme] = existingSchemeInfo;
-      }
-      let { handlers } = existingSchemeInfo;
+      let protoInfo = gExternalProtocolService.getProtocolHandlerInfo(scheme);
+
+      // cache the possible handlers to avoid extra xpconnect traversals.
+      let possibleHandlers = protoInfo.possibleApplicationHandlers;
+
       for (let handlerNumber of Object.keys(schemes[scheme])) {
-        let newHandler = schemes[scheme][handlerNumber];
+        let handlerApp = this.handlerAppFromSerializable(
+          schemes[scheme][handlerNumber]
+        );
         // If there is already a handler registered with the same template
-        // URL, ignore the new one:
-        let matchingTemplate = handler =>
-          handler && handler.uriTemplate == newHandler.uriTemplate;
-        if (!handlers.some(matchingTemplate)) {
-          handlers.push(newHandler);
-        }
+        // URL, the newly added one will be ignored when saving.
+        possibleHandlers.appendElement(handlerApp);
       }
+
+      this.store(protoInfo);
     }
   },
 
@@ -417,9 +396,6 @@ HandlerService.prototype = {
       }
     }
 
-    // If we're saving *anything*, it stops being a stub:
-    delete storedHandlerInfo.stubEntry;
-
     this._store.saveSoon();
   },
 
@@ -436,33 +412,22 @@ HandlerService.prototype = {
       );
     }
 
-    let isStub = !!storedHandlerInfo.stubEntry;
-    // In the normal case, this is not a stub, so we can just read stored info
-    // and write to the handlerInfo object we were passed.
-    if (!isStub) {
-      handlerInfo.preferredAction = storedHandlerInfo.action;
-      handlerInfo.alwaysAskBeforeHandling = !!storedHandlerInfo.ask;
-    } else {
-      // If we've got a stub, ensure the defaults are still set:
-      gExternalProtocolService.setProtocolHandlerDefaults(
-        handlerInfo,
-        handlerInfo.hasDefaultHandler
-      );
-      if (
-        handlerInfo.preferredAction == Ci.nsIHandlerInfo.alwaysAsk &&
-        handlerInfo.alwaysAskBeforeHandling
-      ) {
-        // `store` will default to `useHelperApp` because `alwaysAsk` is
-        // not one of the 3 recognized options; for compatibility, do
-        // the same here.
-        handlerInfo.preferredAction = Ci.nsIHandlerInfo.useHelperApp;
+    handlerInfo.preferredAction = storedHandlerInfo.action;
+    handlerInfo.alwaysAskBeforeHandling = !!storedHandlerInfo.ask;
+
+    // If the first item is not null, it is also the preferred handler. Since
+    // we cannot modify the stored array, use a boolean to keep track of this.
+    let isFirstItem = true;
+    for (let handler of storedHandlerInfo.handlers || [null]) {
+      let handlerApp = this.handlerAppFromSerializable(handler || {});
+      if (isFirstItem) {
+        isFirstItem = false;
+        handlerInfo.preferredApplicationHandler = handlerApp;
+      }
+      if (handlerApp) {
+        handlerInfo.possibleApplicationHandlers.appendElement(handlerApp);
       }
     }
-    // If it *is* a stub, don't override alwaysAskBeforeHandling or the
-    // preferred actions. Instead, just append the stored handlers, without
-    // overriding the preferred app, and then schedule a task to store proper
-    // info for this handler.
-    this._appendStoredHandlers(handlerInfo, storedHandlerInfo.handlers, isStub);
 
     if (this._isMIMEInfo(handlerInfo) && storedHandlerInfo.extensions) {
       for (let extension of storedHandlerInfo.extensions) {
@@ -471,34 +436,6 @@ HandlerService.prototype = {
     }
   },
 
-  /**
-   * Private method to inject stored handler information into an nsIHandlerInfo
-   * instance.
-   * @param handlerInfo           the nsIHandlerInfo instance to write to
-   * @param storedHandlers        the stored handlers
-   * @param keepPreferredApp      whether to keep the handlerInfo's
-   *                              preferredApplicationHandler or override it
-   *                              (default: false, ie override it)
-   */
-  _appendStoredHandlers(handlerInfo, storedHandlers, keepPreferredApp) {
-    // If the first item is not null, it is also the preferred handler. Since
-    // we cannot modify the stored array, use a boolean to keep track of this.
-    let isFirstItem = true;
-    for (let handler of storedHandlers || [null]) {
-      let handlerApp = this.handlerAppFromSerializable(handler || {});
-      if (isFirstItem) {
-        isFirstItem = false;
-        // Do not overwrite the preferred app unless that's allowed
-        if (!keepPreferredApp) {
-          handlerInfo.preferredApplicationHandler = handlerApp;
-        }
-      }
-      if (handlerApp) {
-        handlerInfo.possibleApplicationHandlers.appendElement(handlerApp);
-      }
-    }
-  },
-
   /**
    * @param handler
    *        A nsIHandlerApp handler app

uriloader/exthandler/tests/unit/test_defaults_handlerService.js

@@ -1,106 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-XPCOMUtils.defineLazyServiceGetter(
-  this,
-  "gExternalProtocolService",
-  "@mozilla.org/uriloader/external-protocol-service;1",
-  "nsIExternalProtocolService"
-);
-
-const kDefaultHandlerList = Services.prefs
-  .getChildList("gecko.handlerService.schemes")
-  .filter(p => {
-    try {
-      let val = Services.prefs.getComplexValue(p, Ci.nsIPrefLocalizedString)
-        .data;
-      return !!val;
-    } catch (ex) {
-      return false;
-    }
-  });
-
-add_task(async function test_check_defaults_get_added() {
-  let protocols = new Set(
-    kDefaultHandlerList.map(p => p.match(/schemes\.(\w+)/)[1])
-  );
-  for (let protocol of protocols) {
-    const kPrefStr = `schemes.${protocol}.`;
-    let matchingPrefs = kDefaultHandlerList.filter(p => p.includes(kPrefStr));
-    let protocolHandlerCount = matchingPrefs.length / 2;
-    Assert.ok(
-      protocolHandlerCount,
-      `Prefs for ${protocol} have at least 1 protocol handler`
-    );
-    Assert.ok(
-      gHandlerService.wrappedJSObject._store.data.schemes[protocol].stubEntry,
-      `Expect stub for ${protocol}`
-    );
-    let info = gExternalProtocolService.getProtocolHandlerInfo(protocol, {});
-    Assert.ok(
-      info,
-      `Should be able to get protocol handler info for ${protocol}`
-    );
-    let handlers = Array.from(
-      info.possibleApplicationHandlers.enumerate(Ci.nsIHandlerApp)
-    );
-    handlers = handlers.filter(h => h instanceof Ci.nsIWebHandlerApp);
-    Assert.equal(
-      handlers.length,
-      protocolHandlerCount,
-      `Default web handlers for ${protocol} should match`
-    );
-    let { alwaysAskBeforeHandling, preferredAction } = info;
-    // Actually store something, pretending there was a change:
-    let infoToWrite = gExternalProtocolService.getProtocolHandlerInfo(
-      protocol,
-      {}
-    );
-    gHandlerService.store(infoToWrite);
-    ok(
-      !gHandlerService.wrappedJSObject._store.data.schemes[protocol].stubEntry,
-      "Expect stub entry info to go away"
-    );
-
-    let newInfo = gExternalProtocolService.getProtocolHandlerInfo(protocol, {});
-    Assert.equal(
-      alwaysAskBeforeHandling,
-      newInfo.alwaysAskBeforeHandling,
-      protocol + " - always ask shouldn't change"
-    );
-    Assert.equal(
-      preferredAction,
-      newInfo.preferredAction,
-      protocol + " - preferred action shouldn't change"
-    );
-    await deleteHandlerStore();
-  }
-});
-
-add_task(async function test_check_default_modification() {
-  let mailtoHandlerCount =
-    kDefaultHandlerList.filter(p => p.includes("mailto")).length / 2;
-  Assert.ok(mailtoHandlerCount, "Prefs have at least 1 mailto handler");
-  Assert.ok(
-    true,
-    JSON.stringify(gHandlerService.wrappedJSObject._store.data.schemes.mailto)
-  );
-  Assert.ok(
-    gHandlerService.wrappedJSObject._store.data.schemes.mailto.stubEntry,
-    "Expect stub for mailto"
-  );
-  let mailInfo = gExternalProtocolService.getProtocolHandlerInfo("mailto", {});
-  mailInfo.alwaysAskBeforeHandling = false;
-  mailInfo.preferredAction = Ci.nsIHandlerInfo.useSystemDefault;
-  gHandlerService.store(mailInfo);
-  Assert.ok(
-    !gHandlerService.wrappedJSObject._store.data.schemes.mailto.stubEntry,
-    "Stub entry should be removed immediately."
-  );
-  let newMail = gExternalProtocolService.getProtocolHandlerInfo("mailto", {});
-  Assert.equal(newMail.preferredAction, Ci.nsIHandlerInfo.useSystemDefault);
-  Assert.equal(newMail.alwaysAskBeforeHandling, false);
-  await deleteHandlerStore();
-});

uriloader/exthandler/tests/unit/xpcshell.ini

@@ -3,7 +3,6 @@ head = head.js
 run-sequentially = Bug 912235 - Intermittent failures
 firefox-appdir = browser
 
-[test_defaults_handlerService.js]
 [test_getMIMEInfo_unknown_mime_type.js]
 run-if = os == "win" # Windows only test
 [test_getTypeFromExtension_ext_to_type_mapping.js]