Bug 1175102: generalize lost callback detection and reporting for async MozLoopAPI functions. r=Standard8
authorMike de Boer <mdeboer@mozilla.com>
Wed, 17 Jun 2015 18:14:24 +0200
changeset 249391 95d13cce0bc193d3aae6dc485c9207acea2cf20a
parent 249390 89fa4cae20d13106a304c8332fd4f2757476284a
child 249392 052b3627ccb1a7d35bbfce9c9c8a38b3a63f883d
push id28924
push userryanvm@gmail.com
push dateWed, 17 Jun 2015 19:01:32 +0000
treeherdermozilla-central@a3f280b6f8d5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8
bugs1175102
milestone41.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 1175102: generalize lost callback detection and reporting for async MozLoopAPI functions. r=Standard8
browser/components/loop/modules/MozLoopAPI.jsm
--- a/browser/components/loop/modules/MozLoopAPI.jsm
+++ b/browser/components/loop/modules/MozLoopAPI.jsm
@@ -119,16 +119,35 @@ const cloneValueInto = function(value, t
     MozLoopService.log.debug("Failed to clone value:", value);
     throw ex;
   }
 
   return clone;
 };
 
 /**
+ * Guarded callback invocation that reports when a callback function doesn't
+ * exist anymore.
+ *
+ * @param {Function} callback Callback function to be invoked.
+ * @param {...mixed} args     Rest param of callback function arguments.
+ */
+const invokeCallback = function(callback, ...args) {
+  if (typeof callback != "function") {
+    // We log an error, because it will have a stack trace attached which will
+    // be helpful whilst debugging.
+    MozLoopService.log.error.apply(MozLoopService.log,
+      [new Error("Callback function was lost!"), ...args]);
+    return;
+  }
+
+  return callback.apply(null, args);
+};
+
+/**
  * Get the two-digit hexadecimal code for a byte
  *
  * @param {byte} charCode
  */
 const toHexString = function(charCode) {
   return ("0" + charCode.toString(16)).slice(-2);
 };
 
@@ -429,17 +448,17 @@ function injectLoopAPI(targetWindow) {
      *                            `Error` object or `null`. The second argument will
      *                            be the result of the operation, if successfull.
      */
     startImport: {
       enumerable: true,
       writable: true,
       value: function(options, callback) {
         LoopContacts.startImport(options, getChromeWindow(targetWindow), function(...results) {
-          callback(...[cloneValueInto(r, targetWindow) for (r of results)]);
+          invokeCallback(callback, ...[cloneValueInto(r, targetWindow) for (r of results)]);
         });
       }
     },
 
     /**
      * Returns translated strings associated with an element. Designed
      * for use with l10n.js
      *
@@ -489,27 +508,27 @@ function injectLoopAPI(targetWindow) {
         let buttonFlags;
         if (options.okButton && options.cancelButton) {
           buttonFlags =
             (Ci.nsIPrompt.BUTTON_POS_0 * Ci.nsIPrompt.BUTTON_TITLE_IS_STRING) +
             (Ci.nsIPrompt.BUTTON_POS_1 * Ci.nsIPrompt.BUTTON_TITLE_IS_STRING);
         } else if (!options.okButton && !options.cancelButton) {
           buttonFlags = Services.prompt.STD_YES_NO_BUTTONS;
         } else {
-          callback(cloneValueInto(new Error("confirm: missing button options"), targetWindow));
+          invokeCallback(callback, cloneValueInto(new Error("confirm: missing button options"), targetWindow));
         }
 
         try {
           let chosenButton = Services.prompt.confirmEx(null, "",
             options.message, buttonFlags, options.okButton, options.cancelButton,
             null, null, {});
 
-          callback(null, chosenButton == 0);
+          invokeCallback(callback, null, chosenButton == 0);
         } catch (ex) {
-          callback(cloneValueInto(ex, targetWindow));
+          invokeCallback(callback, cloneValueInto(ex, targetWindow));
         }
       }
     },
 
     /**
      * Set any preference under "loop."
      *
      * @param {String} prefName The name of the pref without the preceding "loop."
@@ -612,30 +631,23 @@ function injectLoopAPI(targetWindow) {
      *                            transmitted with the request.
      * @param {Function} callback Called when the request completes.
      */
     hawkRequest: {
       enumerable: true,
       writable: true,
       value: function(sessionType, path, method, payloadObj, callback) {
         // XXX Should really return a DOM promise here.
-        let callbackIsFunction = (typeof callback == "function");
         MozLoopService.hawkRequest(sessionType, path, method, payloadObj).then((response) => {
-          callback(null, response.body);
+          invokeCallback(callback, null, response.body);
         }, hawkError => {
-          // When the function was garbage collected due to async events, like
-          // closing a window, we want to circumvent a JS error.
-          if (callbackIsFunction && typeof callback != "function") {
-            MozLoopService.log.error("hawkRequest: callback function was lost.", hawkError);
-            return;
-          }
           // 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({
+          invokeCallback(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);
       }
@@ -838,22 +850,22 @@ function injectLoopAPI(targetWindow) {
                         .createInstance(Ci.nsIXMLHttpRequest);
         let url = `chrome://browser/content/loop/shared/sounds/${name}.ogg`;
 
         request.open("GET", url, true);
         request.responseType = "arraybuffer";
         request.onload = () => {
           if (request.status < 200 || request.status >= 300) {
             let error = new Error(request.status + " " + request.statusText);
-            callback(cloneValueInto(error, targetWindow));
+            invokeCallback(callback, cloneValueInto(error, targetWindow));
             return;
           }
 
           let blob = new Blob([request.response], {type: "audio/ogg"});
-          callback(null, cloneValueInto(blob, targetWindow));
+          invokeCallback(callback, null, cloneValueInto(blob, targetWindow));
         };
 
         request.send();
       }
     },
 
     /**
      * Compose a URL pointing to the location of an avatar by email address.
@@ -907,17 +919,17 @@ function injectLoopAPI(targetWindow) {
           win.LoopUI.getFavicon(function(err, favicon) {
             if (err) {
               MozLoopService.log.error("Error occurred whilst fetching favicon", err);
               // We don't return here intentionally to make sure the callback is
               // invoked at all times. We just report the error here.
             }
             pageData.favicon = favicon || null;
 
-            callback(cloneValueInto(pageData, targetWindow));
+            invokeCallback(callback, cloneValueInto(pageData, targetWindow));
           });
         });
         win.gBrowser.selectedBrowser.messageManager.sendAsyncMessage("PageMetadata:GetPageData");
       }
     },
 
     /**
      * Associates a session-id and a call-id with a window for debugging.