From f580f63a40610d79b87620e7ee8e33ccdcf7b057 Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Mon, 5 Jan 2015 13:42:09 +0100 Subject: [PATCH] Bug 1018320 - RequestSync API - patch 7 - reject promise when an exception is thrown, r=fabrice --- b2g/app/b2g.js | 3 ++ dom/messages/SystemMessageInternal.js | 31 ++++++++++++++----- dom/messages/SystemMessageManager.js | 19 +++++++++--- dom/requestsync/RequestSyncService.jsm | 1 + dom/requestsync/tests/test_promise_app.html | 28 ++++++++++++----- dom/requestsync/tests/test_webidl.html | 4 +++ .../mochitest/general/test_interfaces.html | 8 ++--- modules/libpref/init/all.js | 3 ++ 8 files changed, 72 insertions(+), 25 deletions(-) diff --git a/b2g/app/b2g.js b/b2g/app/b2g.js index dee771455973..37af59e97d1b 100644 --- a/b2g/app/b2g.js +++ b/b2g/app/b2g.js @@ -1081,3 +1081,6 @@ pref("dom.mozSettings.SettingsService.verbose.enabled", false); // 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); diff --git a/dom/messages/SystemMessageInternal.js b/dom/messages/SystemMessageInternal.js index 0f4b7feb79f9..bde7b8926d98 100644 --- a/dom/messages/SystemMessageInternal.js +++ b/dom/messages/SystemMessageInternal.js @@ -470,7 +470,7 @@ SystemMessageInternal.prototype = { true, null); - this._rejectPendingPromises(manifestURL); + this._rejectPendingPromisesFromManifestURL(manifestURL); } break; } @@ -482,7 +482,7 @@ SystemMessageInternal.prototype = { msg.manifestURL, false, msg.pageURL); - this._rejectPendingPromises(msg.manifestURL); + this._rejectPendingPromisesFromManifestURL(msg.manifestURL); break; } case "SystemMessageManager:GetPendingMessages": @@ -554,10 +554,14 @@ SystemMessageInternal.prototype = { { 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. @@ -644,7 +648,7 @@ SystemMessageInternal.prototype = { } } - this._rejectPendingPromises(manifestURL); + this._rejectPendingPromisesFromManifestURL(manifestURL); debug("Finish updating registered pages for an uninstalled app."); break; @@ -789,7 +793,20 @@ SystemMessageInternal.prototype = { } }, - _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(); diff --git a/dom/messages/SystemMessageManager.js b/dom/messages/SystemMessageManager.js index b9740a52d14f..b2864e757ff2 100644 --- a/dom/messages/SystemMessageManager.js +++ b/dom/messages/SystemMessageManager.js @@ -95,9 +95,14 @@ SystemMessageManager.prototype = { } } - 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; @@ -111,7 +116,8 @@ SystemMessageManager.prototype = { { type: aType, manifestURL: self._manifestURL, pageURL: self._pageURL, - msgID: aMessageID }); + msgID: aMessageID, + rejected: rejectPromise }); } if (!this._promise) { @@ -119,7 +125,10 @@ SystemMessageManager.prototype = { 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; } diff --git a/dom/requestsync/RequestSyncService.jsm b/dom/requestsync/RequestSyncService.jsm index d1a705db4c4b..66770d5b620d 100644 --- a/dom/requestsync/RequestSyncService.jsm +++ b/dom/requestsync/RequestSyncService.jsm @@ -678,6 +678,7 @@ this.RequestSyncService = { }, timeout, Ci.nsITimer.TYPE_ONE_SHOT); // Sending the message. + debug("Sending message."); let promise = systemMessenger.sendMessage('request-sync', this.createPartialTaskObject(aObj.data), diff --git a/dom/requestsync/tests/test_promise_app.html b/dom/requestsync/tests/test_promise_app.html index 9039c8e0fba3..26c2a24e4f82 100644 --- a/dom/requestsync/tests/test_promise_app.html +++ b/dom/requestsync/tests/test_promise_app.html @@ -17,23 +17,35 @@ 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(); } } @@ -47,7 +59,7 @@ } function test_register_foobar() { - navigator.sync.register('foobar', { minInterval: 5, + navigator.sync.register('foobar', { minInterval: 2, oneShot: false, data: 42, wifiOnly: false, @@ -59,7 +71,7 @@ } function test_register_pending() { - navigator.sync.register('pending', { minInterval: 6, + navigator.sync.register('pending', { minInterval: 2, oneShot: false, data: 'hello world!', wifiOnly: false, @@ -94,7 +106,7 @@ 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); }, diff --git a/dom/requestsync/tests/test_webidl.html b/dom/requestsync/tests/test_webidl.html index 46bed4590119..45e8616a2695 100644 --- a/dom/requestsync/tests/test_webidl.html +++ b/dom/requestsync/tests/test_webidl.html @@ -41,6 +41,10 @@ } var tests = [ + function() { + SpecialPowers.pushPrefEnv({"set": [["dom.requestSync.enabled", false]]}, runTests); + }, + test_no_interface, function() { diff --git a/dom/tests/mochitest/general/test_interfaces.html b/dom/tests/mochitest/general/test_interfaces.html index 2586c34a1a88..daca03056b69 100644 --- a/dom/tests/mochitest/general/test_interfaces.html +++ b/dom/tests/mochitest/general/test_interfaces.html @@ -1210,13 +1210,11 @@ var interfaceNamesInGlobalScope = // 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" }, + {name: "RequestSyncManager", b2g: true, pref: "dom.requestSync.enabled", permission: ["requestsync-manager"] }, // IMPORTANT: Do not change this list without review from a DOM peer! - {name: "RequestSyncScheduler", b2g: true, pref: "dom.requestSync.enabled" }, + {name: "RequestSyncApp", 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" }, -// 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! diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 82ca623d3893..4a285bcc3cec 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -4438,3 +4438,6 @@ pref("dom.mozSettings.SettingsService.verbose.enabled", false); // 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);