Add speculative connection type for webrequest and proxy

RESOLVED FIXED in Firefox 63

Status

P1
normal
RESOLVED FIXED
8 months ago
6 months ago

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

Tracking

({dev-doc-complete})

unspecified
mozilla63
dev-doc-complete
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

8 months ago
Bug 1448500 added a type for speculative connections, but there is no way to specify that in the filter types.  If for example, you only want to filter what requests you proxy, you may also need to proxy speculative connections for them else the requests may fail.  In that case, you need to have e.g. types: ["xmlhttprequest", "speculative"].

For docs, speculative connections are described here: https://searchfox.org/mozilla-central/rev/033d45ca70ff32acf04286244644d19308c359d5/netwerk/base/nsISpeculativeConnect.idl#13
Comment hidden (mozreview-request)
(Assignee)

Updated

8 months ago
Assignee: nobody → mixedpuppy
Priority: -- → P1

Comment 2

8 months ago
mozreview-review
Comment on attachment 8996076 [details]
Bug 1479565 - add speculative filter type for webRequest and proxy API,

https://reviewboard.mozilla.org/r/260328/#review267404

::: toolkit/components/extensions/test/xpcshell/test_ext_proxy_startup.js:58
(Diff revision 1)
>  add_task(async function test_proxy_startup() {
>    await promiseStartupManager();
>  
>    function background(proxyInfo) {
>      browser.proxy.onRequest.addListener(details => {
> -      // ignore speculative requests
> +      // Ignore speculative requests, but we need to proxy them for this to work.

I don't really understand what's going on here.
Based on the comment, could the code become `if (details.type != "speculative") ...` ?
(Assignee)

Comment 3

8 months ago
mozreview-review-reply
Comment on attachment 8996076 [details]
Bug 1479565 - add speculative filter type for webRequest and proxy API,

https://reviewboard.mozilla.org/r/260328/#review267404

> I don't really understand what's going on here.
> Based on the comment, could the code become `if (details.type != "speculative") ...` ?

Because the test doesn't finish until the xhr is seen.

However, this change is not necessary.  I added it due to a try failure that actually turned out to be a different problem in the original proxy patch.  So I'll remove it.
Comment hidden (mozreview-request)

Comment 5

8 months ago
mozreview-review
Comment on attachment 8996076 [details]
Bug 1479565 - add speculative filter type for webRequest and proxy API,

https://reviewboard.mozilla.org/r/260328/#review268066
Attachment #8996076 - Flags: review?(aswan) → review+

Comment 6

8 months ago
Pushed by mixedpuppy@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/020b2bf5594f
add speculative filter type for webRequest and proxy API, r=aswan

Comment 7

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/020b2bf5594f
Status: NEW → RESOLVED
Last Resolved: 8 months ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63

Comment 8

7 months ago
Is manual testing required on this bug? If yes, please provide some STR and the proper extension(if required) or set the “qe-verify -“ flag.

Thanks!
Flags: needinfo?(mixedpuppy)
(Assignee)

Updated

7 months ago
Flags: needinfo?(mixedpuppy) → qe-verify-
I've updated https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/webRequest/ResourceType to list "speculative" in the page and compat table.

Please let me know if this covers it.
Flags: needinfo?(mixedpuppy)
(Assignee)

Comment 10

6 months ago
thanks!
Flags: needinfo?(mixedpuppy)
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.