Bug 1633790 - allow PDF.js use when we've misled the user into misconfiguring PDF handlers, r=jaws,mattwoodrow
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 06 May 2020 20:28:36 +0000
changeset 528515 979a77b1cbf396e2442e8ff9e345fa2d459e94eb
parent 528514 418a69055fc5583ff82416ab707f5ee917c55bab
child 528516 ae1a72a9dce69a7fd11cf79f270002659a6d9838
push id37388
push usercsabou@mozilla.com
push dateThu, 07 May 2020 04:06:39 +0000
treeherdermozilla-central@98040184b6c0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws, mattwoodrow
bugs1633790
milestone78.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 1633790 - allow PDF.js use when we've misled the user into misconfiguring PDF handlers, r=jaws,mattwoodrow Prior to this patch, PDF.js tracks both its own 'disabled' pref (which is used by enterprise policy) and whether it is the default handler per the handler service - but it tracks both in one bool, which determines whether its streamconverter registers. Really, what we want is to never use PDF.js if it's preffed off. However, if there is some other default, it should be acceptable to use PDF.js in some circumstances, like for <embed> or <object>s where otherwise we would show no content at all. Even for toplevel PDFs, if the user has configured Firefox to open PDFs in an external helper app which is Firefox (which is currently an easy mistake to make in the unknownContentType dialog), or has it set to the OS default, but has changed their OS default to Firefox, we really still want to open those PDFs with PDF.js. This patch fixes all of this by splitting out the pref tracking from the handler state tracking. Only the pref will completely disable PDF.js. Then, in the streamconverter code, we check whether PDF.js should be used for PDFs, and if there's a misconfiguration that we can correct. This code is invoked from the parent process when we load PDFs in frames or toplevel documents, and will prevent us from invoking PDF.js in the child if the user would prefer that not to happen. As a driveby, this cleans up how we track the pref inside PDF.js, and how we get notified of changes to the handler - we were missing changes made in the unknown content type dialog, so it seemed worth making it generic. Differential Revision: https://phabricator.services.mozilla.com/D73510
browser/components/BrowserGlue.jsm
browser/components/preferences/main.js
browser/extensions/pdfjs/content/PdfJs.jsm
browser/extensions/pdfjs/content/PdfStreamConverter.jsm
browser/extensions/pdfjs/pdfjs.js
browser/extensions/pdfjs/test/browser_pdfjs_savedialog.js
uriloader/exthandler/HandlerService.js
--- a/browser/components/BrowserGlue.jsm
+++ b/browser/components/BrowserGlue.jsm
@@ -57,17 +57,17 @@ ChromeUtils.defineModuleGetter(
 
 XPCOMUtils.defineLazyServiceGetter(
   this,
   "PushService",
   "@mozilla.org/push/Service;1",
   "nsIPushService"
 );
 
-const PREF_PDFJS_ENABLED_CACHE_STATE = "pdfjs.enabledCache.state";
+const PREF_PDFJS_ISDEFAULT_CACHE_STATE = "pdfjs.enabledCache.state";
 
 /**
  * Fission-compatible JSWindowActor implementations.
  * Detailed documentation of these is in dom/docs/Fission.rst,
  * available at https://firefox-source-docs.mozilla.org/dom/Fission.html#jswindowactor
  */
 let ACTORS = {
   AboutLogins: {
@@ -1277,23 +1277,18 @@ BrowserGlue.prototype = {
     }
 
     // apply distribution customizations
     this._distributionCustomizer.applyCustomizations();
 
     // 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);
+    if (!Services.prefs.prefHasUserValue(PREF_PDFJS_ISDEFAULT_CACHE_STATE)) {
+      PdfJs.checkIsDefault(this._isNewProfile);
     }
 
     listeners.init();
 
     SessionStore.init();
 
     AddonManager.maybeInstallBuiltinAddon(
       "firefox-compact-light@mozilla.org",
--- a/browser/components/preferences/main.js
+++ b/browser/components/preferences/main.js
@@ -44,17 +44,16 @@ XPCOMUtils.defineLazyServiceGetters(this
   ],
   gMIMEService: ["@mozilla.org/mime;1", "nsIMIMEService"],
 });
 
 // Constants & Enumeration Values
 const TYPE_PDF = "application/pdf";
 
 const PREF_PDFJS_DISABLED = "pdfjs.disabled";
-const TOPIC_PDFJS_HANDLER_CHANGED = "pdfjs:handlerChanged";
 
 const PREF_DISABLED_PLUGIN_TYPES = "plugin.disable_full_page_plugin_for_types";
 
 // Pref for when containers is being controlled
 const PREF_CONTAINERS_EXTENSION = "privacy.userContext.extension";
 
 // Preferences that affect which entries to show in the list.
 const PREF_SHOW_PLUGINS_IN_LIST = "browser.download.show_plugins_in_list";
@@ -3643,37 +3642,32 @@ class InternalHandlerInfoWrapper extends
   constructor(mimeType) {
     super(mimeType, gMIMEService.getFromTypeAndExtension(mimeType, null));
   }
 
   // Override store so we so we can notify any code listening for registration
   // or unregistration of this handler.
   store() {
     super.store();
-    Services.obs.notifyObservers(null, this._handlerChanged);
   }
 
   get enabled() {
     throw Components.Exception("", Cr.NS_ERROR_NOT_IMPLEMENTED);
   }
 
   get description() {
     return { id: this._appPrefLabel };
   }
 }
 
 class PDFHandlerInfoWrapper extends InternalHandlerInfoWrapper {
   constructor() {
     super(TYPE_PDF);
   }
 
-  get _handlerChanged() {
-    return TOPIC_PDFJS_HANDLER_CHANGED;
-  }
-
   get _appPrefLabel() {
     return "applications-type-pdf";
   }
 
   get enabled() {
     return !Services.prefs.getBoolPref(PREF_PDFJS_DISABLED);
   }
 }
--- a/browser/extensions/pdfjs/content/PdfJs.jsm
+++ b/browser/extensions/pdfjs/content/PdfJs.jsm
@@ -18,17 +18,17 @@
 var EXPORTED_SYMBOLS = ["PdfJs"];
 
 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_ENABLED_CACHE_STATE = PREF_PREFIX + ".enabledCache.state";
+const PREF_ISDEFAULT_CACHE_STATE = PREF_PREFIX + ".enabledCache.state";
 const TOPIC_PDFJS_HANDLER_CHANGED = "pdfjs:handlerChanged";
 const PDF_CONTENT_TYPE = "application/pdf";
 
 const { XPCOMUtils } = ChromeUtils.import(
   "resource://gre/modules/XPCOMUtils.jsm"
 );
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
@@ -93,66 +93,51 @@ const gPdfFakeHandlerInfo = {
   alwaysAskBeforeHandling: false,
   preferredAction: Ci.nsIHandlerInfo.handleInternally,
   type: PDF_CONTENT_TYPE,
 };
 
 var PdfJs = {
   QueryInterface: ChromeUtils.generateQI([Ci.nsIObserver]),
   _initialized: false,
+  _cachedIsDefault: true,
 
   init: function init(isNewProfile) {
     if (
       Services.appinfo.processType !== Services.appinfo.PROCESS_TYPE_DEFAULT
     ) {
       throw new Error(
         "PdfJs.init should only get called in the parent process."
       );
     }
     PdfjsChromeUtils.init();
     this.initPrefs();
-
-    Services.ppmm.sharedData.set(
-      "pdfjs.enabled",
-      this.checkEnabled(isNewProfile)
-    );
-  },
-
-  earlyInit(isNewProfile) {
-    // Note: Please keep this in sync with the duplicated logic in
-    // BrowserGlue.jsm.
-    Services.ppmm.sharedData.set(
-      "pdfjs.enabled",
-      this.checkEnabled(isNewProfile)
-    );
+    this.checkIsDefault(isNewProfile);
   },
 
   initPrefs: function initPrefs() {
     if (this._initialized) {
       return;
     }
     this._initialized = true;
 
-    if (Services.prefs.getBoolPref(PREF_DISABLED, false)) {
+    if (this._prefDisabled) {
       this._unbecomeHandler();
     } else {
       this._migrate();
     }
 
-    // Listen for when pdf.js is completely disabled or a different pdf handler
-    // is chosen.
-    Services.prefs.addObserver(PREF_DISABLED, this);
+    // Listen for when a different pdf handler is chosen.
     Services.obs.addObserver(this, TOPIC_PDFJS_HANDLER_CHANGED);
 
     initializeDefaultPreferences();
   },
 
   uninit: function uninit() {
     if (this._initialized) {
-      Services.prefs.removeObserver(PREF_DISABLED, this);
       Services.obs.removeObserver(this, TOPIC_PDFJS_HANDLER_CHANGED);
       this._initialized = false;
     }
   },
 
   _migrate: function migrate() {
     const VERSION = 2;
     var currentVersion = Services.prefs.getIntPref(PREF_MIGRATION_VERSION, 0);
@@ -227,53 +212,57 @@ 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) {
+  _isDefault(isNewProfile) {
     let { processType, PROCESS_TYPE_DEFAULT } = Services.appinfo;
     if (processType !== PROCESS_TYPE_DEFAULT) {
       throw new Error(
-        "isEnabled should only get called in the parent process."
+        "isDefault should only get called in the parent process."
       );
     }
 
-    if (Services.prefs.getBoolPref(PREF_DISABLED, true)) {
+    if (this._prefDisabled) {
       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
+      handlerInfo.preferredAction == Ci.nsIHandlerInfo.handleInternally
     );
   },
 
-  checkEnabled(isNewProfile) {
-    let isEnabled = this._isEnabled(isNewProfile);
-    // 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;
+  checkIsDefault(isNewProfile) {
+    this._cachedIsDefault = this._isDefault(isNewProfile);
+    Services.prefs.setBoolPref(
+      PREF_ISDEFAULT_CACHE_STATE,
+      this._cachedIsDefault
+    );
+  },
+
+  cachedIsDefault() {
+    return this._cachedIsDefault;
   },
 
   // 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."
-      );
-    }
-
-    Services.ppmm.sharedData.set("pdfjs.enabled", this.checkEnabled());
+  observe(aSubject, aTopic, aData) {
+    this.checkIsDefault();
   },
 };
+
+XPCOMUtils.defineLazyPreferenceGetter(
+  PdfJs,
+  "_prefDisabled",
+  PREF_DISABLED,
+  false,
+  () => PdfJs.checkIsDefault()
+);
--- a/browser/extensions/pdfjs/content/PdfStreamConverter.jsm
+++ b/browser/extensions/pdfjs/content/PdfStreamConverter.jsm
@@ -18,21 +18,25 @@
 var EXPORTED_SYMBOLS = ["PdfStreamConverter"];
 
 const PDFJS_EVENT_ID = "pdf.js.message";
 const PREF_PREFIX = "pdfjs";
 const PDF_VIEWER_ORIGIN = "resource://pdf.js";
 const PDF_VIEWER_WEB_PAGE = "resource://pdf.js/web/viewer.html";
 const MAX_NUMBER_OF_PREFS = 50;
 const MAX_STRING_PREF_LENGTH = 128;
+const PDF_CONTENT_TYPE = "application/pdf";
 
 const { XPCOMUtils } = ChromeUtils.import(
   "resource://gre/modules/XPCOMUtils.jsm"
 );
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
+const { AppConstants } = ChromeUtils.import(
+  "resource://gre/modules/AppConstants.jsm"
+);
 
 ChromeUtils.defineModuleGetter(
   this,
   "NetUtil",
   "resource://gre/modules/NetUtil.jsm"
 );
 
 ChromeUtils.defineModuleGetter(
@@ -53,26 +57,47 @@ ChromeUtils.defineModuleGetter(
   "resource://pdf.js/PdfJsTelemetry.jsm"
 );
 
 ChromeUtils.defineModuleGetter(
   this,
   "PdfjsContentUtils",
   "resource://pdf.js/PdfjsContentUtils.jsm"
 );
+ChromeUtils.defineModuleGetter(this, "PdfJs", "resource://pdf.js/PdfJs.jsm");
 
 XPCOMUtils.defineLazyGlobalGetters(this, ["XMLHttpRequest"]);
 
 var Svc = {};
 XPCOMUtils.defineLazyServiceGetter(
   Svc,
   "mime",
   "@mozilla.org/mime;1",
   "nsIMIMEService"
 );
+XPCOMUtils.defineLazyServiceGetter(
+  Svc,
+  "handlers",
+  "@mozilla.org/uriloader/handler-service;1",
+  "nsIHandlerService"
+);
+
+XPCOMUtils.defineLazyGetter(this, "gOurBinary", () => {
+  let file = Services.dirsvc.get("XREExeF", Ci.nsIFile);
+  // Make sure to get the .app on macOS
+  if (AppConstants.platform == "macosx") {
+    while (file) {
+      if (/\.app\/?$/i.test(file.leafName)) {
+        break;
+      }
+      file = file.parent;
+    }
+  }
+  return file;
+});
 
 function getBoolPref(pref, def) {
   try {
     return Services.prefs.getBoolPref(pref);
   } catch (ex) {
     return def;
   }
 }
@@ -321,17 +346,17 @@ class ChromeActions {
 
       var listener = {
         extListener: null,
         onStartRequest(aRequest) {
           var loadContext = self.domWindow.docShell.QueryInterface(
             Ci.nsILoadContext
           );
           this.extListener = extHelperAppSvc.doContent(
-            data.isAttachment ? "application/octet-stream" : "application/pdf",
+            data.isAttachment ? "application/octet-stream" : PDF_CONTENT_TYPE,
             aRequest,
             loadContext,
             false
           );
           this.extListener.onStartRequest(aRequest);
         },
         onStopRequest(aRequest, aStatusCode) {
           if (this.extListener) {
@@ -1005,17 +1030,96 @@ PdfStreamConverter.prototype = {
   },
 
   // nsIStreamConverter::asyncConvertData
   asyncConvertData(aFromType, aToType, aListener, aCtxt) {
     // Store the listener passed to us
     this.listener = aListener;
   },
 
+  _usableHandler(handlerInfo) {
+    let { preferredApplicationHandler } = handlerInfo;
+    if (
+      !preferredApplicationHandler ||
+      !(preferredApplicationHandler instanceof Ci.nsILocalHandlerApp)
+    ) {
+      return false;
+    }
+    preferredApplicationHandler.QueryInterface(Ci.nsILocalHandlerApp);
+    // We have an app, grab the executable
+    let { executable } = preferredApplicationHandler;
+    if (!executable) {
+      return false;
+    }
+    return !executable.equals(gOurBinary);
+  },
+
+  /*
+   * Check if the user wants to use PDF.js. Returns true if PDF.js should
+   * handle PDFs, and false if not. Will always return true on non-parent
+   * processes.
+   *
+   * If the user has selected to open PDFs with a helper app, and we are that
+   * helper app, or if the user has selected the OS default, and we are that
+   * OS default, reset the preference back to pdf.js .
+   *
+   */
+  _validateAndMaybeUpdatePDFPrefs() {
+    let { processType, PROCESS_TYPE_DEFAULT } = Services.appinfo;
+    // If we're not in the parent, or are the default, then just say yes.
+    if (processType != PROCESS_TYPE_DEFAULT || PdfJs.cachedIsDefault()) {
+      return true;
+    }
+
+    // OK, PDF.js might not be the default. Find out if we've misled the user
+    // into making Firefox an external handler or if we're the OS default and
+    // Firefox is set to use the OS default:
+    let mime = Svc.mime.getFromTypeAndExtension(PDF_CONTENT_TYPE, "pdf");
+    // The above might throw errors. We're deliberately letting those bubble
+    // back up, where they'll tell the stream converter not to use us.
+
+    if (!mime) {
+      // This shouldn't happen, but we can't fix what isn't there. Assume
+      // we're OK to handle with PDF.js
+      return true;
+    }
+
+    const { saveToDisk, useHelperApp, useSystemDefault } = Ci.nsIHandlerInfo;
+    let { preferredAction, alwaysAskBeforeHandling } = mime;
+    // If the user has indicated they want to be asked or want to save to
+    // disk, we shouldn't render inline immediately:
+    if (alwaysAskBeforeHandling || preferredAction == saveToDisk) {
+      return false;
+    }
+    // If we have usable helper app info, don't use PDF.js
+    if (preferredAction == useHelperApp && this._usableHandler(mime)) {
+      return false;
+    }
+    // If we want the OS default and that's not Firefox, don't use PDF.js
+    if (preferredAction == useSystemDefault && !mime.isCurrentAppOSDefault()) {
+      return false;
+    }
+    // Log that we're doing this to help debug issues if people end up being
+    // surprised by this behaviour.
+    Cu.reportError("Found unusable PDF preferences. Fixing back to PDF.js");
+
+    mime.preferredAction = Ci.nsIHandlerInfo.handleInternally;
+    mime.alwaysAskBeforeHandling = false;
+    Svc.handlers.store(mime);
+    return true;
+  },
+
   getConvertedType(aFromType) {
+    if (!this._validateAndMaybeUpdatePDFPrefs()) {
+      throw new Components.Exception(
+        "Can't use PDF.js",
+        Cr.NS_ERROR_NOT_AVAILABLE
+      );
+    }
+
     return "text/html";
   },
 
   // nsIStreamListener::onDataAvailable
   onDataAvailable(aRequest, aInputStream, aOffset, aCount) {
     if (!this.dataListener) {
       return;
     }
--- a/browser/extensions/pdfjs/pdfjs.js
+++ b/browser/extensions/pdfjs/pdfjs.js
@@ -20,15 +20,15 @@ const { Services } = ChromeUtils.import(
 ChromeUtils.defineModuleGetter(
   this,
   "PdfStreamConverter",
   "resource://pdf.js/PdfStreamConverter.jsm"
 );
 
 // Register/unregister a constructor as a factory.
 function StreamConverterFactory() {
-  if (Services.cpmm.sharedData.get("pdfjs.enabled")) {
+  if (!Services.prefs.getBoolPref("pdfjs.disabled", false)) {
     return new PdfStreamConverter();
   }
   throw Components.Exception("", Cr.NS_ERROR_FACTORY_NOT_REGISTERED);
 }
 
 var EXPORTED_SYMBOLS = ["StreamConverterFactory"];
--- a/browser/extensions/pdfjs/test/browser_pdfjs_savedialog.js
+++ b/browser/extensions/pdfjs/test/browser_pdfjs_savedialog.js
@@ -35,19 +35,16 @@ function changeMimeHandler(preferredActi
     handlerInfo.alwaysAskBeforeHandling,
   ];
 
   // Change and save mime handler settings
   handlerInfo.alwaysAskBeforeHandling = alwaysAskBeforeHandling;
   handlerInfo.preferredAction = preferredAction;
   handlerService.store(handlerInfo);
 
-  Services.obs.notifyObservers(null, "pdfjs:handlerChanged");
-  Services.ppmm.sharedData.flush();
-
   // Refresh data
   handlerInfo = mimeService.getFromTypeAndExtension("application/pdf", "pdf");
 
   // Test: Mime handler was updated
   is(
     handlerInfo.alwaysAskBeforeHandling,
     alwaysAskBeforeHandling,
     "always-ask prompt change successful"
--- a/uriloader/exthandler/HandlerService.js
+++ b/uriloader/exthandler/HandlerService.js
@@ -3,16 +3,18 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm");
 const { XPCOMUtils } = ChromeUtils.import(
   "resource://gre/modules/XPCOMUtils.jsm"
 );
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
+const TOPIC_PDFJS_HANDLER_CHANGED = "pdfjs:handlerChanged";
+
 ChromeUtils.defineModuleGetter(
   this,
   "FileUtils",
   "resource://gre/modules/FileUtils.jsm"
 );
 ChromeUtils.defineModuleGetter(
   this,
   "JSONFile",
@@ -438,16 +440,22 @@ HandlerService.prototype = {
         delete storedHandlerInfo.extensions;
       }
     }
 
     // If we're saving *anything*, it stops being a stub:
     delete storedHandlerInfo.stubEntry;
 
     this._store.saveSoon();
+
+    // Now notify PDF.js. This is hacky, but a lot better than expecting all
+    // the consumers to do it...
+    if (handlerInfo.type == "application/pdf") {
+      Services.obs.notifyObservers(null, TOPIC_PDFJS_HANDLER_CHANGED);
+    }
   },
 
   // nsIHandlerService
   fillHandlerInfo(handlerInfo, overrideType) {
     let type = overrideType || handlerInfo.type;
     let storedHandlerInfo = this._getHandlerListByHandlerInfoType(handlerInfo)[
       type
     ];