Bug 1389968 - Reject sendMessage() promise when response handle gets GCd
MozReview-Commit-ID: C2g3VSWYKuz
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -19,16 +19,19 @@ this.EXPORTED_SYMBOLS = ["ExtensionChild
const Ci = Components.interfaces;
const Cc = Components.classes;
const Cu = Components.utils;
const Cr = Components.results;
Cu.import("resource://gre/modules/Services.jsm");
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+XPCOMUtils.defineLazyServiceGetter(this, "finalizationService",
+ "@mozilla.org/toolkit/finalizationwitness;1", "nsIFinalizationWitnessService");
+
XPCOMUtils.defineLazyModuleGetters(this, {
ExtensionContent: "resource://gre/modules/ExtensionContent.jsm",
MessageChannel: "resource://gre/modules/MessageChannel.jsm",
NativeApp: "resource://gre/modules/NativeMessaging.jsm",
PromiseUtils: "resource://gre/modules/PromiseUtils.jsm",
});
Cu.import("resource://gre/modules/ExtensionCommon.jsm");
@@ -70,16 +73,43 @@ function injectAPI(source, dest) {
injectAPI(desc.value, obj);
} else {
Object.defineProperty(dest, prop, desc);
}
}
}
/**
+ * A finalization witness helper that wraps a sendMessage response and
+ * guarantees to either get the promise resolved, or rejected when the
+ * wrapped promise goes out of scope.
+ *
+ * Holding a reference to a returned StrongPromise doesn't prevent the
+ * wrapped promise from being garbage collected.
+ */
+const StrongPromise = {
+ wrap(promise, id) {
+ return new Promise((resolve, reject) => {
+ const witness = finalizationService.make("extensions-sendMessage-witness", id);
+ promise.then(value => {
+ witness.forget();
+ resolve(value);
+ }, error => {
+ witness.forget();
+ reject(error);
+ });
+ });
+ },
+ observe(subject, topic, id) {
+ MessageChannel.abortChannel(id, {message: "Response handle went out of scope"});
+ },
+};
+Services.obs.addObserver(StrongPromise, "extensions-sendMessage-witness");
+
+/**
* Abstraction for a Port object in the extension API.
*
* @param {BaseContext} context The context that owns this port.
* @param {nsIMessageSender} senderMM The message manager to send messages to.
* @param {Array<nsIMessageListenerManager>} receiverMMs Message managers to
* listen on.
* @param {string} name Arbitrary port name as defined by the addon.
* @param {string} id An ID that uniquely identifies this port's channel.
@@ -354,17 +384,17 @@ class Messenger {
messageFilterStrict: this.filter,
filterMessage: (sender, recipient) => {
// Ignore the message if it was sent by this Messenger.
return (sender.contextId !== this.context.contextId &&
filter(sender, recipient));
},
- receiveMessage: ({target, data: holder, sender, recipient}) => {
+ receiveMessage: ({target, data: holder, sender, recipient, channelId}) => {
if (!this.context.active) {
return;
}
let sendResponse;
let response = undefined;
let promise = new Promise(resolve => {
sendResponse = value => {
@@ -376,19 +406,19 @@ class Messenger {
let message = holder.deserialize(this.context.cloneScope);
sender = Cu.cloneInto(sender, this.context.cloneScope);
sendResponse = Cu.exportFunction(sendResponse, this.context.cloneScope);
// Note: We intentionally do not use runSafe here so that any
// errors are propagated to the message sender.
let result = fire.raw(message, sender, sendResponse);
if (result instanceof this.context.cloneScope.Promise) {
- return result;
+ return StrongPromise.wrap(result, channelId);
} else if (result === true) {
- return promise;
+ return StrongPromise.wrap(promise, channelId);
}
return response;
},
};
MessageChannel.addListener(this.messageManagers, "Extension:Message", listener);
return () => {
MessageChannel.removeListener(this.messageManagers, "Extension:Message", listener);
--- a/toolkit/components/extensions/MessageChannel.jsm
+++ b/toolkit/components/extensions/MessageChannel.jsm
@@ -639,16 +639,17 @@ this.MessageChannel = {
}
let target = new MessageManagerProxy(data.target);
let deferred = {
sender: data.sender,
messageManager: target,
channelId: data.channelId,
+ respondingSide: true,
};
deferred.promise = new Promise((resolve, reject) => {
deferred.reject = reject;
this._callHandlers(handlers, data).then(resolve, reject);
}).then(
value => {
let response = {
@@ -763,16 +764,35 @@ this.MessageChannel = {
let cleanup = () => {
this.pendingResponses.delete(deferred);
};
this.pendingResponses.add(deferred);
deferred.promise.then(cleanup, cleanup);
},
/**
+ * Aborts pending message response for the specific channel.
+ *
+ * @param {string} channelId
+ * A string for channelId of the response.
+ * @param {object} reason
+ * An object describing the reason the response was aborted.
+ * Will be passed to the promise rejection handler of the aborted
+ * response.
+ */
+ abortChannel(channelId, reason) {
+ for (let response of this.pendingResponses) {
+ if (channelId === response.channelId && response.respondingSide) {
+ this.pendingResponses.delete(response);
+ response.reject(reason);
+ }
+ }
+ },
+
+ /**
* Aborts any pending message responses to senders matching the given
* filter.
*
* @param {object} sender
* The object on which to filter senders, as determined by
* `matchesFilter`.
* @param {object} [reason]
* An optional object describing the reason the response was aborted.
--- a/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage.js
@@ -104,8 +104,108 @@ add_task(async function tabsSendMessageB
"extensionpage.html": `<!DOCTYPE html><meta charset="utf-8"><script src="senderScript.js"></script>`,
},
});
await extension.startup();
await extension.awaitFinish("sendBlob");
await extension.unload();
});
+
+add_task(async function sendMessageResponseGC() {
+ function background() {
+ let savedResolve, savedRespond;
+
+ browser.runtime.onMessage.addListener((msg, _, respond) => {
+ browser.test.log(`Got request: ${msg}`);
+ switch (msg) {
+ case "ping":
+ respond("pong");
+ return;
+
+ case "promise-save":
+ return new Promise(resolve => {
+ savedResolve = resolve;
+ });
+ case "promise-resolve":
+ savedResolve("saved-resolve");
+ return;
+ case "promise-never":
+ return new Promise(r => {});
+
+ case "callback-save":
+ savedRespond = respond;
+ return true;
+ case "callback-call":
+ savedRespond("saved-respond");
+ return;
+ case "callback-never":
+ return true;
+ }
+ });
+
+ const frame = document.createElement("iframe");
+ frame.src = "page.html";
+ document.body.appendChild(frame);
+ }
+
+ function page() {
+ browser.test.onMessage.addListener(msg => {
+ browser.runtime.sendMessage(msg)
+ .then(response => {
+ if (response) {
+ browser.test.log(`Got response: ${response}`);
+ browser.test.sendMessage(response);
+ }
+ }, error => {
+ browser.test.assertEq(error.message,
+ "Response handle went out of scope",
+ "The promise rejected with the correct error");
+ browser.test.sendMessage("rejected");
+ }
+ );
+ });
+ browser.test.sendMessage("ready");
+ }
+
+ let extension = ExtensionTestUtils.loadExtension({
+ background,
+ files: {
+ "page.html": "<!DOCTYPE html><meta charset=utf-8><script src=page.js></script>",
+ "page.js": page,
+ },
+ });
+
+ await extension.startup();
+ await extension.awaitMessage("ready");
+
+ // Setup long-running tasks before GC.
+ extension.sendMessage("promise-save");
+ extension.sendMessage("callback-save");
+
+ // Test returning a Promise that can never resolve.
+ extension.sendMessage("promise-never");
+
+ extension.sendMessage("ping");
+ await extension.awaitMessage("pong");
+
+ Services.ppmm.loadProcessScript("data:,Components.utils.forceGC()", false);
+ await extension.awaitMessage("rejected");
+
+ // Test returning `true` without holding the response handle.
+ extension.sendMessage("callback-never");
+
+ extension.sendMessage("ping");
+ await extension.awaitMessage("pong");
+
+ Services.ppmm.loadProcessScript("data:,Components.utils.forceGC()", false);
+ await extension.awaitMessage("rejected");
+
+ // Test that promises from long-running tasks didn't get GCd.
+ extension.sendMessage("promise-resolve");
+ await extension.awaitMessage("saved-resolve");
+
+ extension.sendMessage("callback-call");
+ await extension.awaitMessage("saved-respond");
+
+ ok("Long running tasks responded");
+ await extension.unload();
+});