Bill, have you started working on this already? Bug 1254204 gets us most of the way there, so I've been thinking about doing the rest.

Nope. Please feel free to take it. Thanks!

Thanks

Comment on attachment 8809239 [details] Bug 1305217: Part 1 - Run webRequest listeners asynchronously. https://reviewboard.mozilla.org/r/91824/#review91870 ::: toolkit/components/extensions/ExtensionChild.jsm:507 (Diff revision 1) > > addListener(listener, args) { > - let set = this.childApiManager.listeners.get(this.path); > - if (!set) { > - set = new Set(); > - this.childApiManager.listeners.set(this.path, set); > + let map = this.childApiManager.listeners.get(this.path); > + > + if (map.listeners.has(listener)) { > + // TODO: Called with different args? Add a reference to https://bugzilla.mozilla.org/show_bug.cgi?id=1305216 ("Use ChildAPIManager for webNavigation API") (comment 2 mentions the need for recognizing different filters in addListener) ::: toolkit/components/extensions/ExtensionChild.jsm:565 (Diff revision 1) > // delegated to the ParentAPIManager. > this.localApis = localApis; > > this.id = `${context.extension.id}.${context.contextId}`; > > - messageManager.addMessageListener("API:RunListener", this); > + MessageChannel.addListener(messageManager, "API:RunListener", this); This change adds overhead to all method calls. If a listener for which the API does not expect a response returns a response, then an error could occur. How about keeping the existing "API:RunListener", and add a new MessageChannel-based handler when an API explicitly needs to handle responses?

Comment on attachment 8809239 [details] Bug 1305217: Part 1 - Run webRequest listeners asynchronously. https://reviewboard.mozilla.org/r/91824/#review91870 > This change adds overhead to all method calls. > If a listener for which the API does not expect a response returns a response, then an error could occur. > > How about keeping the existing "API:RunListener", and add a new MessageChannel-based handler when an API explicitly needs to handle responses? I'll deal with optimizing it if and when it becomes an issue. For now, I don't think it's a major concern.

Comment on attachment 8809240 [details] Bug 1305217: Part 2 - Run webNavigation listeners asynchronously. https://reviewboard.mozilla.org/r/91826/#review92166

Comment on attachment 8809239 [details] Bug 1305217: Part 1 - Run webRequest listeners asynchronously. https://reviewboard.mozilla.org/r/91824/#review92448 I'm somewhat bothered by the redirect issue possibly allowing onHeadersReceived, but dont see a way around that, and I'm not convinced its a real problem. Otherwise good to go.

Comment on attachment 8809239 [details] Bug 1305217: Part 1 - Run webRequest listeners asynchronously. https://reviewboard.mozilla.org/r/91824/#review92448 To be clear, that only happens when the request is redirected after the connection has been initiated (i.e., from onBeforeSendHeaders). Redirects from onBeforeRequest should still prevent a connection from being established or subsequent listeners from firing.

https://hg.mozilla.org/integration/mozilla-inbound/rev/4efa832fc852bb0499f8518c458ff052bede560e Bug 1305217: Part 1 - Run webRequest listeners asynchronously. r=mixedpuppy https://hg.mozilla.org/integration/mozilla-inbound/rev/05c206c4311bc4d6e727f89243aea2b5139fca61 Bug 1305217: Part 2 - Run webNavigation listeners asynchronously. r=mixedpuppy

Backed out bug 1316914, bug 1316784 and bug 1305217 for frequent timeouts in test_ext_webrequest_upload.html: Bug 1305217: https://hg.mozilla.org/integration/mozilla-inbound/rev/217ae1d5285328611de99e2e48042b19751dbb54 https://hg.mozilla.org/integration/mozilla-inbound/rev/6cbde9c5e058999ba95319391b59a7d67649ca53 Bug 1316784: https://hg.mozilla.org/integration/mozilla-inbound/rev/9381a00d19f81c21114f9d24cafee791fa0d9dbe Bug 1316914: https://hg.mozilla.org/integration/mozilla-inbound/rev/ddec56f2c469512ba99e424540f6f77a447fdea0 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=39104153&repo=mozilla-inbound 21:16:58 INFO - TEST-PASS | toolkit/components/extensions/test/mochitest/test_ext_webrequest_upload.html | Binary upload size matches - Expected: 16, Actual: 16 21:16:58 INFO - onCompleted 32 http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/file_WebRequest_page3.html?trigger=form&upload=16+bytes&enctype=multipart%2Fform-data&xhr=1 21:16:58 INFO - onBeforeRequest 33 http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/file_WebRequest_page3.html?trigger=form&upload=%7B%22textInput%22%3A%5B%22value1%22%2C%22value2%22%5D%7D&enctype=application%2Fx-www-form-urlencoded 21:16:58 INFO - onUpload http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/file_WebRequest_page3.html?trigger=form&upload=%7B%22textInput%22%3A%5B%22value1%22%2C%22value2%22%5D%7D&enctype=application%2Fx-www-form-urlencoded {"formData":{"textInput":["value1","value2"]}} 21:16:58 INFO - TEST-PASS | toolkit/components/extensions/test/mochitest/test_ext_webrequest_upload.html | Intercepted upload http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/file_WebRequest_page3.html?trigger=form&upload=%7B%22textInput%22%3A%5B%22value1%22%2C%22value2%22%5D%7D&enctype=application%2Fx-www-form-urlencoded #33 {"textInput":["value1","value2"]} have a requestBody 21:16:58 INFO - TEST-PASS | toolkit/components/extensions/test/mochitest/test_ext_webrequest_upload.html | Upload http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/file_WebRequest_page3.html?trigger=form&upload=%7B%22textInput%22%3A%5B%22value1%22%2C%22value2%22%5D%7D&enctype=application%2Fx-www-form-urlencoded #33 matches form data. - Expected: {"textInput":["value1","value2"]}, Actual: {"textInput":["value1","value2"]} 21:16:58 INFO - Buffered messages finished 21:16:58 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/extensions/test/mochitest/test_ext_webrequest_upload.html | Test timed out. 21:16:58 INFO - reportError@SimpleTest/TestRunner.js:114:7 21:16:58 INFO - TestRunner._checkForHangs@SimpleTest/TestRunner.js:135:7

Also backed out the change without bug to make ESlint happy: https://hg.mozilla.org/integration/mozilla-inbound/rev/5e2ec37ba69ffa5c78be6651815362d7c196c5f4 Message from ESlint was: TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/toolkit/components/extensions/ExtensionChild.jsm:536:5 | 'nextId' is assigned a value but never used. (no-unused-vars)

https://hg.mozilla.org/integration/mozilla-inbound/rev/2b8b797d3583e2f4d968f64bfbde8e5dcf19bf68 Bug 1305217: Part 1 - Run webRequest listeners asynchronously. r=mixedpuppy https://hg.mozilla.org/integration/mozilla-inbound/rev/c8980badcd5a40d474e31b41e84ba861c8301440 Bug 1305217: Part 2 - Run webNavigation listeners asynchronously. r=mixedpuppy

https://hg.mozilla.org/mozilla-central/rev/2b8b797d3583 https://hg.mozilla.org/mozilla-central/rev/c8980badcd5a