Bug 1018320 - RequestSync API - patch 7 - reject promise when an exception is thrown, r=fabrice
☠☠ backed out by c3569fd75d4e ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Mon, 05 Jan 2015 13:42:09 +0100
changeset 222000 f60d4ad64070be0ae3427d52f9f7b5b1f1b0fc77
parent 221999 1aac4d23ccd263789b5a7a865d6e80ec7a247b82
child 222001 597b1585ab72a17551beedf8003ffe1cbcd5bb6d
push id53487
push useramarchesini@mozilla.com
push dateMon, 05 Jan 2015 12:42:21 +0000
treeherdermozilla-inbound@f60d4ad64070 [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
b2g/app/b2g.js
dom/messages/SystemMessageInternal.js
dom/messages/SystemMessageManager.js
dom/requestsync/RequestSyncService.jsm
dom/requestsync/tests/test_promise_app.html
dom/requestsync/tests/test_webidl.html
dom/tests/mochitest/general/test_interfaces.html
modules/libpref/init/all.js
--- a/b2g/app/b2g.js
+++ b/b2g/app/b2g.js
@@ -1076,8 +1076,11 @@ pref("dom.mozSettings.SettingsDB.verbose
 pref("dom.mozSettings.SettingsManager.verbose.enabled", false);
 pref("dom.mozSettings.SettingsRequestManager.verbose.enabled", false);
 pref("dom.mozSettings.SettingsService.verbose.enabled", false);
 
 // Controlling whether we want to allow forcing some Settings
 // IndexedDB transactions to be opened as readonly or keep everything as
 // readwrite.
 pref("dom.mozSettings.allowForceReadOnly", false);
+
+// RequestSync API is enabled by default on B2G.
+pref("dom.requestSync.enabled", true);
--- 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);
     },
 
--- a/dom/requestsync/tests/test_webidl.html
+++ b/dom/requestsync/tests/test_webidl.html
@@ -36,16 +36,20 @@
 
     test_sync();
 
     ok("registrations" in navigator.syncManager, "navigator.syncManager.registrations exists");
     runTests();
   }
 
   var tests = [
+    function() {
+      SpecialPowers.pushPrefEnv({"set": [["dom.requestSync.enabled", false]]}, runTests);
+    },
+
     test_no_interface,
 
     function() {
       SpecialPowers.pushPrefEnv({"set": [["dom.ignore_webidl_scope_checks", true]]}, runTests);
     },
 
     test_no_interface,
 
--- a/dom/tests/mochitest/general/test_interfaces.html
+++ b/dom/tests/mochitest/general/test_interfaces.html
@@ -1205,23 +1205,21 @@ var interfaceNamesInGlobalScope =
     "SVGUseElement",
 // IMPORTANT: Do not change this list without review from a DOM peer!
     "SVGViewElement",
 // IMPORTANT: Do not change this list without review from a DOM peer!
     "SVGZoomAndPan",
 // IMPORTANT: Do not change this list without review from a DOM peer!
     "SVGZoomEvent",
 // IMPORTANT: Do not change this list without review from a DOM peer!
-    {name: "RequestSyncManager", b2g: true, pref: "dom.requestSync.enabled", premission: "requestsync-manager" },
-// IMPORTANT: Do not change this list without review from a DOM peer!
-    {name: "RequestSyncScheduler", b2g: true, pref: "dom.requestSync.enabled" },
+    {name: "RequestSyncManager", b2g: true, pref: "dom.requestSync.enabled", permission: ["requestsync-manager"] },
 // IMPORTANT: Do not change this list without review from a DOM peer!
-    {name: "RequestSyncApp", b2g: true, pref: "dom.requestSync.enabled", premission: "requestsync-manager" },
+    {name: "RequestSyncApp", b2g: true, pref: "dom.requestSync.enabled", permission: ["requestsync-manager"] },
 // IMPORTANT: Do not change this list without review from a DOM peer!
-    {name: "RequestSyncTask", b2g: true, pref: "dom.requestSync.enabled", premission: "requestsync-manager" },
+    {name: "RequestSyncTask", b2g: true, pref: "dom.requestSync.enabled", permission: ["requestsync-manager"] },
 // IMPORTANT: Do not change this list without review from a DOM peer!
     {name: "Telephony", b2g: true, pref: "dom.telephony.enabled"},
 // IMPORTANT: Do not change this list without review from a DOM peer!
     {name: "TelephonyCall", b2g: true, pref: "dom.telephony.enabled"},
 // IMPORTANT: Do not change this list without review from a DOM peer!
     {name: "TelephonyCallGroup", b2g: true, pref: "dom.telephony.enabled"},
 // IMPORTANT: Do not change this list without review from a DOM peer!
     {name: "TelephonyCallId", b2g: true, pref: "dom.telephony.enabled"},
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -4433,8 +4433,11 @@ pref("dom.mozSettings.SettingsDB.verbose
 pref("dom.mozSettings.SettingsManager.verbose.enabled", false);
 pref("dom.mozSettings.SettingsRequestManager.verbose.enabled", false);
 pref("dom.mozSettings.SettingsService.verbose.enabled", false);
 
 // Controlling whether we want to allow forcing some Settings
 // IndexedDB transactions to be opened as readonly or keep everything as
 // readwrite.
 pref("dom.mozSettings.allowForceReadOnly", false);
+
+// RequestSync API is disabled by default.
+pref("dom.requestSync.enabled", false);