Bug 1059754 - Propagating errors to content with cloneInto fails in MozLoopAPI. r=Gijs
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Fri, 12 Sep 2014 11:10:10 +0100
changeset 205144 9aae4c1be901302c98f2743435ea70d4dd886bb8
parent 205143 3f3e0c97c988d14c273e67d3f512261876a9a253
child 205145 7b7a4bb58bff45456f0f8fd40d363f50f9bf8e62
push id49106
push userphilringnalda@gmail.com
push dateSat, 13 Sep 2014 17:12:34 +0000
treeherdermozilla-inbound@ab04d0f2665f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1059754
milestone35.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 1059754 - Propagating errors to content with cloneInto fails in MozLoopAPI. r=Gijs
browser/components/loop/MozLoopAPI.jsm
--- a/browser/components/loop/MozLoopAPI.jsm
+++ b/browser/components/loop/MozLoopAPI.jsm
@@ -37,41 +37,53 @@ const cloneErrorObject = function(error,
   let obj = new targetWindow.Error();
   for (let prop of Object.getOwnPropertyNames(error)) {
     obj[prop] = String(error[prop]);
   }
   return obj;
 };
 
 /**
+ * Makes an object or value available to an unprivileged target window.
+ *
+ * Primitives are returned as they are, while objects are cloned into the
+ * specified target.  Error objects are also handled correctly.
+ *
+ * @param {any}          value        Value or object to copy
+ * @param {nsIDOMWindow} targetWindow The content window to copy to
+ */
+const cloneValueInto = function(value, targetWindow) {
+  if (!value || typeof value != "object") {
+    return value;
+  }
+
+  // Inspect for an error this way, because the Error object is special.
+  if (value.constructor.name == "Error") {
+    return cloneErrorObject(value, targetWindow);
+  }
+
+  return Cu.cloneInto(value, targetWindow);
+};
+
+/**
  * Inject any API containing _only_ function properties into the given window.
  *
  * @param {Object}       api          Object containing functions that need to
  *                                    be exposed to content
  * @param {nsIDOMWindow} targetWindow The content window to attach the API
  */
 const injectObjectAPI = function(api, targetWindow) {
   let injectedAPI = {};
   // Wrap all the methods in `api` to help results passed to callbacks get
   // through the priv => unpriv barrier with `Cu.cloneInto()`.
   Object.keys(api).forEach(func => {
     injectedAPI[func] = function(...params) {
       let callback = params.pop();
       api[func](...params, function(...results) {
-        results = results.map(result => {
-          if (result && typeof result == "object") {
-            // Inspect for an error this way, because the Error object is special.
-            if (result.constructor.name == "Error") {
-              return cloneErrorObject(result.message)
-            }
-            return Cu.cloneInto(result, targetWindow);
-          }
-          return result;
-        });
-        callback(...results);
+        callback(...[cloneValueInto(r, targetWindow) for (r of results)]);
       });
     };
   });
 
   let contentObj = Cu.cloneInto(injectedAPI, targetWindow, {cloneFunctions: true});
   // Since we deny preventExtensions on XrayWrappers, because Xray semantics make
   // it difficult to act like an object has actually been frozen, we try to seal
   // the `contentObj` without Xrays.
@@ -198,21 +210,21 @@ function injectLoopAPI(targetWindow) {
      *                            happened.
      */
     ensureRegistered: {
       enumerable: true,
       writable: true,
       value: function(callback) {
         // We translate from a promise to a callback, as we can't pass promises from
         // Promise.jsm across the priv versus unpriv boundary.
-        return MozLoopService.register().then(() => {
+        MozLoopService.register().then(() => {
           callback(null);
         }, err => {
-          callback(err);
-        });
+          callback(cloneValueInto(err, targetWindow));
+        }).catch(Cu.reportError);
       }
     },
 
     /**
      * Used to note a call url expiry time. If the time is later than the current
      * latest expiry time, then the stored expiry time is increased. For times
      * sooner, this function is a no-op; this ensures we always have the latest
      * expiry time for a url.
@@ -352,21 +364,30 @@ function injectLoopAPI(targetWindow) {
      * @param {Function} callback Called when the request completes.
      */
     hawkRequest: {
       enumerable: true,
       writable: true,
       value: function(path, method, payloadObj, callback) {
         // XXX: Bug 1065153 - Should take a sessionType parameter instead of hard-coding GUEST
         // XXX Should really return a DOM promise here.
-        return MozLoopService.hawkRequest(LOOP_SESSION_TYPE.GUEST, path, method, payloadObj).then((response) => {
+        MozLoopService.hawkRequest(LOOP_SESSION_TYPE.GUEST, path, method, payloadObj).then((response) => {
           callback(null, response.body);
-        }, (error) => {
-          callback(Cu.cloneInto(error, targetWindow));
-        });
+        }, hawkError => {
+          // The hawkError.error property, while usually a string representing
+          // an HTTP response status message, may also incorrectly be a native
+          // error object that will cause the cloning function to fail.
+          callback(Cu.cloneInto({
+            error: (hawkError.error && typeof hawkError.error == "string")
+                   ? hawkError.error : "Unexpected exception",
+            message: hawkError.message,
+            code: hawkError.code,
+            errno: hawkError.errno,
+          }, targetWindow));
+        }).catch(Cu.reportError);
       }
     },
 
     LOOP_SESSION_TYPE: {
       enumerable: true,
       writable: false,
       value: function() {
         return LOOP_SESSION_TYPE;