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 225350 9cf1a0a453a029585d5e3692b9e5094d78eaf753
parent 225349 73ad2e4e793db87823170b02e58dc6a49ff70501
child 225351 a016dfb71871205f842e9a96a723ba4748034f2a
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1059754
milestone34.0a2
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;