Bug 1006102 - Include caller fileName and lineNumber info into WebExtensions normalized errors. draft
authorLuca Greco <lgreco@mozilla.com>
Wed, 26 Apr 2017 17:54:24 +0200
changeset 568805 b4353b5dcac51197eb5e388579e735532a695d04
parent 568804 ad1ec31b741367c1ebc6475011b69c1802033c70
child 626033 c99e32ca5ac0884b5cd44be2dab26c604d4fbbb7
push id55987
push userluca.greco@alcacoop.it
push dateWed, 26 Apr 2017 16:09:42 +0000
bugs1006102
milestone55.0a1
Bug 1006102 - Include caller fileName and lineNumber info into WebExtensions normalized errors. MozReview-Commit-ID: JKWYLUbZNTK
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/ExtensionCommon.jsm
toolkit/components/extensions/Schemas.jsm
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -434,16 +434,25 @@ this.ExtensionData = class {
     }).then(() => {
       let context = {
         url: this.baseURI && this.baseURI.spec,
 
         principal: this.principal,
 
         logError: error => {
           this.logger.warn(`Loading extension '${this.id}': Reading manifest: ${error}`);
+
+          // This log message is explicitly linked to the manifest.json and it is going
+          // to get a "Learn More" hyperlink attached to it.
+          let consoleMsg = Cc["@mozilla.org/scripterror;1"].createInstance(Ci.nsIScriptError);
+          consoleMsg.init(error,
+                          `moz-extension://${this.uuid}/manifest.json`,
+                          null, 0, 0, Ci.nsIScriptError.warningFlag,
+                          "WebExtension manifest.json");
+          Services.console.logMessage(consoleMsg);
         },
 
         preprocessors: {},
       };
 
       if (this.manifest.theme) {
         let invalidProps = validateThemeManifest(Object.getOwnPropertyNames(this.manifest));
 
--- a/toolkit/components/extensions/ExtensionCommon.jsm
+++ b/toolkit/components/extensions/ExtensionCommon.jsm
@@ -221,32 +221,42 @@ class BaseContext {
    * the target is an error object which belongs to that scope, it is
    * returned as-is. If it is an ordinary object with a `message`
    * property, it is converted into an error belonging to the target
    * scope. If it is an Error object which does *not* belong to the
    * clone scope, it is reported, and converted to an unexpected
    * exception error.
    *
    * @param {Error|object} error
+   * @param {object} callerInfo
+   * @param {string} callerInfo.fileName
+   * @param {number} callerInfo.lineNumber
    * @returns {Error}
    */
-  normalizeError(error) {
+  normalizeError(error, callerInfo) {
     if (error instanceof this.cloneScope.Error) {
       return error;
     }
-    let message, fileName;
+    let message, fileName, lineNumber;
     if (instanceOf(error, "Object") || error instanceof ExtensionError ||
         (typeof error == "object" && this.principal.subsumes(Cu.getObjectPrincipal(error)))) {
       message = error.message;
       fileName = error.fileName;
+      lineNumber = error.lineNumber;
     } else {
       Cu.reportError(error);
     }
+
+    if (callerInfo) {
+      fileName = callerInfo.fileName;
+      lineNumber = callerInfo.lineNumber;
+    }
+
     message = message || "An unexpected error occurred";
-    return new this.cloneScope.Error(message, fileName);
+    return new this.cloneScope.Error(message, fileName, lineNumber);
   }
 
   /**
    * Sets the value of `.lastError` to `error`, calls the given
    * callback, and reports an error if the value has not been checked
    * when the callback returns.
    *
    * @param {object} error An object with a `message` property. May
@@ -283,20 +293,24 @@ class BaseContext {
    *     each element will be used as a separate argument.
    *
    *     Unless the promise object belongs to the cloneScope global, its
    *     resolution value is cloned into cloneScope prior to calling the
    *     `callback` function or resolving the wrapped promise.
    *
    * @param {function} [callback] The callback function to wrap
    *
+   * @param {object} callerInfo
+   * @param {string} callerInfo.fileName
+   * @param {number} callerInfo.lineNumber
+   *
    * @returns {Promise|undefined} If callback is null, a promise object
    *     belonging to the target scope. Otherwise, undefined.
    */
-  wrapPromise(promise, callback = null) {
+  wrapPromise(promise, callback = null, callerInfo) {
     let runSafe = this.runSafe.bind(this);
     if (promise instanceof this.cloneScope.Promise) {
       runSafe = this.runSafeWithoutClone.bind(this);
     }
 
     if (callback) {
       promise.then(
         args => {
@@ -336,17 +350,17 @@ class BaseContext {
             }
           },
           value => {
             if (this.unloaded) {
               dump(`Promise rejected after context unloaded: ${value && value.message}\n`);
             } else if (!this.active) {
               dump(`Promise rejected while context is inactive: ${value && value.message}\n`);
             } else {
-              this.runSafeWithoutClone(reject, this.normalizeError(value));
+              this.runSafeWithoutClone(reject, this.normalizeError(value, callerInfo));
             }
           });
       });
     }
   }
 
   unload() {
     this.unloaded = true;
@@ -508,24 +522,25 @@ class LocalAPIImplementation extends Sch
   callFunction(args) {
     return this.pathObj[this.name](...args);
   }
 
   callFunctionNoReturn(args) {
     this.pathObj[this.name](...args);
   }
 
-  callAsyncFunction(args, callback) {
+  callAsyncFunction(args, callback, callerInfo) {
     let promise;
     try {
       promise = this.pathObj[this.name](...args) || Promise.resolve();
     } catch (e) {
       promise = Promise.reject(e);
     }
-    return this.context.wrapPromise(promise, callback);
+
+    return this.context.wrapPromise(promise, callback, callerInfo);
   }
 
   getProperty() {
     return this.pathObj[this.name];
   }
 
   setProperty(value) {
     this.pathObj[this.name] = value;
--- a/toolkit/components/extensions/Schemas.jsm
+++ b/toolkit/components/extensions/Schemas.jsm
@@ -915,16 +915,22 @@ class Entry {
     /**
      * @property {Array<string>} allowedContexts A list of allowed contexts
      * to consider before generating the API.
      * These are not parsed by the schema, but passed to `shouldInject`.
      */
     this.allowedContexts = schema.allowedContexts || [];
   }
 
+  getCallerInfo() {
+    return {
+      fileName: Components.stack.caller.caller.filename,
+      lineNumber: Components.stack.caller.caller.lineNumber,
+    };
+  }
   /**
    * Preprocess the given value with the preprocessor declared in
    * `preprocessor`.
    *
    * @param {*} value
    * @param {Context} context
    * @returns {*}
    */
@@ -953,18 +959,21 @@ class Entry {
   }
 
   /**
    * Logs a deprecation warning for this entry, based on the value of
    * its `deprecated` property.
    *
    * @param {Context} context
    * @param {value} [value]
+   * @param {object} callerInfo
+   * @param {string} callerInfo.fileName
+   * @param {number} callerInfo.lineNumber
    */
-  logDeprecation(context, value = null) {
+  logDeprecation(context, value = null, callerInfo) {
     let message = "This property is deprecated";
     if (typeof(this.deprecated) == "string") {
       message = this.deprecated;
       if (message.includes("${value}")) {
         try {
           value = JSON.stringify(value);
         } catch (e) {
           value = String(value);
@@ -977,20 +986,23 @@ class Entry {
   }
 
   /**
    * Checks whether the entry is deprecated and, if so, logs a
    * deprecation message.
    *
    * @param {Context} context
    * @param {value} [value]
+   * @param {object} callerInfo
+   * @param {string} callerInfo.fileName
+   * @param {number} callerInfo.lineNumber
    */
-  checkDeprecated(context, value = null) {
+  checkDeprecated(context, value = null, callerInfo) {
     if (this.deprecated) {
-      this.logDeprecation(context, value);
+      this.logDeprecation(context, value, callerInfo);
     }
   }
 
   /**
    * Returns an object containing property descriptor for use when
    * injecting this entry into an API object.
    *
    * @param {Array<string>} path The API path, e.g. `["storage", "local"]`.
@@ -2071,43 +2083,48 @@ FunctionEntry = class FunctionEntry exte
     this.isAsync = type.isAsync;
     this.hasAsyncCallback = type.hasAsyncCallback;
   }
 
   getDescriptor(path, context) {
     let apiImpl = context.getImplementation(path.join("."), this.name);
 
     let stub;
+
     if (this.isAsync) {
       stub = (...args) => {
-        this.checkDeprecated(context);
+        let callerInfo = this.getCallerInfo();
+        this.checkDeprecated(context, null, callerInfo);
         let actuals = this.checkParameters(args, context);
         let callback = null;
         if (this.hasAsyncCallback) {
           callback = actuals.pop();
         }
         if (callback === null && context.isChromeCompat) {
           // We pass an empty stub function as a default callback for
           // the `chrome` API, so promise objects are not returned,
           // and lastError values are reported immediately.
           callback = () => {};
         }
+
         return apiImpl.callAsyncFunction(actuals, callback);
       };
     } else if (!this.returns) {
       stub = (...args) => {
-        this.checkDeprecated(context);
+        let callerInfo = this.getCallerInfo();
+        this.checkDeprecated(context, null, callerInfo);
         let actuals = this.checkParameters(args, context);
-        return apiImpl.callFunctionNoReturn(actuals);
+        return apiImpl.callFunctionNoReturn(actuals, callerInfo);
       };
     } else {
       stub = (...args) => {
-        this.checkDeprecated(context);
+        let callerInfo = this.getCallerInfo();
+        this.checkDeprecated(context, null, callerInfo);
         let actuals = this.checkParameters(args, context);
-        return apiImpl.callFunction(actuals);
+        return apiImpl.callFunction(actuals, callerInfo);
       };
     }
 
     return {
       descriptor: {value: Cu.exportFunction(stub, context.cloneScope)},
       revoke() {
         apiImpl.revoke();
         apiImpl = null;
@@ -2153,27 +2170,27 @@ class Event extends CallEntry {
   }
 
   getDescriptor(path, context) {
     let apiImpl = context.getImplementation(path.join("."), this.name);
 
     let addStub = (listener, ...args) => {
       listener = this.checkListener(listener, context);
       let actuals = this.checkParameters(args, context);
-      apiImpl.addListener(listener, actuals);
+      apiImpl.addListener(listener, actuals, this.getCallerInfo());
     };
 
     let removeStub = (listener) => {
       listener = this.checkListener(listener, context);
-      apiImpl.removeListener(listener);
+      apiImpl.removeListener(listener, this.getCallerInfo());
     };
 
     let hasStub = (listener) => {
       listener = this.checkListener(listener, context);
-      return apiImpl.hasListener(listener);
+      return apiImpl.hasListener(listener, this.getCallerInfo());
     };
 
     let obj = Cu.createObjectIn(context.cloneScope);
 
     Cu.exportFunction(addStub, obj, {defineAs: "addListener"});
     Cu.exportFunction(removeStub, obj, {defineAs: "removeListener"});
     Cu.exportFunction(hasStub, obj, {defineAs: "hasListener"});