New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 739597 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

SubresourceFilter on the worker thread

Project Member Reported by horo@chromium.org, Jul 6 2017

Issue description

We are implementing off-main-thread-fetch optimization ( issue 443374 ) which enables the worker thread to fetch resources without going to the main thread.

But the SubresourceFilter lives in the main thread.
So when off-main-thread-fetch is enabled, the SubresourceFilter doesn't work while fetching from the worker thread.

I don't know how SubresourceFilter on the worker thread is important or not.
But it is not a web standard feature, so I think this bug is not blocking the off-main-thread-fetch optimization ( issue 443374 ).
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 6 2017

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

commit 05f76ea2d65db9f25b1cccadb8f646b793d25f30
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Thu Jul 06 06:48:28 2017

Add TODO link in WorkerFetchContext::GetSubresourceFilter()

Bug:  739597 
Change-Id: Id185b20178e3c925b14513f56c5d33d443c1f857
Reviewed-on: https://chromium-review.googlesource.com/560812
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484485}
[modify] https://crrev.com/05f76ea2d65db9f25b1cccadb8f646b793d25f30/third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp

Comment 2 by horo@chromium.org, Jul 6 2017

Let me clarify...

Even without the off-main-thread-fetch optimization, SubresourceFilter doesn't work while fetching from shared workers and service workers.
It works while fetching from dedicated workers.

When the off-main-thread-fetch optimization is enabled, SubresourceFilter doesn't work in all (dedicated, shared and service) workers.
Hey horo,
It is important we don't regress SubresourceFilter for the off-main-thread case, since we don't want to allow blocked content to thwart our system.

Thanks for bringing up that currently our system doesn't work in shared workers or service workers, I wasn't aware of that.

It would be really nice if our solution for off-main-thread-fetch could cover more than dedicated workers. If we add the SubresourceFilter to the worker fetch context (as I see you're doing in your WIP CL), would it work?

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 10 2017

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

commit 7f802839b64584e44ba8bae7fb4734ae8f684f67
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Mon Jul 10 06:40:19 2017

Support SubresourceFilter on Dedicated Worker for off-main-thread-fetch

This CL introduces WebDocumentSubresourceFilter::Builder class.

SubresourceFilterAgent::WillCreateWorkerFetchContext() creates the builder
object and set to the WebWorkerFetchContext on the main thread.

WebWorkerFetchContext::TakeSubresourceFilter() is called by the constructor of
WorkerFetchContext on the worker thread to creates a WebDocumentSubresourceFilter
using the builder object.

Note: This CL adds SubresourceFilter supports only for Dedicated Workers with
off-main-thread-fetch optimization. Currently SubresourceFilter is not supported
in Shared Workers and Service Workers regardless of the off-main-thread-fetch
optimization. See  crbug.com/739597#c2 

Bug:  443374 , 739597 
Change-Id: I5719f3b9de33a3e552a3271304e0d530e9c48cc8
Reviewed-on: https://chromium-review.googlesource.com/561263
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485196}
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
[add] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/chrome/test/data/subresource_filter/worker_fetch.html
[add] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/chrome/test/data/subresource_filter/worker_fetch.js
[add] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/chrome/test/data/subresource_filter/worker_fetch_data.txt
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/components/subresource_filter/content/common/ruleset_dealer.cc
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/components/subresource_filter/content/common/ruleset_dealer.h
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/components/subresource_filter/content/renderer/subresource_filter_agent.cc
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/components/subresource_filter/content/renderer/subresource_filter_agent.h
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/components/subresource_filter/content/renderer/web_document_subresource_filter_impl.cc
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/components/subresource_filter/content/renderer/web_document_subresource_filter_impl.h
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/content/public/renderer/render_frame_observer.h
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/content/renderer/service_worker/worker_fetch_context_impl.cc
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/content/renderer/service_worker/worker_fetch_context_impl.h
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/third_party/WebKit/Source/core/exported/WebDataSourceImpl.cpp
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/third_party/WebKit/Source/core/loader/SubresourceFilter.cpp
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/third_party/WebKit/Source/core/loader/SubresourceFilter.h
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/third_party/WebKit/Source/core/loader/WorkerFetchContext.h
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/third_party/WebKit/public/platform/WebDocumentSubresourceFilter.h
[modify] https://crrev.com/7f802839b64584e44ba8bae7fb4734ae8f684f67/third_party/WebKit/public/platform/WebWorkerFetchContext.h

Owner: horo@chromium.org
Status: Assigned (was: Available)
(Worker bug triage)

horo@: Any updates on this? What's the remaining work?

Comment 6 by horo@chromium.org, Jun 25 2018

Status: Fixed (was: Assigned)

Sign in to add a comment