Bug 1018320 - RequestSync API - patch 7 - reject promise when an exception is thrown, r=fabrice
☠☠ backed out by 636498d041b5 ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Sun, 04 Jan 2015 10:37:24 +0100
changeset 221959 2ef1c26d77d3f892e385b828f0694a41a3740006
parent 221958 bce9ed290dddb7492cf3b333751589af21c4a563
child 221960 c61b88b3479a7356136fe0916c6e5df8e00e67ff
push id53474
push useramarchesini@mozilla.com
push dateSun, 04 Jan 2015 09:38:35 +0000
treeherdermozilla-inbound@2ef1c26d77d3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfabrice
bugs1018320
milestone37.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_promise_app.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;
+    }
 
     aDispatcher.isHandling = false;
     this._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_promise_app.html
+++ b/dom/requestsync/tests/test_promise_app.html
@@ -12,59 +12,71 @@
   <script type="application/javascript;version=1.7">
 
   var foobarCounter = 0;
   var pendingCounter = 0;
   function setMessageHandler() {
     navigator.mozSetMessageHandler('request-sync', function(e) {
 
       if (e.task == 'foobar') {
-        ok(true, "foobar message received:" + foobarCounter);
+        ok(true, "foobar message received:" + ++foobarCounter);
 
-        if (++foobarCounter == 1) {
+        if (foobarCounter == 1) {
           // The first time we wait 2 seconds.
           info("Setting a promise object.");
           navigator.mozSetMessageHandlerPromise(new Promise(function(a, b) {
+            SimpleTest.requestFlakyTimeout("Just testing to make sure things work.");
             setTimeout(a, 2000);
           }));
-        } else {
+        } else if (foobarCounter == 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 (foobarCounter == 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);
+          }));
         }
       }
 
       else if (e.task  == 'pending') {
-        ok(true, "pending message received: " + pendingCounter);
-        if (++pendingCounter == 2) {
+        ok(true, "pending message received: " + ++pendingCounter);
+        if (pendingCounter == 5) {
           runTests();
         }
       }
 
       else {
       ok(false, "Unknown message");
       }
     });
 
     runTests();
   }
 
   function test_register_foobar() {
-    navigator.sync.register('foobar', { minInterval: 5,
+    navigator.sync.register('foobar', { minInterval: 2,
                                         oneShot: false,
                                         data: 42,
                                         wifiOnly: false,
                                         wakeUpPage: location.href }).then(
     function() {
       ok(true, "navigator.sync.register() foobar done");
       runTests();
     }, genericError);
   }
 
   function test_register_pending() {
-    navigator.sync.register('pending', { minInterval: 6,
+    navigator.sync.register('pending', { minInterval: 2,
                                          oneShot: false,
                                          data: 'hello world!',
                                          wifiOnly: false,
                                          wakeUpPage: location.href }).then(
     function() {
       ok(true, "navigator.sync.register() pending done");
       runTests();
     }, genericError);
@@ -89,17 +101,17 @@
   function test_wait() {
     // nothing to do here.
   }
 
   var tests = [
     function() {
       SpecialPowers.pushPrefEnv({"set": [["dom.requestSync.enabled", true],
                                          ["dom.requestSync.minInterval", 1],
-                                         ["dom.requestSync.maxTaskTimeout", 10000 /* 10 seconds */],
+                                         ["dom.requestSync.maxTaskTimeout", 5000 /* 5 seconds */],
                                          ["dom.ignore_webidl_scope_checks", true]]}, runTests);
     },
 
     function() {
       SpecialPowers.pushPermissions(
         [{ "type": "requestsync-manager", "allow": 1, "context": document } ], runTests);
     },