Bug 1398630: Part 1 - Remove/cleanup some old ExtensionUtils helpers. r?zombie draft
authorKris Maglione <maglione.k@gmail.com>
Sun, 10 Sep 2017 15:39:28 -0700
changeset 662071 8a7e15de7bf9a73f963ada5da88f26de44459c44
parent 662070 10629cc177b4a3925d5547bf94df550c36b1b155
child 662072 71f5c6267f68a842c2fd3f3b98c307a4857b1770
push id78941
push usermaglione.k@gmail.com
push dateSun, 10 Sep 2017 22:46:43 +0000
reviewerszombie
bugs1398630
milestone57.0a1
Bug 1398630: Part 1 - Remove/cleanup some old ExtensionUtils helpers. r?zombie MozReview-Commit-ID: FeLUjH7pkiB
toolkit/components/extensions/ExtensionCommon.jsm
toolkit/components/extensions/ExtensionParent.jsm
toolkit/components/extensions/ExtensionPermissions.jsm
toolkit/components/extensions/ExtensionUtils.jsm
toolkit/components/extensions/Schemas.jsm
--- a/toolkit/components/extensions/ExtensionCommon.jsm
+++ b/toolkit/components/extensions/ExtensionCommon.jsm
@@ -41,17 +41,16 @@ var {
   DefaultWeakMap,
   EventEmitter,
   ExtensionError,
   defineLazyGetter,
   filterStack,
   getConsole,
   getInnerWindowID,
   getUniqueId,
-  instanceOf,
 } = ExtensionUtils;
 
 XPCOMUtils.defineLazyGetter(this, "console", getConsole);
 
 var ExtensionCommon;
 
 /**
  * A sentinel class to indicate that an array of values should be
@@ -372,18 +371,20 @@ class BaseContext {
    * @param {Error|object} error
    * @returns {Error}
    */
   normalizeError(error) {
     if (error instanceof this.cloneScope.Error) {
       return error;
     }
     let message, fileName;
-    if (instanceOf(error, "Object") || error instanceof ExtensionError ||
-        (typeof error == "object" && this.principal.subsumes(Cu.getObjectPrincipal(error)))) {
+    if (error && typeof error === "object" &&
+        (ChromeUtils.getClassName(error) === "Object" ||
+         error instanceof ExtensionError ||
+         this.principal.subsumes(Cu.getObjectPrincipal(error)))) {
       message = error.message;
       fileName = error.fileName;
     } else {
       Cu.reportError(error);
     }
     message = message || "An unexpected error occurred";
     return new this.cloneScope.Error(message, fileName);
   }
@@ -1445,52 +1446,55 @@ LocaleData.prototype = {
     });
   },
 
   // Validates the contents of a locale JSON file, normalizes the
   // messages into a Map of message key -> localized string pairs.
   addLocale(locale, messages, extension) {
     let result = new Map();
 
+    let isPlainObject = obj => (obj && typeof obj === "object" &&
+                                ChromeUtils.getClassName(obj) === "Object");
+
     // Chrome does not document the semantics of its localization
     // system very well. It handles replacements by pre-processing
     // messages, replacing |$[a-zA-Z0-9@_]+$| tokens with the value of their
     // replacements. Later, it processes the resulting string for
     // |$[0-9]| replacements.
     //
     // Again, it does not document this, but it accepts any number
     // of sequential |$|s, and replaces them with that number minus
     // 1. It also accepts |$| followed by any number of sequential
     // digits, but refuses to process a localized string which
     // provides more than 9 substitutions.
-    if (!instanceOf(messages, "Object")) {
+    if (!isPlainObject(messages)) {
       extension.packagingError(`Invalid locale data for ${locale}`);
       return result;
     }
 
     for (let key of Object.keys(messages)) {
       let msg = messages[key];
 
-      if (!instanceOf(msg, "Object") || typeof(msg.message) != "string") {
+      if (!isPlainObject(msg) || typeof(msg.message) != "string") {
         extension.packagingError(`Invalid locale message data for ${locale}, message ${JSON.stringify(key)}`);
         continue;
       }
 
       // Substitutions are case-insensitive, so normalize all of their names
       // to lower-case.
       let placeholders = new Map();
-      if (instanceOf(msg.placeholders, "Object")) {
+      if (isPlainObject(msg.placeholders)) {
         for (let key of Object.keys(msg.placeholders)) {
           placeholders.set(key.toLowerCase(), msg.placeholders[key]);
         }
       }
 
       let replacer = (match, name) => {
         let replacement = placeholders.get(name.toLowerCase());
-        if (instanceOf(replacement, "Object") && "content" in replacement) {
+        if (isPlainObject(replacement) && "content" in replacement) {
           return replacement.content;
         }
         return "";
       };
 
       let value = msg.message.replace(/\$([A-Za-z0-9@_]+)\$/g, replacer);
 
       // Message names are also case-insensitive, so normalize them to lower-case.
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -47,17 +47,16 @@ var {
 var {
   DefaultMap,
   DefaultWeakMap,
   ExtensionError,
   MessageManagerProxy,
   defineLazyGetter,
   promiseDocumentLoaded,
   promiseEvent,
-  promiseFileContents,
   promiseObserved,
 } = ExtensionUtils;
 
 const BASE_SCHEMA = "chrome://extensions/content/schemas/manifest.json";
 const CATEGORY_EXTENSION_MODULES = "webextension-modules";
 const CATEGORY_EXTENSION_SCHEMAS = "webextension-schemas";
 const CATEGORY_EXTENSION_SCRIPTS = "webextension-scripts";
 
@@ -1430,19 +1429,19 @@ StartupCache = {
   getBlob() {
     return new Uint8Array(aomStartup.encodeBlob(this._data));
   },
 
   _data: null,
   async _readData() {
     let result = new Map();
     try {
-      let data = await promiseFileContents(this.file);
+      let {buffer} = await OS.File.read(this.file);
 
-      result = aomStartup.decodeBlob(data);
+      result = aomStartup.decodeBlob(buffer);
     } catch (e) {
       if (!e.becauseNoSuchFile) {
         Cu.reportError(e);
       }
     }
 
     this._data = result;
     return result;
--- a/toolkit/components/extensions/ExtensionPermissions.jsm
+++ b/toolkit/components/extensions/ExtensionPermissions.jsm
@@ -22,18 +22,18 @@ let _initPromise;
 
 async function _lazyInit() {
   let path = OS.Path.join(OS.Constants.Path.profileDir, FILE_NAME);
 
   prefs = new JSONFile({path});
   prefs.data = {};
 
   try {
-    let blob = await ExtensionUtils.promiseFileContents(path);
-    prefs.data = JSON.parse(new TextDecoder().decode(blob));
+    let {buffer} = await OS.File.read(path);
+    prefs.data = JSON.parse(new TextDecoder().decode(buffer));
   } catch (e) {
     if (!e.becauseNoSuchFile) {
       Cu.reportError(e);
     }
   }
 }
 
 function lazyInit() {
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -32,21 +32,16 @@ const appinfo = Cc["@mozilla.org/xre/app
 
 let nextId = 0;
 const uniqueProcessID = String(appinfo.uniqueProcessID);
 
 function getUniqueId() {
   return `${nextId++}-${uniqueProcessID}`;
 }
 
-async function promiseFileContents(path) {
-  let res = await OS.File.read(path);
-  return res.buffer;
-}
-
 
 /**
  * An Error subclass for which complete error messages are always passed
  * to extensions, rather than being interpreted as an unknown error.
  */
 class ExtensionError extends Error {}
 
 function filterStack(error) {
@@ -58,65 +53,21 @@ function runSafeSyncWithoutClone(f, ...a
   try {
     return f(...args);
   } catch (e) {
     dump(`Extension error: ${e} ${e.fileName} ${e.lineNumber}\n[[Exception stack\n${filterStack(e)}Current stack\n${filterStack(Error())}]]\n`);
     Cu.reportError(e);
   }
 }
 
-// Run a function and report exceptions.
-function runSafeWithoutClone(f, ...args) {
-  if (typeof(f) != "function") {
-    dump(`Extension error: expected function\n${filterStack(Error())}`);
-    return;
-  }
-
-  Promise.resolve().then(() => {
-    runSafeSyncWithoutClone(f, ...args);
-  });
-}
-
-// Run a function, cloning arguments into context.cloneScope, and
-// report exceptions. |f| is expected to be in context.cloneScope.
-function runSafeSync(context, f, ...args) {
-  if (context.unloaded) {
-    Cu.reportError("runSafeSync called after context unloaded");
-    return;
-  }
-
-  try {
-    args = Cu.cloneInto(args, context.cloneScope);
-  } catch (e) {
-    Cu.reportError(e);
-    dump(`runSafe failure: cloning into ${context.cloneScope}: ${e}\n\n${filterStack(Error())}`);
-  }
-  return runSafeSyncWithoutClone(f, ...args);
-}
-
-// Run a function, cloning arguments into context.cloneScope, and
-// report exceptions. |f| is expected to be in context.cloneScope.
-function runSafe(context, f, ...args) {
-  try {
-    args = Cu.cloneInto(args, context.cloneScope);
-  } catch (e) {
-    Cu.reportError(e);
-    dump(`runSafe failure: cloning into ${context.cloneScope}: ${e}\n\n${filterStack(Error())}`);
-  }
-  if (context.unloaded) {
-    dump(`runSafe failure: context is already unloaded ${filterStack(new Error())}\n`);
-    return undefined;
-  }
-  return runSafeWithoutClone(f, ...args);
-}
-
 // Return true if the given value is an instance of the given
 // native type.
 function instanceOf(value, type) {
-  return {}.toString.call(value) == `[object ${type}]`;
+  return (value && typeof value === "object" &&
+          ChromeUtils.getClassName(value) === type);
 }
 
 /**
  * Similar to a WeakMap, but creates a new key with the given
  * constructor if one is not present.
  */
 class DefaultWeakMap extends WeakMap {
   constructor(defaultConstructor, init) {
@@ -679,22 +630,18 @@ this.ExtensionUtils = {
   getUniqueId,
   filterStack,
   getWinUtils,
   instanceOf,
   normalizeTime,
   promiseDocumentLoaded,
   promiseDocumentReady,
   promiseEvent,
-  promiseFileContents,
   promiseObserved,
-  runSafe,
-  runSafeSync,
   runSafeSyncWithoutClone,
-  runSafeWithoutClone,
   withHandlingUserInput,
   DefaultMap,
   DefaultWeakMap,
   EventEmitter,
   ExtensionError,
   LimitedSet,
   MessageManagerProxy,
 };
--- a/toolkit/components/extensions/Schemas.jsm
+++ b/toolkit/components/extensions/Schemas.jsm
@@ -16,17 +16,16 @@ Cu.importGlobalProperties(["URL"]);
 Cu.import("resource://gre/modules/AppConstants.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 var {
   DefaultMap,
   DefaultWeakMap,
-  instanceOf,
 } = ExtensionUtils;
 
 XPCOMUtils.defineLazyModuleGetter(this, "ExtensionParent",
                                   "resource://gre/modules/ExtensionParent.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
                                   "resource://gre/modules/NetUtil.jsm");
 XPCOMUtils.defineLazyServiceGetter(this, "contentPolicyService",
                                    "@mozilla.org/addons/content-policy;1",
@@ -1622,17 +1621,17 @@ class ObjectType extends Type {
           if (Object.keys(this.properties).length ||
               this.patternProperties.length ||
               !(this.additionalProperties instanceof AnyType)) {
             throw new Error("InternalError: isInstanceOf can only be used " +
                             "with objects that are otherwise unrestricted");
           }
         }
 
-        if (!instanceOf(value, this.isInstanceOf)) {
+        if (ChromeUtils.getClassName(value) !== this.isInstanceOf) {
           return context.error(`Object must be an instance of ${this.isInstanceOf}`,
                                `be an instance of ${this.isInstanceOf}`);
         }
 
         // This is kind of a hack, but we can't normalize things that
         // aren't JSON, so we just return them.
         return this.postprocess({value}, context);
       }