Bug 1018320 - RequestSync API - Patch 7 - Reject promise when an exception is thrown. r=fabrice, a=bajaj
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 13 Jan 2015 09:53:26 +0000
changeset 237141 addef534a15bc1689ff4b4e7f697531756a7c073
parent 237140 d283ce0f09967d1c5296c34e135fa8d5a8622596
child 237142 9b267d35a70711ee778c214c6171e15419dc7cfc
push id213
push userryanvm@gmail.com
push dateTue, 24 Feb 2015 00:59:48 +0000
treeherdermozilla-b2g37_v2_2@b5a532c7f606 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfabrice, bajaj
bugs1018320
milestone37.0
Bug 1018320 - RequestSync API - Patch 7 - Reject promise when an exception is thrown. r=fabrice, a=bajaj
dom/messages/SystemMessageInternal.js
dom/messages/SystemMessageManager.js
dom/requestsync/RequestSyncService.jsm
dom/requestsync/tests/test_wakeUp.html
--- a/dom/messages/SystemMessageInternal.js
+++ b/dom/messages/SystemMessageInternal.js
@@ -465,29 +465,29 @@ SystemMessageInternal.prototype = {
         debug("Got child-process-shutdown from " + aMessage.target);
         for (let manifestURL in this._listeners) {
           // See if any processes in this manifest URL have this target.
           this._removeTargetFromListener(aMessage.target,
                                          manifestURL,
                                          true,
                                          null);
 
-          this._rejectPendingPromises(manifestURL);
+          this._rejectPendingPromisesFromManifestURL(manifestURL);
         }
         break;
       }
       case "SystemMessageManager:Unregister":
       {
         debug("Got Unregister from " + aMessage.target +
               " innerWinID " + msg.innerWindowID);
         this._removeTargetFromListener(aMessage.target,
                                        msg.manifestURL,
                                        false,
                                        msg.pageURL);
-        this._rejectPendingPromises(msg.manifestURL);
+        this._rejectPendingPromisesFromManifestURL(msg.manifestURL);
         break;
       }
       case "SystemMessageManager:GetPendingMessages":
       {
         debug("received SystemMessageManager:GetPendingMessages " + msg.type +
           " for " + msg.pageURL + " @ " + msg.manifestURL);
 
         // This is a sync call used to return the pending messages for a page.
@@ -549,20 +549,24 @@ SystemMessageInternal.prototype = {
           }
         }
         break;
       }
       case "SystemMessageManager:HandleMessageDone":
       {
         debug("received SystemMessageManager:HandleMessageDone " + msg.type +
           " with msgID " + msg.msgID + " for " + msg.pageURL +
-          " @ " + msg.manifestURL);
+          " @ " + msg.manifestURL + " - promise rejected: " + msg.rejected);
 
-        // Maybe this should resolve a pending promise.
-        this._resolvePendingPromises(msg.msgID);
+        // Maybe this should resolve/reject a pending promise.
+        if (msg.rejected) {
+          this._rejectPendingPromises(msg.msgID);
+        } else {
+          this._resolvePendingPromises(msg.msgID);
+        }
 
         // A page has finished handling some of its system messages, so we try
         // to release the CPU wake lock we acquired on behalf of that page.
         this._releaseCpuWakeLock(this._createKeyForPage(msg), 1);
         break;
       }
 
       case "SystemMessageManager:HandleMessagesDone":
@@ -639,17 +643,17 @@ SystemMessageInternal.prototype = {
           let page = this._pages[i];
           if (page.manifestURL === manifestURL) {
             this._pages.splice(i, 1);
             debug("Remove " + page.pageURL + " @ " + page.manifestURL +
                   " from registered pages due to app uninstallation.");
           }
         }
 
-        this._rejectPendingPromises(manifestURL);
+        this._rejectPendingPromisesFromManifestURL(manifestURL);
 
         debug("Finish updating registered pages for an uninstalled app.");
         break;
     }
   },
 
   _queueMessage: function(aPage, aMessage, aMessageID) {
     // Queue the message for this page because we've never known if an app is
@@ -784,17 +788,30 @@ SystemMessageInternal.prototype = {
 
     let obj = this._pendingPromises.get(aMessageID);
     if (!--obj.counter) {
       obj.resolvePromiseCb();
       this._pendingPromises.delete(aMessageID);
     }
   },
 
-  _rejectPendingPromises: function(aManifestURL) {
+  _rejectPendingPromises: function(aMessageID) {
+    if (!this._pendingPromises.has(aMessageID)) {
+      debug("Unknown pendingPromise messageID. This seems a bug!!");
+      return;
+    }
+
+    let obj = this._pendingPromises.get(aMessageID);
+    if (!--obj.counter) {
+      obj.rejectPromiseCb();
+      this._pendingPromises.delete(aMessageID);
+    }
+  },
+
+  _rejectPendingPromisesFromManifestURL: function(aManifestURL) {
     for (var [i, obj] of this._pendingPromises) {
       if (obj.manifestURL == aManifestURL) {
         obj.rejectPromiseCb();
         this._pendingPromises.delete(i);
       }
     }
   },
 
--- a/dom/messages/SystemMessageManager.js
+++ b/dom/messages/SystemMessageManager.js
@@ -90,41 +90,50 @@ SystemMessageManager.prototype = {
       let wrapper = Cc[contractID].createInstance(Ci.nsISystemMessagesWrapper);
       if (wrapper) {
         aMessage = wrapper.wrapMessage(aMessage, this._window);
         wrapped = true;
         debug("wrapped = " + aMessage);
       }
     }
 
-    aDispatcher.handler
-      .handleMessage(wrapped ? aMessage
-                             : Cu.cloneInto(aMessage, this._window));
+    let message = wrapped ? aMessage : Cu.cloneInto(aMessage, this._window);
+
+    let rejectPromise = false;
+    try {
+      aDispatcher.handler.handleMessage(message);
+    } catch(e) {
+      rejectPromise = true;
+    }
 
     this._isHandling = false;
     aDispatcher.isHandling = false;
 
     let self = this;
     function sendResponse() {
       // We need to notify the parent one of the system messages has been
       // handled, so the parent can release the CPU wake lock it took on our
       // behalf.
       cpmm.sendAsyncMessage("SystemMessageManager:HandleMessageDone",
                             { type: aType,
                               manifestURL: self._manifestURL,
                               pageURL: self._pageURL,
-                              msgID: aMessageID });
+                              msgID: aMessageID,
+                              rejected: rejectPromise });
     }
 
     if (!this._promise) {
       debug("No promise set, sending the response immediately.");
       sendResponse();
     } else {
       debug("Using the promise to postpone the response.");
-      this._promise.then(sendResponse, sendResponse);
+      this._promise.then(sendResponse, function() {
+        rejectPromise = true;
+        sendResponse();
+      });
       this._promise = null;
     }
 
     if (aDispatcher.messages.length > 0) {
       let msg = aDispatcher.messages.shift();
       this._dispatchMessage(aType, aDispatcher, msg.message, msg.messageID);
     } else {
       // No more messages that need to be handled, we can notify the
--- a/dom/requestsync/RequestSyncService.jsm
+++ b/dom/requestsync/RequestSyncService.jsm
@@ -673,16 +673,17 @@ this.RequestSyncService = {
     } catch(e) {}
 
     timer.initWithCallback(function() {
       debug("Task is taking too much, let's ignore the promise.");
       taskCompleted();
     }, timeout, Ci.nsITimer.TYPE_ONE_SHOT);
 
     // Sending the message.
+    debug("Sending message.");
     let promise =
       systemMessenger.sendMessage('request-sync',
                                   this.createPartialTaskObject(aObj.data),
                                   pageURL, manifestURL);
 
     promise.then(function() {
       debug("promise resolved");
       taskCompleted();
--- a/dom/requestsync/tests/test_wakeUp.html
+++ b/dom/requestsync/tests/test_wakeUp.html
@@ -10,17 +10,17 @@
 <body>
 <div id="container"></div>
   <script type="application/javascript;version=1.7">
 
   var oneShotCounter = 0;
   var multiShotCounter = 0;
 
   function maybeDone() {
-    if (oneShotCounter == 1 && multiShotCounter == 3) {
+    if (oneShotCounter == 1 && multiShotCounter == 5) {
       runTests();
     }
   }
 
   function setMessageHandler() {
     navigator.mozSetMessageHandler('request-sync', function(e) {
       ok(true, "One event has been received!");
 
@@ -52,17 +52,28 @@
 
         if (multiShotCounter == 1) {
           info("Setting a promise object.");
           navigator.mozSetMessageHandlerPromise(new Promise(function(a, b) {
             setTimeout(a, 0);
           }));
         } else if (multiShotCounter == 2) {
           // The second time we don't reply at all.
+          info("Setting a promise object without resolving it.");
           navigator.mozSetMessageHandlerPromise(new Promise(function(a, b) {}));
+        } else if (multiShotCounter == 3) {
+          info("Throwing an exception.");
+          // Now we throw an exception
+          SimpleTest.expectUncaughtException();
+          throw "Booom!";
+        } else {
+          info("Setting a promise object and reject it.");
+          navigator.mozSetMessageHandlerPromise(new Promise(function(a, b) {
+            setTimeout(b, 0);
+          }));
         }
 
         maybeDone();
       }
 
       else {
         ok(false, "Unknown event has been received!");
       }