Bug 1389443 - Load handlers.json asynchronously r?Paolo draft
authorDoug Thayer <dothayer@mozilla.com>
Tue, 15 Aug 2017 14:57:32 -0700
changeset 651414 83f2f6e7f00bf54534bb7ee48c57a93633f8ed0c
parent 645833 df9beb781895fcd0493c21e95ad313e0044515ec
child 727709 bbbfd5508a18b09a31f0701240d92abf1a18e1ae
push id75723
push userbmo:dothayer@mozilla.com
push dateWed, 23 Aug 2017 17:58:58 +0000
reviewersPaolo
bugs1389443
milestone57.0a1
Bug 1389443 - Load handlers.json asynchronously r?Paolo Asynchronously load handlers.json for nsHandlerService-json.js in order to avoid blocking IO early on. MozReview-Commit-ID: F3THSxvXR7I
browser/components/nsBrowserGlue.js
browser/extensions/pdfjs/content/PdfJs.jsm
uriloader/exthandler/ContentHandlerService.cpp
uriloader/exthandler/nsHandlerService-json.js
uriloader/exthandler/nsHandlerService.js
uriloader/exthandler/nsIHandlerService.idl
uriloader/exthandler/tests/unit/test_handlerService_json.js
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -478,16 +478,26 @@ BrowserGlue.prototype = {
         });
         break;
       case "test-initialize-sanitizer":
         this._sanitizer.onStartup();
         break;
       case "sync-ui-state:update":
         this._updateFxaBadges();
         break;
+      case "handlersvc-store-initialized":
+        // Initialize PdfJs when running in-process and remote. This only
+        // happens once since PdfJs registers global hooks. If the PdfJs
+        // extension is installed the init method below will be overridden
+        // leaving initialization to the extension.
+        // 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(true);
+        break;
     }
   },
 
   // initialization (called on application startup)
   _init: function BG__init() {
     let os = Services.obs;
     os.addObserver(this, "notifications-open-settings");
     os.addObserver(this, "prefservice:after-app-defaults");
@@ -513,16 +523,17 @@ BrowserGlue.prototype = {
     os.addObserver(this, "handle-xul-text-link");
     os.addObserver(this, "profile-before-change");
     os.addObserver(this, "keyword-search");
     os.addObserver(this, "browser-search-engine-modified");
     os.addObserver(this, "restart-in-safe-mode");
     os.addObserver(this, "flash-plugin-hang");
     os.addObserver(this, "xpi-signature-changed");
     os.addObserver(this, "sync-ui-state:update");
+    os.addObserver(this, "handlersvc-store-initialized");
 
     this._flashHangCount = 0;
     this._firstWindowReady = new Promise(resolve => this._firstWindowLoaded = resolve);
 
     if (AppConstants.platform == "macosx" ||
         (AppConstants.platform == "win" && AppConstants.RELEASE_OR_BETA)) {
       // Handles prompting to inform about incompatibilites when accessibility
       // and e10s are active together.
@@ -852,30 +863,22 @@ BrowserGlue.prototype = {
     let scaling = aWindow.devicePixelRatio * 100;
     try {
       Services.telemetry.getHistogramById("DISPLAY_SCALING").add(scaling);
     } catch (ex) {}
   },
 
   // the first browser window has finished initializing
   _onFirstWindowLoaded: function BG__onFirstWindowLoaded(aWindow) {
-    // Initialize PdfJs when running in-process and remote. This only
-    // happens once since PdfJs registers global hooks. If the PdfJs
-    // extension is installed the init method below will be overridden
-    // leaving initialization to the extension.
-    // 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(true);
-    // child only: similar to the call above for parent - register content
-    // handler and init message manager child shim for privileged api access.
-    // With older versions of the extension installed, this load will fail
-    // passively.
+    // Set up listeners and, if PdfJs is enabled, register the PDF stream converter.
+    // We delay all of the parent's initialization other than stream converter
+    // registration, because it requires file IO from nsHandlerService-json.js
     Services.ppmm.loadProcessScript("resource://pdf.js/pdfjschildbootstrap.js", true);
     if (PdfJs.enabled) {
+      PdfJs.ensureRegistered();
       Services.ppmm.loadProcessScript("resource://pdf.js/pdfjschildbootstrap-enabled.js", true);
     }
 
     TabCrashHandler.init();
     if (AppConstants.MOZ_CRASHREPORTER) {
       PluginCrashReporter.init();
     }
 
@@ -1146,16 +1149,22 @@ BrowserGlue.prototype = {
     });
 
     Services.tm.idleDispatchToMainThread(() => {
       let {setTimeout} = Cu.import("resource://gre/modules/Timer.jsm", {});
       setTimeout(function() {
         Services.tm.idleDispatchToMainThread(Services.startup.trackStartupCrashEnd);
       }, STARTUP_CRASHES_END_DELAY_MS);
     });
+
+    Services.tm.idleDispatchToMainThread(() => {
+      let handlerService = Cc["@mozilla.org/uriloader/handler-service;1"].
+                           getService(Ci.nsIHandlerService);
+      handlerService.asyncInit();
+    });
   },
 
   /**
    * Use this function as an entry point to schedule tasks that need
    * to run once per session, at any arbitrary point in time.
    * This function will be called from an idle observer. Check the value of
    * LATE_TASKS_IDLE_TIME_SEC to see the current value for this idle
    * observer.
--- a/browser/extensions/pdfjs/content/PdfJs.jsm
+++ b/browser/extensions/pdfjs/content/PdfJs.jsm
@@ -25,16 +25,20 @@ const Cu = Components.utils;
 
 const PREF_PREFIX = "pdfjs";
 const PREF_DISABLED = PREF_PREFIX + ".disabled";
 const PREF_MIGRATION_VERSION = PREF_PREFIX + ".migrationVersion";
 const PREF_PREVIOUS_ACTION = PREF_PREFIX + ".previousHandler.preferredAction";
 const PREF_PREVIOUS_ASK = PREF_PREFIX +
                           ".previousHandler.alwaysAskBeforeHandling";
 const PREF_DISABLED_PLUGIN_TYPES = "plugin.disable_full_page_plugin_for_types";
+const PREF_ENABLED_CACHE_STATE = PREF_PREFIX + ".enabledCache.state";
+const PREF_ENABLED_CACHE_INITIALIZED = PREF_PREFIX +
+                                       ".enabledCache.initialized";
+const PREF_APP_UPDATE_POSTUPDATE = "app.update.postupdate";
 const TOPIC_PDFJS_HANDLER_CHANGED = "pdfjs:handlerChanged";
 const TOPIC_PLUGINS_LIST_UPDATED = "plugins-list-updated";
 const TOPIC_PLUGIN_INFO_UPDATED = "plugin-info-updated";
 const PDF_CONTENT_TYPE = "application/pdf";
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 
@@ -185,17 +189,17 @@ var PdfJs = {
     Services.obs.addObserver(this, TOPIC_PDFJS_HANDLER_CHANGED);
     Services.obs.addObserver(this, TOPIC_PLUGINS_LIST_UPDATED);
     Services.obs.addObserver(this, TOPIC_PLUGIN_INFO_UPDATED);
 
     initializeDefaultPreferences();
   },
 
   updateRegistration: function updateRegistration() {
-    if (this.enabled) {
+    if (this.checkEnabled()) {
       this.ensureRegistered();
     } else {
       this.ensureUnregistered();
     }
   },
 
   uninit: function uninit() {
     if (this._initialized) {
@@ -264,36 +268,17 @@ var PdfJs = {
     // Update the category manager in case the plugins are already loaded.
     let categoryManager = Cc["@mozilla.org/categorymanager;1"];
     categoryManager.getService(Ci.nsICategoryManager).
                     deleteCategoryEntry("Gecko-Content-Viewers",
                                         PDF_CONTENT_TYPE,
                                         false);
   },
 
-  // nsIObserver
-  observe: function observe(aSubject, aTopic, aData) {
-    if (Services.appinfo.processType !==
-        Services.appinfo.PROCESS_TYPE_DEFAULT) {
-      throw new Error("Only the parent process should be observing PDF " +
-                      "handler changes.");
-    }
-
-    this.updateRegistration();
-    let jsm = "resource://pdf.js/PdfjsChromeUtils.jsm";
-    let PdfjsChromeUtils = Components.utils.import(jsm, {}).PdfjsChromeUtils;
-    PdfjsChromeUtils.notifyChildOfSettingsChange(this.enabled);
-  },
-
-  /**
-   * pdf.js is only enabled if it is both selected as the pdf viewer and if the
-   * global switch enabling it is true.
-   * @return {boolean} Whether or not it's enabled.
-   */
-  get enabled() {
+  _isEnabled: function _isEnabled() {
     var disabled = getBoolPref(PREF_DISABLED, true);
     if (disabled) {
       return false;
     }
 
     // Check if the 'application/pdf' preview handler is configured properly.
     if (!isDefaultHandler()) {
       return false;
@@ -323,16 +308,57 @@ var PdfJs = {
         return mimeType === PDF_CONTENT_TYPE;
       });
     });
 
     // Use pdf.js if pdf plugin is not present or disabled
     return !enabledPluginFound;
   },
 
+  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);
+    return isEnabled;
+  },
+
+  // nsIObserver
+  observe: function observe(aSubject, aTopic, aData) {
+    if (Services.appinfo.processType !==
+        Services.appinfo.PROCESS_TYPE_DEFAULT) {
+      throw new Error("Only the parent process should be observing PDF " +
+                      "handler changes.");
+    }
+
+    this.updateRegistration();
+    let jsm = "resource://pdf.js/PdfjsChromeUtils.jsm";
+    let PdfjsChromeUtils = Components.utils.import(jsm, {}).PdfjsChromeUtils;
+    PdfjsChromeUtils.notifyChildOfSettingsChange(this.enabled);
+  },
+
+  /**
+   * pdf.js is only enabled if it is both selected as the pdf viewer and if the
+   * global switch enabling it is true.
+   * @return {boolean} Whether or not it's enabled.
+   */
+  get enabled() {
+    if (!Services.prefs.getBoolPref(PREF_ENABLED_CACHE_INITIALIZED, false)) {
+      // If we just updated, and the cache hasn't been initialized, then we
+      // can't assume a default state, and need to synchronously initialize
+      // PdfJs
+      if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_POSTUPDATE)) {
+        this.checkEnabled();
+      }
+
+      Services.prefs.setBoolPref(PREF_ENABLED_CACHE_INITIALIZED, true);
+    }
+    return Services.prefs.getBoolPref(PREF_ENABLED_CACHE_STATE, true);
+  },
+
   ensureRegistered: function ensureRegistered() {
     if (this._registered) {
       return;
     }
     this._pdfStreamConverterFactory = new Factory();
     Cu.import("resource://pdf.js/PdfStreamConverter.jsm");
     this._pdfStreamConverterFactory.register(PdfStreamConverter);
 
@@ -345,9 +371,8 @@ var PdfJs = {
     }
     this._pdfStreamConverterFactory.unregister();
     Cu.unload("resource://pdf.js/PdfStreamConverter.jsm");
     delete this._pdfStreamConverterFactory;
 
     this._registered = false;
   },
 };
-
--- a/uriloader/exthandler/ContentHandlerService.cpp
+++ b/uriloader/exthandler/ContentHandlerService.cpp
@@ -112,16 +112,21 @@ static inline void CopyHanderInfoTonsIHa
   nsCOMPtr<nsIMutableArray> possibleHandlers;
   aHandlerInfo->GetPossibleApplicationHandlers(getter_AddRefs(possibleHandlers));
   possibleHandlers->AppendElement(preferredApp, false);
 }
 ContentHandlerService::~ContentHandlerService()
 {
 }
 
+NS_IMETHODIMP ContentHandlerService::AsyncInit()
+{
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
 NS_IMETHODIMP ContentHandlerService::Enumerate(nsISimpleEnumerator * *_retval)
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP ContentHandlerService::FillHandlerInfo(nsIHandlerInfo *aHandlerInfo, const nsACString & aOverrideType)
 {
   HandlerInfo info;
--- a/uriloader/exthandler/nsHandlerService-json.js
+++ b/uriloader/exthandler/nsHandlerService-json.js
@@ -39,24 +39,37 @@ HandlerService.prototype = {
 
   __store: null,
   get _store() {
     if (!this.__store) {
       this.__store = new JSONFile({
         path: OS.Path.join(OS.Constants.Path.profileDir, "handlers.json"),
         dataPostProcessor: this._dataPostProcessor.bind(this),
       });
+    }
+
+    // Always call this even if this.__store was set, since it may have been
+    // set by asyncInit, which might not have completed yet.
+    this._ensureStoreInitialized();
+    return this.__store;
+  },
+
+  __storeInitialized: false,
+  _ensureStoreInitialized() {
+    if (!this.__storeInitialized) {
+      this.__storeInitialized = true;
       this.__store.ensureDataReady();
 
       // We have to inject new default protocol handlers only if we haven't
       // already done this when migrating data from the RDF back-end.
       let alreadyInjected = this._migrateFromRDFIfNeeded();
       this._injectDefaultProtocolHandlersIfNeeded(alreadyInjected);
+
+      Services.obs.notifyObservers(null, "handlersvc-store-initialized");
     }
-    return this.__store;
   },
 
   _dataPostProcessor(data) {
     return data.defaultHandlersVersion ? data : {
       defaultHandlersVersion: {},
       mimeTypes: {},
       schemes: {},
     };
@@ -213,31 +226,48 @@ HandlerService.prototype = {
   },
 
   _onDBChange() {
     return (async () => {
       if (this.__store) {
         await this.__store.finalize();
       }
       this.__store = null;
+      this.__storeInitialized = false;
     })().catch(Cu.reportError);
   },
 
   // nsIObserver
   observe(subject, topic, data) {
     if (topic != "handlersvc-json-replace") {
       return;
     }
     let promise = this._onDBChange();
     promise.then(() => {
       Services.obs.notifyObservers(null, "handlersvc-json-replace-complete");
     });
   },
 
   // nsIHandlerService
+  asyncInit() {
+    if (!this.__store) {
+      this.__store = new JSONFile({
+        path: OS.Path.join(OS.Constants.Path.profileDir, "handlers.json"),
+        dataPostProcessor: this._dataPostProcessor.bind(this),
+      });
+      this.__store.load().then(() => {
+        // __store can be null if we called _onDBChange in the mean time.
+        if (this.__store) {
+          this._ensureStoreInitialized();
+        }
+      }).catch(Cu.reportError);
+    }
+  },
+
+  // nsIHandlerService
   enumerate() {
     let handlers = Cc["@mozilla.org/array;1"]
                      .createInstance(Ci.nsIMutableArray);
     for (let type of Object.keys(this._store.data.mimeTypes)) {
       let handler = gMIMEService.getFromTypeAndExtension(type, null);
       handlers.appendElement(handler);
     }
     for (let type of Object.keys(this._store.data.schemes)) {
--- a/uriloader/exthandler/nsHandlerService.js
+++ b/uriloader/exthandler/nsHandlerService.js
@@ -297,16 +297,19 @@ HandlerService.prototype = {
         this._observerSvc.notifyObservers(null, "handlersvc-rdf-replace-complete");
         break;
     }
   },
 
 
   //**************************************************************************//
   // nsIHandlerService
+  asyncInit: function HS_asyncInit() {
+    // noop
+  },
 
   enumerate: function HS_enumerate() {
     var handlers = Cc["@mozilla.org/array;1"].
                    createInstance(Ci.nsIMutableArray);
     this._appendHandlers(handlers, CLASS_MIMEINFO);
     this._appendHandlers(handlers, CLASS_PROTOCOLINFO);
     return handlers.enumerate();
   },
--- a/uriloader/exthandler/nsIHandlerService.idl
+++ b/uriloader/exthandler/nsIHandlerService.idl
@@ -6,16 +6,22 @@
 
 interface nsIHandlerInfo;
 interface nsISimpleEnumerator;
 
 [scriptable, uuid(53f0ad17-ec62-46a1-adbc-efccc06babcd)]
 interface nsIHandlerService : nsISupports
 {
   /**
+   * Asynchronously performs any IO that the nsIHandlerService needs to do
+   * before it can be of use.
+   */
+  void asyncInit();
+
+  /**
    * Retrieve a list of all handlers in the datastore.  This list is not
    * guaranteed to be in any particular order, and callers should not assume
    * it will remain in the same order in the future.
    *
    * @returns a list of all handlers in the datastore
    */
   nsISimpleEnumerator enumerate();
 
--- a/uriloader/exthandler/tests/unit/test_handlerService_json.js
+++ b/uriloader/exthandler/tests/unit/test_handlerService_json.js
@@ -28,16 +28,54 @@ add_task(async function test_store_keeps
 
   await unloadHandlerStore();
   let data = JSON.parse(new TextDecoder().decode(await OS.File.read(jsonPath)));
   do_check_eq(data.mimeTypes["example/type.handleinternally"].unknownProperty,
               "preserved");
 });
 
 /**
+ * Runs the asyncInit method, ensuring that it successfully inits the store
+ * and calls the handlersvc-store-initialized topic.
+ */
+add_task(async function test_async_init() {
+  await deleteHandlerStore();
+  await copyTestDataToHandlerStore();
+  gHandlerService.asyncInit();
+  await TestUtils.topicObserved("handlersvc-store-initialized");
+  await assertAllHandlerInfosMatchTestData();
+
+  await unloadHandlerStore();
+});
+
+/**
+ * Races the asyncInit method against the sync init (implicit in enumerate),
+ * to ensure that the store will be synchronously initialized without any
+ * ill effects.
+ */
+add_task(async function test_race_async_init() {
+  await deleteHandlerStore();
+  await copyTestDataToHandlerStore();
+  let storeInitialized = false;
+  // Pass a callback to synchronously observe the topic, as a promise would
+  // resolve asynchronously
+  TestUtils.topicObserved("handlersvc-store-initialized", () => {
+    storeInitialized = true;
+    return true;
+  });
+  gHandlerService.asyncInit();
+  do_check_false(storeInitialized);
+  gHandlerService.enumerate();
+  do_check_true(storeInitialized);
+  await assertAllHandlerInfosMatchTestData();
+
+  await unloadHandlerStore();
+});
+
+/**
  * Tests the migration from an existing RDF data source.
  */
 add_task(async function test_migration_rdf_present() {
   // Perform the most common migration, with the JSON file missing.
   await deleteHandlerStore();
   await copyTestDataToHandlerStoreRDF();
   Services.prefs.setBoolPref("gecko.handlerService.migrated", false);
   await assertAllHandlerInfosMatchTestData();