Bug 1248497 – Add promise support to the sendMessage APIs. r?billm draft
authorKris Maglione <maglione.k@gmail.com>
Mon, 15 Feb 2016 17:37:19 -0800
changeset 335113 cd8828f23123c9565d72024f86c27ecec4164c66
parent 335069 36d6bc68fe0f21d87b306c7712e939d8ae537b88
child 515079 5d6f24d001ee533ba3d5e88b040382f232e7816e
push id11727
push usermaglione.k@gmail.com
push dateSat, 27 Feb 2016 01:22:02 +0000
reviewersbillm
bugs1248497
milestone47.0a1
Bug 1248497 – Add promise support to the sendMessage APIs. r?billm MozReview-Commit-ID: AZH9LUq8kGr
browser/components/extensions/test/browser/browser_ext_tabs_sendMessage.js
toolkit/components/extensions/ExtensionContent.jsm
toolkit/components/extensions/ExtensionUtils.jsm
--- a/browser/components/extensions/test/browser/browser_ext_tabs_sendMessage.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_sendMessage.js
@@ -1,12 +1,101 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
+add_task(function* tabsSendMessageReply() {
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "permissions": ["tabs"],
+
+      "content_scripts": [{
+        "matches": ["http://example.com/"],
+        "js": ["content-script.js"],
+        "run_at": "document_start",
+      }],
+    },
+
+    background: function() {
+      let promiseResponse = new Promise(resolve => {
+        browser.runtime.onMessage.addListener((msg, sender, respond) => {
+          if (msg == "content-script-ready") {
+            let tabId = sender.tab.id;
+
+            browser.tabs.sendMessage(tabId, "respond-never", response => {
+              browser.test.fail("Got unexpected resposne callback");
+              browser.test.notifyFail("sendMessage");
+            });
+
+            Promise.all([
+              promiseResponse,
+              browser.tabs.sendMessage(tabId, "respond-now"),
+              new Promise(resolve => browser.tabs.sendMessage(tabId, "respond-soon", resolve)),
+              browser.tabs.sendMessage(tabId, "respond-promise"),
+              browser.tabs.sendMessage(tabId, "respond-never"),
+              browser.tabs.sendMessage(tabId, "respond-error").catch(error => Promise.resolve({error})),
+              browser.tabs.sendMessage(tabId, "throw-error").catch(error => Promise.resolve({error})),
+            ]).then(([response, respondNow, respondSoon, respondPromise, respondNever, respondError, throwError]) => {
+              browser.test.assertEq("expected-response", response, "Content script got the expected response");
+
+              browser.test.assertEq("respond-now", respondNow, "Got the expected immediate response");
+              browser.test.assertEq("respond-soon", respondSoon, "Got the expected delayed response");
+              browser.test.assertEq("respond-promise", respondPromise, "Got the expected promise response");
+              browser.test.assertEq(undefined, respondNever, "Got the expected no-response resolution");
+
+              browser.test.assertEq("respond-error", respondError.error.message, "Got the expected error response");
+              browser.test.assertEq("throw-error", throwError.error.message, "Got the expected thrown error response");
+
+              return browser.tabs.remove(tabId);
+            }).then(() => {
+              browser.test.notifyPass("sendMessage");
+            });
+
+            return Promise.resolve("expected-response");
+          } else if (msg[0] == "got-response") {
+            resolve(msg[1]);
+          }
+        });
+      });
+
+      browser.tabs.create({url: "http://example.com/"});
+    },
+
+    files: {
+      "content-script.js": function() {
+        browser.runtime.onMessage.addListener((msg, sender, respond) => {
+          if (msg == "respond-now") {
+            respond(msg);
+          } else if (msg == "respond-soon") {
+            setTimeout(() => { respond(msg); }, 0);
+            return true;
+          } else if (msg == "respond-promise") {
+            return Promise.resolve(msg);
+          } else if (msg == "respond-never") {
+            return;
+          } else if (msg == "respond-error") {
+            return Promise.reject(new Error(msg));
+          } else if (msg == "throw-error") {
+            throw new Error(msg);
+          }
+        });
+        browser.runtime.sendMessage("content-script-ready").then(response => {
+          browser.runtime.sendMessage(["got-response", response]);
+        });
+      },
+    },
+  });
+
+  yield extension.startup();
+
+  yield extension.awaitFinish("sendMessage");
+
+  yield extension.unload();
+});
+
 add_task(function* tabsSendMessageNoExceptionOnNonExistentTab() {
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       "permissions": ["tabs"],
     },
 
     background: function() {
       browser.tabs.create({url: "about:robots"}, tab => {
--- a/toolkit/components/extensions/ExtensionContent.jsm
+++ b/toolkit/components/extensions/ExtensionContent.jsm
@@ -94,17 +94,17 @@ var api = context => {
           message = args[0];
         } else if (args.length == 2) {
           [message, responseCallback] = args;
         } else {
           [extensionId, message, options, responseCallback] = args;
         }
 
         let recipient = extensionId ? {extensionId} : {extensionId: context.extensionId};
-        context.messenger.sendMessage(context.messageManager, message, recipient, responseCallback);
+        return context.messenger.sendMessage(context.messageManager, message, recipient, responseCallback);
       },
     },
 
     extension: {
       getURL: function(url) {
         return context.extension.baseURI.resolve(url);
       },
 
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -860,66 +860,85 @@ function Messenger(context, broker, send
 
 Messenger.prototype = {
   sendMessage(messageManager, msg, recipient, responseCallback) {
     let id = this.broker.makeId();
     let replyName = `Extension:Reply-${id}`;
     recipient.messageId = id;
     this.broker.sendMessage(messageManager, "message", msg, this.sender, recipient);
 
-    let onClose;
-    let listener = ({data: response}) => {
-      messageManager.removeMessageListener(replyName, listener);
-      this.context.forgetOnClose(onClose);
+    let promise = new Promise((resolve, reject) => {
+      let onClose;
+      let listener = ({data: response}) => {
+        messageManager.removeMessageListener(replyName, listener);
+        this.context.forgetOnClose(onClose);
 
-      if (response.gotData) {
-        // TODO: Handle failure to connect to the extension?
-        runSafe(this.context, responseCallback, response.data);
-      }
-    };
-    onClose = {
-      close() {
-        messageManager.removeMessageListener(replyName, listener);
-      },
-    };
-    if (responseCallback) {
+        if (response.gotData) {
+          resolve(response.data);
+        } else if (response.error) {
+          reject(response.error);
+        } else if (!responseCallback) {
+          // As a special case, we don't call the callback variant if we
+          // receive no response, but the promise needs to resolve or
+          // reject in either case.
+          resolve();
+        }
+      };
+      onClose = {
+        close() {
+          messageManager.removeMessageListener(replyName, listener);
+        },
+      };
+
       messageManager.addMessageListener(replyName, listener);
       this.context.callOnClose(onClose);
-    }
+    });
+
+    return this.context.wrapPromise(promise, responseCallback);
   },
 
   onMessage(name) {
     return new SingletonEventManager(this.context, name, callback => {
       let listener = (type, target, message, sender, recipient) => {
         message = Cu.cloneInto(message, this.context.cloneScope);
         if (this.delegate) {
           this.delegate.getSender(this.context, target, sender);
         }
         sender = Cu.cloneInto(sender, this.context.cloneScope);
 
         let mm = getMessageManager(target);
         let replyName = `Extension:Reply-${recipient.messageId}`;
 
-        let valid = true, sent = false;
-        let sendResponse = data => {
-          if (!valid) {
-            return;
+        new Promise((resolve, reject) => {
+          let sendResponse = Cu.exportFunction(resolve, this.context.cloneScope);
+
+          // Note: We intentionally do not use runSafe here so that any
+          // errors are propagated to the message sender.
+          let result = callback(message, sender, sendResponse);
+          if (result instanceof Promise) {
+            resolve(result);
+          } else if (result !== true) {
+            reject();
           }
-          sent = true;
-          mm.sendAsyncMessage(replyName, {data, gotData: true});
-        };
-        sendResponse = Cu.exportFunction(sendResponse, this.context.cloneScope);
-
-        let result = runSafeSyncWithoutClone(callback, message, sender, sendResponse);
-        if (result !== true) {
-          valid = false;
-          if (!sent) {
-            mm.sendAsyncMessage(replyName, {gotData: false});
-          }
-        }
+        }).then(
+          data => {
+            mm.sendAsyncMessage(replyName, {data, gotData: true});
+          },
+          error => {
+            if (error) {
+              // The result needs to be structured-clonable, which
+              // ordinary Error objects are not.
+              try {
+                error = {message: String(error.message), stack: String(error.stack)};
+              } catch (e) {
+                error = {message: String(error)};
+              }
+            }
+            mm.sendAsyncMessage(replyName, {error, gotData: false});
+          });
       };
 
       this.broker.addListener("message", listener, this.filter);
       return () => {
         this.broker.removeListener("message", listener);
       };
     }).api();
   },