Support sendResponse callback in onMessage

[Rob--W]

Now onMessage behaves as documented in MDN, and like Firefox.

The only thing I was not completely sure about is what to do when return true is used. It can be resolved in two ways:

1. Calling sendResponse with true.
2. Returning true in the real onMessage handler and allowing sendResponse to be called asynchronously.

I follow Firefox's implementation and do the latter - http://searchfox.org/mozilla-central/rev/848c29538ab007fb95dc6cff194f0e3d3809613d/toolkit/components/extensions/ExtensionChild.jsm#363

This fixes #16.

[nklein]

I futzed around for a bit before finding a solution to this same problem. The above solution to the problem works well for me, too. So, I've just started using this branch. I look forward to seeing it merged.

[slhck]

Updated, I now use the dist version of the polyfill from your forked repo.

I used to implement message passing like this:

chrome.tabs.sendMessage(id, { event : "foo" }, {}, (response) => {
    // do something
});

// handler
chrome.runtime.onMessage.addListener(function(request, sender, sendResponse) {
    sendResponse("bar");
});

This worked fine. However, when I now follow the approach from MDN as shown here:

// handler in content script
browser.runtime.onMessage.addListener(function(request, sender, sendResponse) {
    return Promise.resolve("bar");
});

When I call this from the background script console, I don't get any result:

browser.tabs.sendMessage(418, { event: "foo" }).then(response => { console.log(response) }, error => { console.error(error) }).catch(error => console.error(error))

Returns:

Promise {[[PromiseStatus]]: "pending", [[PromiseValue]]: undefined}

Shouldn't this print the value "bar" on the console? When I break in the polyfill code that you changed, this is where it returns:

I'm not calling sendResponse since it's not a synchronous call and I want to return via a Promise…

[slhck]

@nklein How did you implement this? Could you perhaps post a sample so I can double-check what's going wrong with my code. Thanks!

[nklein]

As I understand the documentation on the Mozilla Developer Network site, the listener is supposed to return 'true' if it has called or will call sendResponse(). So, I would have expected your listener code to be:

sendResponse("bar");
return true;

Or, if it needed to be asynchronous, then:

var promise = new Promise( (resolve, reject) => {
    var result = do_something_long();
    resolve( "bar" );
} );
return true;

Or if you are doing something explicitly asynchronous, then:

doSomethingThatReturnsPromise.then( sendResponse, (err) => {
   sendResponse( { 'err': err } );
});
return true;

And, actually, it appears that I changed the return result; at line 834 in your screenshot of the patch to return true;

[slhck]

Thanks! Actually, using the patch from @Rob--W – without changing his code – I just do this:

return Promise.resolve("bar");

Which gives me the correct response, but a few seconds (like 10–20?) later, it shows errors in my console:

When I instead do:

// background page
let ret = browser.tabs.sendMessage(id, "foo");
ret.then((response) => {
  // handle response
}, (error) => {
  // handle error
}).catch((error) => {
  // handle other errors
});

// content script
browser.runtime.onMessage.addListener((request, sender, sendResponse) => {
  sendResponse("bar");
  return(true);
}

I get an error inside then(): The message port closed before a reponse was received.

[rpl]

Hi @Rob--W,
As I mentioned in #16 (#16 (comment)), I discussed about this issue and the related pull requests with @kmaglione, and it looks like the sendResponse callback has not been included in the polyfill implementation on purpose because is going to be removed from the W3C specs.

Based on the above discussion, we agreed to mark the above issue as needs: docs (to fix the ambiguity in our MDN docs) and state: wontfix (to state that the deprecated sendResponse callback will not be implemented).