New issue
Advanced search Search tips

Issue 591988 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug-Regression



Sign in to add a comment

chrome.webRequest + "types" in RequestFilter = no callback calls

Reported by perceptr...@gmail.com, Mar 4 2016

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.75 Safari/537.36

Steps to reproduce the problem:
1. Clone, review and install
https://github.com/perceptron8/chromium-requestFilter-issue

2. Go to
http://www.adobe.com/devnet/actionscript/samples/media_2.html

What is the expected behavior?
1. Requests types should appear in console (because of console.log).

2. Pink rectangle should be loaded instead of original images (because of callback's return).

What went wrong?
The "callback" function is not called because of incorrect "types" handling (or something else?).

However, if I remove "types" from filter, callback IS called. "details.type" is "object" in Chrome 49. It was "other" until Chrome 48. I don't know it's related, but it may be, so I decided to let you know.

WebStore page: 

Did this work before? Yes Before Chrome 49

Chrome version: 49.0.2623.75  Channel: n/a
OS Version: 4.2.0-30-generic
Flash Version: Shockwave Flash 20.0 r0

It may be related to Flash, but I'm not sure what version I was using before things had gone wrong, sorry. It's possible however that it was 19.x, not 20.x.
 
Can you at least confirm? Sorry for the impatience, but I believe I gave you everything you need to reproduce this bug within a few minutes. It's just 20 lines of sample extension, easy to review, easy to test.

Is my description incomplete? Do you need something else?

Comment 2 by rob@robwu.nl, Apr 4 2016

Labels: -Type-Bug Type-Bug-Regression
Owner: rob@robwu.nl
Status: Assigned (was: Unconfirmed)
Thanks for the clear bug report!

I confirmed that this worked fine in 48.0.2564.82, but not in 49.0.2623.75.

Bisecting (https://www.chromium.org/developers/bisect-builds-py) gives the following range:
https://chromium.googlesource.com/chromium/src/+log/176e2693216e9b05a2affea94fe72014c671d65f..6a32a203bbae04ef29a42f3f364ec00a0adf9179

So the bug seems indeed related to the change from "other" -> "object" ( https://crbug.com/410382#c39 ).

Comment 3 by rob@robwu.nl, Apr 4 2016

Labels: M-50
Status: Started (was: Assigned)
This turns out to be a small oversight: https://codereview.chromium.org/1853283002/

I'll try to get the fix in Chrome 50, since it's a recent (49) regression and the patch is relatively simple.
Glad to hear :) Thank you and good luck!
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 6 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4bb2b298018be3f9edebe3009d9e7a92f04e06aa

commit 4bb2b298018be3f9edebe3009d9e7a92f04e06aa
Author: rob <rob@robwu.nl>
Date: Wed Apr 06 00:18:05 2016

Map webRequest filter type to multiple internal ResourceTypes if needed

In the past, there was a 1-to-1 relation between content::ResourceType
and the public webRequest types. Since  https://crbug.com/410382#c39 
added multiple resource types that mapped to the same webRequest type,
this relation is no longer true (it's now a n-to-1 relation).

This change needs to be accounted for by registering multiple internal
filter types if a webRequest filter type is set that maps to more than
one content::ResourceType.

BUG= 591988 
TEST=browser_tests --gtest_filter=ExtensionWebRequestApiTest.WebRequestTypes

Review URL: https://codereview.chromium.org/1853283002

Cr-Commit-Position: refs/heads/master@{#385340}

[modify] https://crrev.com/4bb2b298018be3f9edebe3009d9e7a92f04e06aa/chrome/test/data/extensions/api_test/webrequest/framework.js
[modify] https://crrev.com/4bb2b298018be3f9edebe3009d9e7a92f04e06aa/chrome/test/data/extensions/api_test/webrequest/test_types.js
[modify] https://crrev.com/4bb2b298018be3f9edebe3009d9e7a92f04e06aa/extensions/browser/api/declarative_webrequest/webrequest_condition_attribute.cc
[modify] https://crrev.com/4bb2b298018be3f9edebe3009d9e7a92f04e06aa/extensions/browser/api/web_request/web_request_api.cc
[modify] https://crrev.com/4bb2b298018be3f9edebe3009d9e7a92f04e06aa/extensions/browser/api/web_request/web_request_api_helpers.cc
[modify] https://crrev.com/4bb2b298018be3f9edebe3009d9e7a92f04e06aa/extensions/browser/api/web_request/web_request_api_helpers.h

Comment 6 by rob@robwu.nl, Apr 6 2016

Labels: -OS-Linux Merge-Request-50 OS-All
Status: Fixed (was: Started)
Requesting a merge of 4bb2b298018be3f9edebe3009d9e7a92f04e06aa with 50 because it is a simple patch for a recent regression.

Comment 7 by tin...@google.com, Apr 6 2016

Labels: -Merge-Request-50 Merge-Review-50 Hotlist-Merge-Review
[Automated comment] Less than 2 weeks to go before stable on M50, manual review required.

Comment 8 by tin...@google.com, Apr 7 2016

Labels: -Merge-Review-50 Merge-Approved-50
Merge approved for M50 (branch 2661). Pls go ahead merge.
Please merge your change to M50 branch 2661 by 5:00 PM PST on April 8th,Friday to make into the desktop Stable final build cut. Thank you.
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 8 2016

Labels: -merge-approved-50 merge-merged-2661
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/578357907484815345da27beeeca60cd660272fb

commit 578357907484815345da27beeeca60cd660272fb
Author: Rob Wu <rob@robwu.nl>
Date: Fri Apr 08 16:37:11 2016

Map webRequest filter type to multiple internal ResourceTypes if needed

In the past, there was a 1-to-1 relation between content::ResourceType
and the public webRequest types. Since  https://crbug.com/410382#c39 
added multiple resource types that mapped to the same webRequest type,
this relation is no longer true (it's now a n-to-1 relation).

This change needs to be accounted for by registering multiple internal
filter types if a webRequest filter type is set that maps to more than
one content::ResourceType.

BUG= 591988 
TEST=browser_tests --gtest_filter=ExtensionWebRequestApiTest.WebRequestTypes

Review URL: https://codereview.chromium.org/1853283002

Cr-Commit-Position: refs/heads/master@{#385340}
(cherry picked from commit 4bb2b298018be3f9edebe3009d9e7a92f04e06aa)

Review URL: https://codereview.chromium.org/1865303006 .

Cr-Commit-Position: refs/branch-heads/2661@{#529}
Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081}

[modify] https://crrev.com/578357907484815345da27beeeca60cd660272fb/chrome/test/data/extensions/api_test/webrequest/framework.js
[modify] https://crrev.com/578357907484815345da27beeeca60cd660272fb/chrome/test/data/extensions/api_test/webrequest/test_types.js
[modify] https://crrev.com/578357907484815345da27beeeca60cd660272fb/extensions/browser/api/declarative_webrequest/webrequest_condition_attribute.cc
[modify] https://crrev.com/578357907484815345da27beeeca60cd660272fb/extensions/browser/api/web_request/web_request_api.cc
[modify] https://crrev.com/578357907484815345da27beeeca60cd660272fb/extensions/browser/api/web_request/web_request_api_helpers.cc
[modify] https://crrev.com/578357907484815345da27beeeca60cd660272fb/extensions/browser/api/web_request/web_request_api_helpers.h

Sign in to add a comment