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

Issue 734950 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocked on:
issue 739092



Sign in to add a comment

Set WebContentSettingsClient for service workers

Project Member Reported by shimazu@chromium.org, Jun 20 2017

Issue description

Currently WebEmbeddedWorker::Create takes only nullptr as a param of content_settings_client. The param has been nullptr from ancient times.

We need to confirm it's expected behavior, and if it isn't, let's fix that. 

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.cpp?type=cs&q=WebEmbeddedWorker::Create&l=85
 

Comment 1 by falken@chromium.org, Jun 23 2017

Issue 734958 has been merged into this issue.

Comment 2 by falken@chromium.org, Jun 23 2017

What do SharedWorkers do?

We seem to have EmbeddedSharedWorkerContentSettingsClientProxy which is only used by SharedWorkers so far?


Comment 3 by falken@chromium.org, Jun 27 2017

I suspect it's not a big deal in practice because if service worker is enabled, indexed db etc probably are as well because Chrome's content settings are not super granular (they all count as "storage" or "cookies"). But I think we should probably fix this.
Cc: shimazu@chromium.org nhiroki@chromium.org
Labels: M-61
Owner: yukiy@google.com
Status: Assigned (was: Available)
Labels: -M-61 M-62
Blockedon: 739092
Status: Started (was: Assigned)
Summary: Set WebContentSettingsClient for service workers (was: Confirm if it's necessary to set WebContentSettingsClient for service workers)
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 10 2017

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

commit a20ac9c693c445d43b5f9710e901c813bc56c75a
Author: Yuki Yamada <yukiy@google.com>
Date: Thu Aug 10 08:04:20 2017

Set WebContentSettingsClient for service workers.

WebEmbeddedWorker::Create has taken only nullptr as a param of
content_settings_client from ancient times, so I made
ServiceWorkerContentSettingsProxy and ServiceWorkerContentSettingsProxyImpl.
mojom::ServiceWorkerContentSettingsProxyPtr is created on browser process and
passed onto renderer process through mojom::EmbeddedWorkerInstanceClient::StartWorker.

Bug:  734950 
Change-Id: I1b58e46d6d09783064528e991575cc0fb4831cfd
Reviewed-on: https://chromium-review.googlesource.com/582728
Commit-Queue: Yuki Yamada <yukiy@google.com>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493327}
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/content/browser/BUILD.gn
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/content/browser/service_worker/DEPS
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/content/browser/service_worker/embedded_worker_instance.cc
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/content/browser/service_worker/embedded_worker_instance_unittest.cc
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/content/browser/service_worker/embedded_worker_test_helper.cc
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/content/browser/service_worker/embedded_worker_test_helper.h
[add] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/content/browser/service_worker/service_worker_content_settings_proxy_impl.cc
[add] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/content/browser/service_worker/service_worker_content_settings_proxy_impl.h
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/content/browser/service_worker/service_worker_context_unittest.cc
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/content/browser/service_worker/service_worker_job_unittest.cc
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/content/browser/shared_worker/DEPS
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/content/browser/shared_worker/shared_worker_content_settings_proxy_impl.cc
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/content/browser/shared_worker/shared_worker_content_settings_proxy_impl.h
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/content/browser/shared_worker/shared_worker_host.cc
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/content/common/service_worker/embedded_worker.mojom
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/content/renderer/service_worker/embedded_worker_instance_client_impl.cc
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/content/renderer/service_worker/embedded_worker_instance_client_impl.h
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.cpp
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.h
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/third_party/WebKit/Source/core/workers/SharedWorkerContentSettingsProxy.cpp
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/third_party/WebKit/Source/core/workers/SharedWorkerContentSettingsProxy.h
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.cpp
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.h
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/third_party/WebKit/Source/modules/serviceworkers/BUILD.gn
[add] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerContentSettingsProxy.cpp
[add] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerContentSettingsProxy.h
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/third_party/WebKit/Source/modules/serviceworkers/WebEmbeddedWorkerImplTest.cpp
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/third_party/WebKit/public/web/WebEmbeddedWorker.h
[rename] https://crrev.com/a20ac9c693c445d43b5f9710e901c813bc56c75a/third_party/WebKit/public/web/worker_content_settings_proxy.mojom

Comment 9 by yukiy@google.com, Aug 10 2017

Status: Fixed (was: Started)
This issue is followed up by  https://crbug.com/754182 

Sign in to add a comment