Bug 1018320 - RequestSync API - patch 7 - reject promise when an exception is thrown, r=fabrice
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 13 Jan 2015 09:53:26 +0000
changeset 223542 f15ef701bdf2c9505c37ddd456ee10278fcc69f2
parent 223541 71263e63af0b7e11308c288eba4ce893d64c8c9d
child 223543 2f2b89a7f71a4290667d0b21a9a62bd340c4a764
push id53943
push useramarchesini@mozilla.com
push dateTue, 13 Jan 2015 09:53:46 +0000
treeherdermozilla-inbound@f15ef701bdf2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfabrice
bugs1018320
milestone38.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 1018320 - RequestSync API - patch 7 - reject promise when an exception is thrown, r=fabrice
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!");
       }