w3c / webextensions

Charter and administrivia for the WebExtensions Community Group (WECG)
Other
579 stars 50 forks source link

Support Promise as return value from webRequest.onAuthRequired #490

Open Rob--W opened 7 months ago

Rob--W commented 7 months ago

The webRequest.onAuthRequired event is the only webRequest event that can block a request asynchronously in Chrome (In Firefox, all webRequest events can be async; Safari does not support any blocking webRequest API).

Firefox and Chrome have an interoperable API when an extension wants to return the result synchronously from the event handler function of webRequest.onAuthRequired. But for asynchronous results, there is currently a large discrepancy, as I elaborated at https://github.com/mdn/content/pull/30188#pullrequestreview-1729828656 (The permission aspect is out-of-scope for this issue, that's part of #264).

Chrome and Firefox support this syntax to handle an event synchronously:

// In Chrome: use chrome. instead of browser.
browser.webRequest.onAuthRequired.addListener(
  function (details) {
    return { /* BlockingResponse here*/ };
  },
  { urls: ["*://example.com/*"] },
  ["blocking"]
);

Firefox also supports asynchronous results through a Promise. In the above example, replace "function (details) {" with "async function (details) {`, and it would be effectively the same as the following. Note that the event registration looks identical to before:

browser.webRequest.onAuthRequired.addListener(
 function () {
    // Does NOT work in Chrome!
    return new Promise(resolve => {
      resolve({ /* BlockingResponse here*/ });
    });
  },
  { urls: ["*://example.com/*"] },
  ["blocking"]
);

Chrome does not support Promise as return value. Instead, it requires a different way to register the listener, and passing the result via a callback:

chrome.webRequest.onAuthRequired.addListener(
  function (details, asyncCallback) { // listener receives "asyncCallback" parameter.
    asyncCallback({ /* BlockingResponse here*/ });
    // Note: any return value is ignored.
  },
  { urls: ["*://example.com/*"] },
  ["asyncBlocking"] // "asyncBlocking" instead of "blocking"
);

I propose to update Chrome's implementation to support Promise, with the following remarks:

Rob--W commented 7 months ago

FYI: Another example of an event handler that accepts a Promise in Firefox but requires a callback in Chrome is runtime.onMessage. Chrome expressed being in favor of accepting a Promise there at https://github.com/w3c/webextensions/issues/338.

Rob--W commented 6 months ago

I created an issue on Chromium's issue tracker for this, at https://bugs.chromium.org/p/chromium/issues/detail?id=1510405

Rob--W commented 3 months ago

For compatibility, Firefox is considering to also implement asyncBlocking + asyncCallback: https://bugzilla.mozilla.org/show_bug.cgi?id=1889897

@oliverdunk At the end of the initial issue, I sketched a potential way to support Promises in onAuthRequired with minimal impact on backward compatibility. Could you confirm which direction Chrome would prefer here? Ideally Firefox and Chrome should behave consistently here.

oliverdunk commented 2 months ago

I spoke to the Chrome team about this and also had some chats with Rob. We are not keen on using asyncBlocking for this as it would be a breaking change to start giving meaning to the return value, especially in cases where you are using async functions that implicitly return a promise. I liked adding promise return support to blocking, but there is some concern about the implementation complexity. We are currently leaning towards the new asyncBlockingPromise value mentioned at the bottom of the initial issue but open to more discussion.