Bug 1353029 - Pass PdfJs.enabled into child on change r=bdahl
authorDoug Thayer <dothayer@mozilla.com>
Tue, 30 May 2017 16:34:53 -0700
changeset 361714 d2e141768c61
parent 361713 bc5283bd5f85
child 361715 7a404b72b9b8
push id31939
push usercbook@mozilla.com
push dateThu, 01 Jun 2017 11:49:28 +0000
treeherdermozilla-central@d96110d76619 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbdahl
bugs1353029
milestone55.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 1353029 - Pass PdfJs.enabled into child on change r=bdahl isDefaultHandler in PdfJs.jsm appears to only be called on startup and when the settings for pdfs (either the pref or the setting in about:preferences) are changed. During startup, it's only the parent process which makes this call, which it uses to conditionally load a script in the content process. On change, the parent process controls notifying the content process, so it can simply pass along the enabled boolean. This change simply shifts to pass this boolean along to the child, and adds some guards to assert that we're only checking the actual values in the parent process. MozReview-Commit-ID: 9JSEJqHR2Ni
browser/extensions/pdfjs/content/PdfJs.jsm
browser/extensions/pdfjs/content/PdfjsChromeUtils.jsm
browser/extensions/pdfjs/content/PdfjsContentUtils.jsm
--- a/browser/extensions/pdfjs/content/PdfJs.jsm
+++ b/browser/extensions/pdfjs/content/PdfJs.jsm
@@ -62,20 +62,20 @@ function getIntPref(aPref, aDefaultValue
   try {
     return Services.prefs.getIntPref(aPref);
   } catch (ex) {
     return aDefaultValue;
   }
 }
 
 function isDefaultHandler() {
- if (Services.appinfo.processType === Services.appinfo.PROCESS_TYPE_CONTENT) {
-   return PdfjsContentUtils.isDefaultHandlerApp();
- }
- return PdfjsChromeUtils.isDefaultHandlerApp();
+  if (Services.appinfo.processType !== Services.appinfo.PROCESS_TYPE_DEFAULT) {
+    throw new Error("isDefaultHandler should only get called in the parent process.");
+  }
+  return PdfjsChromeUtils.isDefaultHandlerApp();
 }
 
 function initializeDefaultPreferences() {
   var DEFAULT_PREFERENCES =
 {
   "showPreviousViewOnLoad": true,
   "defaultZoomValue": "",
   "sidebarViewOnLoad": 0,
@@ -264,23 +264,24 @@ var PdfJs = {
     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();
-    if (Services.appinfo.processType ===
-        Services.appinfo.PROCESS_TYPE_DEFAULT) {
-      let jsm = "resource://pdf.js/PdfjsChromeUtils.jsm";
-      let PdfjsChromeUtils = Components.utils.import(jsm, {}).PdfjsChromeUtils;
-      PdfjsChromeUtils.notifyChildOfSettingsChange();
-    }
+    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() {
--- a/browser/extensions/pdfjs/content/PdfjsChromeUtils.jsm
+++ b/browser/extensions/pdfjs/content/PdfjsChromeUtils.jsm
@@ -119,25 +119,25 @@ var PdfjsChromeUtils = {
   },
 
   /*
    * Called by the main module when preference changes are picked up
    * in the parent process. Observers don't propagate so we need to
    * instruct the child to refresh its configuration and (possibly)
    * the module's registration.
    */
-  notifyChildOfSettingsChange() {
+  notifyChildOfSettingsChange(enabled) {
     if (Services.appinfo.processType ===
         Services.appinfo.PROCESS_TYPE_DEFAULT && this._ppmm) {
       // XXX kinda bad, we want to get the parent process mm associated
       // with the content process. _ppmm is currently the global process
       // manager, which means this is going to fire to every child process
       // we have open. Unfortunately I can't find a way to get at that
       // process specific mm from js.
-      this._ppmm.broadcastAsyncMessage("PDFJS:Child:refreshSettings", {});
+      this._ppmm.broadcastAsyncMessage("PDFJS:Child:updateSettings", {enabled});
     }
   },
 
   /*
    * Events
    */
 
   observe(aSubject, aTopic, aData) {
--- a/browser/extensions/pdfjs/content/PdfjsContentUtils.jsm
+++ b/browser/extensions/pdfjs/content/PdfjsContentUtils.jsm
@@ -38,25 +38,25 @@ var PdfjsContentUtils = {
   },
 
   init() {
     // child *process* mm, or when loaded into the parent for in-content
     // support the psuedo child process mm 'child PPMM'.
     if (!this._mm) {
       this._mm = Cc["@mozilla.org/childprocessmessagemanager;1"].
         getService(Ci.nsISyncMessageSender);
-      this._mm.addMessageListener("PDFJS:Child:refreshSettings", this);
+      this._mm.addMessageListener("PDFJS:Child:updateSettings", this);
 
       Services.obs.addObserver(this, "quit-application");
     }
   },
 
   uninit() {
     if (this._mm) {
-      this._mm.removeMessageListener("PDFJS:Child:refreshSettings", this);
+      this._mm.removeMessageListener("PDFJS:Child:updateSettings", this);
       Services.obs.removeObserver(this, "quit-application");
     }
     this._mm = null;
   },
 
   /*
    * prefs utilities - the child does not have write access to prefs.
    * note, the pref names here are cross-checked against a list of
@@ -93,24 +93,16 @@ var PdfjsContentUtils = {
   setStringPref(aPrefName, aPrefValue) {
     this._mm.sendSyncMessage("PDFJS:Parent:setStringPref", {
       name: aPrefName,
       value: aPrefValue
     });
   },
 
   /*
-   * Forwards default app query to the parent where we check various
-   * handler app settings only available in the parent process.
-   */
-  isDefaultHandlerApp() {
-    return this._mm.sendSyncMessage("PDFJS:Parent:isDefaultHandlerApp")[0];
-  },
-
-  /*
    * Request the display of a notification warning in the associated window
    * when the renderer isn't sure a pdf displayed correctly.
    */
   displayWarning(aWindow, aMessage, aLabel, aAccessKey) {
     // the child's dom frame mm associated with the window.
     let winmm = aWindow.QueryInterface(Ci.nsIInterfaceRequestor)
                        .getInterface(Ci.nsIDocShell)
                        .QueryInterface(Ci.nsIInterfaceRequestor)
@@ -129,21 +121,25 @@ var PdfjsContentUtils = {
   observe(aSubject, aTopic, aData) {
     if (aTopic === "quit-application") {
       this.uninit();
     }
   },
 
   receiveMessage(aMsg) {
     switch (aMsg.name) {
-      case "PDFJS:Child:refreshSettings":
+      case "PDFJS:Child:updateSettings":
         // Only react to this if we are remote.
         if (Services.appinfo.processType ===
             Services.appinfo.PROCESS_TYPE_CONTENT) {
           let jsm = "resource://pdf.js/PdfJs.jsm";
           let pdfjs = Components.utils.import(jsm, {}).PdfJs;
-          pdfjs.updateRegistration();
+          if (aMsg.data.enabled) {
+            pdfjs.ensureRegistered();
+          } else {
+            pdfjs.ensureUnregistered();
+          }
         }
         break;
     }
   }
 };