1311576 - webRequest.RequestFilter doesn't support tabId or windowId values

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect, P2)

Description

[21Naown]

For example:

chrome.webRequest.onResponseStarted.addListener(getURL,	{urls: ["TO_MATCH"], tabId: 22}, ["responseHeaders"]);

work out of tabId 22.

Comment 1

[Shane Caraveo (:mixedpuppy)]

expanding this to include both missing items.

Comment 3

[Shane Caraveo (:mixedpuppy)]

Attachment: WIP webrequestFilter

Wondering about the approach here.

One approach would be to just do this in WebRequest.jsm, but it doesn't seem right to pull in ext-* stuff there.  So this is a later filtering than the filtering done in WebRequest.jsm.

Comment 4

[21Naown]

I forgot to add that in the doc (https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest#Chrome_incompatibilities), the problem is already reported. May be you known it too?

Comment 5

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1311576 fix webrequest filter for tabId and windowId,

Review commit: https://reviewboard.mozilla.org/r/90792/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/90792/

Comment 6

[Shane Caraveo (:mixedpuppy)]

Building the tests on top of patches in bug 1314492

Comment 7

[Kris Maglione [:kmag]]

Comment on attachment 8807749 [details]
Bug 1311576 fix webrequest filter for tabId and windowId,

https://reviewboard.mozilla.org/r/90792/#review90670

::: toolkit/components/extensions/test/mochitest/test_ext_webrequest_filter.html:22
(Diff revision 1)
> +
> +add_task(function* setup() {
> +  testWindow = window.open("about:blank", "_blank", "width=100,height=100");
> +  yield waitForLoad(testWindow);
> +
> +  // fetch the windowId and tabId we need to filter with WebRequest

Nit: Please capitalize comments and end with full stop.

::: toolkit/components/extensions/test/mochitest/test_ext_webrequest_filter.html:31
(Diff revision 1)
> +        "tabs",
> +      ],
> +    },
> +    background() {
> +      browser.windows.getCurrent({populate: true}).then(window => {
> +        browser.test.log(`current window ${window.id} tabs: ${JSON.stringify(window.tabs.map((tab) => { return [tab.id, tab.url]; }))}`);

Nit: `.map(tab => [tab.id, tab.url])`

::: toolkit/components/extensions/test/mochitest/test_ext_webrequest_filter.html:38
(Diff revision 1)
> +      });
> +    },
> +  });
> +  yield extension.startup();
> +  windowData = yield extension.awaitMessage("windowData");
> +  info(`window is is ${JSON.stringify(windowData)}`);

s/is is/is/

::: toolkit/components/extensions/test/mochitest/test_ext_webrequest_filter.html:48
(Diff revision 1)
> +  yield SpecialPowers.pushPrefEnv({
> +    set: [["dom.serviceWorkers.testing.enabled", true]],
> +  });
> +
> +  let events = {
> +    "onBeforeRequest":     [{urls: ["<all_urls>"], windowId: windowData.windowId}, ["blocking", "requestBody"]],

Probably best to leave "requestBody" out, and none of these probably need to be blocking.

Comment 8

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8807749 [details]
Bug 1311576 fix webrequest filter for tabId and windowId,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/90792/diff/1-2/

Comment 9

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8807749 [details]
Bug 1311576 fix webrequest filter for tabId and windowId,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/90792/diff/2-3/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15886f6c3dd1
fix webrequest filter for tabId and windowId, r=kmag

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back this out for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=6484705&repo=autoland

https://hg.mozilla.org/integration/autoland/rev/2a0faaf8a4ff

Comment 12

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8807749 [details]
Bug 1311576 fix webrequest filter for tabId and windowId,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/90792/diff/3-4/

Comment 13

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8807749 [details]
Bug 1311576 fix webrequest filter for tabId and windowId,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/90792/diff/4-5/

Comment 14

[Shane Caraveo (:mixedpuppy)]

Kris, can you look at the test changes in the last push to review?

Comment 15

[Kris Maglione [:kmag]]

Comment on attachment 8807749 [details]
Bug 1311576 fix webrequest filter for tabId and windowId,

https://reviewboard.mozilla.org/r/90792/#review93290

::: toolkit/components/extensions/test/mochitest/head_webrequest.js:163
(Diff revisions 4 - 5)
>        let expectedEvent = expected.events[0] == name;
>        if (expectedEvent) {
>          expected.events.shift();
>        } else {
> -        expectedEvent = expected.optional_events[0] == name;
> -        if (expectedEvent) {
> +        // e10s vs. non-e10s errors can end with either onCompleted or onErrorOccurred
> +        expectedEvent = expected.optional_events.indexOf(name) >= 0;

`expected.optional_events.includes(name)`

Comment 16

[Kris Maglione [:kmag]]

Looks OK to me

Comment 17

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8807749 [details]
Bug 1311576 fix webrequest filter for tabId and windowId,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/90792/diff/5-6/

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b227cc95b6d9
fix webrequest filter for tabId and windowId, r=kmag

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b227cc95b6d9

Comment 20

[Will Bamberg [:wbamberg]]

I've updated the compat data for this change. I think that's all we need, so am marking this dev-doc complete.