Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(28)

Issue 2804843005: Implement the infrastructure of creating WorkerFetchContext in worker global scope. (Closed)

Created:
3 years, 8 months ago by horo
Modified:
3 years, 7 months ago
CC:
chromium-reviews, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, nasko+codewatch_chromium.org, yzshen+watch_chromium.org, kinuko+watch, jsbell+serviceworker_chromium.org, Yoav Weiss, tzik, jam, abarth-chromium, dglazkov+blink, darin-cc_chromium.org, gavinp+loader_chromium.org, blink-reviews, blink-reviews-api_chromium.org, blink-worker-reviews_chromium.org, mlamouri+watch-content_chromium.org, creis+watch_chromium.org, nhiroki, loading-reviews_chromium.org, loading-reviews+fetch_chromium.org, Nate Chapin, michaeln, shimazu+serviceworker_chromium.org, tyoshino+watch_chromium.org, serviceworker-reviews, Aaron Boodman, kinuko+serviceworker, horo+watch_chromium.org, darin (slow to review), yhirano
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Implement the infrastructure of creating WorkerFetchContext in worker global scope. This CL implements the infrastructure of creating WorkerFetchContext in worker global scope of Dedicated Worker, Shared Worker and Service Worker. 1. WorkerFetchContext for Dedicated Worker 1.1. DedicatedWorkerMessagingProxyProviderImpl::CreateWorkerMessagingProxy() calls RenderFrameImpl::CreateWorkerFetchContext() on the main thread. 1.2. RenderFrameImpl::CreateWorkerFetchContext() gets the interface of WorkerURLLoaderFactoryProvider which is implemented in the browser process as WorkerURLLoaderFactoryProviderImpl, and creates a WorkerFetchContextImpl with the interface. WorkerFetchContextImpl keeps the interface as |provider_info_| until InitializeOnWorkerThread() is called on the worker thread. 1.3. RenderFrameImpl::CreateWorkerFetchContext() copies the service worker state (ServiceWorkerProviderID and IsControlledByServiceWorker) of the frame to the WorkerFetchContextImpl. 1.4. ProvideWorkerFetchContextToWorker() creates a WorkerFetchContextHolder which holds the WorkerFetchContextImpl and provides it to the WorkerClients using the Supplement mechanism. 1.5. After the worker thread is created, the WorkerClients is reattached to the worker thread in the constructor of WorkerGlobalScope. 1.6. WorkerGlobalScope::FetchContext() will be called to get the WorkerFetchContext in ThreadableLoader::Create() When fetch() is called in the worker thread. (This is not in this CL yet.) 1.7. When WorkerGlobalScope::FetchContext() is called for the first time, WorkerFetchContext::Create() is called to get the WorkerFetchContextHolder from the WorkerClients and take the WorkerFetchContextImpl from it and create the WorkerFetchContext. 1.8. The constructor of WorkerFetchContext calls WorkerFetchContextImpl:: InitializeOnWorkerThread() which creates a ResourceDispatcher and calls WorkerURLLoaderFactoryProviderImpl::GetURLLoaderFactoryAndRegisterClient() in the browser process via mojo IPC with a request of URLLoaderFactory and the pointer (ServiceWorkerWorkerClient) of itself. 1.9. WorkerURLLoaderFactoryProviderImpl::GetURLLoaderFactoryAndRegisterClient() binds the request of URLLoaderFactory to ResourceMessageFilter and calls ServiceWorkerContextWrapper::BindWorkerFetchContext() to bind the ServiceWorkerWorkerClient (the pointer to WorkerFetchContextImpl in the worker thread) with the parent frame's ServiceWorkerProviderHost. When the controller service worker of the ServiceWorkerProviderHost changed, WorkerFetchContextImpl::SetControllerServiceWorker() will be called via mojo. So WorkerFetchContextImpl::IsControlledByServiceWorker() can return the correct status. 1.10. WorkerFetchContext::CreateURLLoader() calls WorkerFetchContextImpl:: CreateURLLoader() which returns a new WebURLLoaderImpl with the pointer of URLLoaderFactory. 2. WorkerFetchContext for Shared Worker is created almost same as the Dedicated Worker. The only difference is that CreateWorkerFetchContext() is called in WebSharedWorkerImpl::OnScriptLoaderFinished(). 3. WorkerFetchContext for Service Worker 3.1. WebEmbeddedWorkerImpl::StartWorkerThread() calls ServiceWorkerContextClient::CreateServiceWorkerFetchContext() on the main thread. 3.2. ServiceWorkerContextClient::CreateServiceWorkerFetchContext() gets the interface of WorkerURLLoaderFactoryProvider which is implemented in the browser process as WorkerURLLoaderFactoryProviderImpl, and creates a ServiceWorkerFetchContextImpl with the interface. ServiceWorkerFetchContextImpl keeps the interface as |provider_info_| until InitializeOnWorkerThread() is called on the worker thread. 3.3. In WebEmbeddedWorkerImpl::StartWorkerThread(), the fetch context related information (ex: DataSaverEnabled) is set to the ServiceWorkerFetchContextImpl. (This is not in this CL yet.) 3.4. Same as 1.4. ProvideWorkerFetchContextToWorker() creates a WorkerFetchContextHolder which holds the ServiceWorkerFetchContextImpl and provides it to the WorkerClientsusing the Supplement mechanism. 3.5. Same as 1.5. the WorkerClients is reattached to the worker thread. 3.6. Same as 1.6. 3.7. Same as 1.7. When WorkerGlobalScope::FetchContext() is called for the first time, WorkerFetchContext::Create() is called to get the WorkerFetchContextHolder from the WorkerClients and take the ServiceWorkerFetchContextImpl from it and create the WorkerFetchContext. 3.8. The constructor of WorkerFetchContext calls ServiceWorkerFetchContextImpl ::InitializeOnWorkerThread() which creates a ResourceDispatcher and calls WorkerURLLoaderFactoryProviderImpl::GetURLLoaderFactory() in the browser process via mojo IPC with a request of URLLoaderFactory. 3.9. WorkerURLLoaderFactoryProviderImpl::GetURLLoaderFactory() binds the request of URLLoaderFactory to ResourceMessageFilter. 3.10. WorkerFetchContext::CreateURLLoader() calls ServiceWorkerFetchContextImpl::CreateURLLoader() which returns a new WebURLLoaderImpl with the pointer of URLLoaderFactory. BUG=443374 Review-Url: https://codereview.chromium.org/2804843005 Cr-Commit-Position: refs/heads/master@{#467200} Committed: https://chromium.googlesource.com/chromium/src/+/e612058238bb3f549a917d55f94badf32377f85c

Patch Set 1 #

Total comments: 16

Patch Set 2 : incorporated nhiroki and kinuko's comment #

Total comments: 17

Patch Set 3 : incorporated kinuko's comment #

Patch Set 4 : clean up #

Patch Set 5 : rebase #

Total comments: 42

Patch Set 6 : incorporated kinuko and nhiroki's comment #

Patch Set 7 : rebase #

Patch Set 8 : move CreateWorkerFetchContext from Platform to WebFrameClient #

Total comments: 8

Patch Set 9 : implement EmbeddedSharedWorkerStub::CreateWorkerFetchContext #

Patch Set 10 : incorporated kinuko's comment #

Total comments: 30

Patch Set 11 : incorporated yhirno, nhiroki and kinuko's comment #

Patch Set 12 : fix android build error #

Patch Set 13 : rebase #

Total comments: 4

Patch Set 14 : incorporated yhirano's comment #

Total comments: 3

Patch Set 15 : rebase #

Patch Set 16 : s/WebScheduler.h/web_scheduler.h/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+820 lines, -17 lines) Patch
M content/browser/renderer_host/render_process_host_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 4 chunks +57 lines, -0 lines 0 comments Download
M content/browser/service_worker/service_worker_context_core.h View 1 2 3 2 chunks +9 lines, -0 lines 0 comments Download
M content/browser/service_worker/service_worker_context_core.cc View 1 2 1 chunk +11 lines, -0 lines 0 comments Download
M content/browser/service_worker/service_worker_context_wrapper.h View 1 2 3 2 chunks +9 lines, -0 lines 0 comments Download
M content/browser/service_worker/service_worker_context_wrapper.cc View 1 2 1 chunk +10 lines, -0 lines 0 comments Download
M content/browser/service_worker/service_worker_provider_host.h View 1 2 3 4 5 5 chunks +18 lines, -0 lines 0 comments Download
M content/browser/service_worker/service_worker_provider_host.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 4 chunks +33 lines, -1 line 0 comments Download
M content/child/resource_dispatcher.h View 1 2 3 4 5 6 7 8 9 10 3 chunks +5 lines, -5 lines 0 comments Download
M content/child/resource_dispatcher.cc View 1 2 3 4 5 6 7 8 9 10 7 chunks +10 lines, -10 lines 0 comments Download
M content/child/runtime_features.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +3 lines, -0 lines 0 comments Download
M content/common/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -0 lines 0 comments Download
A content/common/worker_url_loader_factory_provider.mojom View 1 2 3 4 5 6 7 8 9 1 chunk +33 lines, -0 lines 0 comments Download
M content/public/app/mojo/content_browser_manifest.json View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -0 lines 0 comments Download
M content/renderer/BUILD.gn View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download
M content/renderer/render_frame_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +2 lines, -0 lines 0 comments Download
M content/renderer/render_frame_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 5 chunks +29 lines, -0 lines 0 comments Download
M content/renderer/render_thread_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -1 line 0 comments Download
M content/renderer/service_worker/service_worker_context_client.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 3 chunks +4 lines, -0 lines 0 comments Download
M content/renderer/service_worker/service_worker_context_client.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 3 chunks +21 lines, -0 lines 0 comments Download
A content/renderer/service_worker/service_worker_fetch_context_impl.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +43 lines, -0 lines 0 comments Download
A content/renderer/service_worker/service_worker_fetch_context_impl.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +49 lines, -0 lines 0 comments Download
A content/renderer/service_worker/worker_fetch_context_impl.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +60 lines, -0 lines 0 comments Download
A content/renderer/service_worker/worker_fetch_context_impl.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +74 lines, -0 lines 0 comments Download
M content/renderer/shared_worker/embedded_shared_worker_stub.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -0 lines 0 comments Download
M content/renderer/shared_worker/embedded_shared_worker_stub.cc View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +30 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/loader/BUILD.gn View 1 2 3 4 5 6 10 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/loader/FrameFetchContext.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/loader/FrameFetchContext.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +5 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/core/loader/WorkerFetchContext.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +47 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +103 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/workers/WorkerGlobalScope.h View 1 2 3 4 5 6 7 8 9 10 3 chunks +6 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/workers/WorkerGlobalScope.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 4 chunks +11 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5 View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/loader/fetch/FetchContext.h View 1 2 3 4 5 6 7 8 9 10 2 chunks +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/DedicatedWorkerMessagingProxyProviderImpl.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +13 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 4 chunks +14 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/WebRuntimeFeatures.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 4 chunks +17 lines, -0 lines 0 comments Download
M third_party/WebKit/public/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -0 lines 0 comments Download
A third_party/WebKit/public/platform/WebWorkerFetchContext.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +43 lines, -0 lines 0 comments Download
M third_party/WebKit/public/web/WebFrameClient.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +8 lines, -0 lines 0 comments Download
M third_party/WebKit/public/web/WebRuntimeFeatures.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/public/web/WebSharedWorkerClient.h View 1 2 3 4 5 6 7 8 9 10 2 chunks +9 lines, -0 lines 0 comments Download
M third_party/WebKit/public/web/modules/serviceworker/WebServiceWorkerContextClient.h View 1 2 3 4 5 6 7 8 9 10 2 chunks +9 lines, -0 lines 0 comments Download

Messages

Total messages: 139 (101 generated)
horo
kinuko@ Could you please review this?
3 years, 8 months ago (2017-04-13 05:41:08 UTC) #38
kinuko
Looking very promising! Sorry for slow comments, I have a few high-level questions (I'm pretty ...
3 years, 8 months ago (2017-04-14 02:27:02 UTC) #41
nhiroki
DBC from the core/worker POV. https://codereview.chromium.org/2804843005/diff/180001/third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp File third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp (right): https://codereview.chromium.org/2804843005/diff/180001/third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp#newcode43 third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp:43: ->ToSingleThreadTaskRunner()); FYI: CurrentThread()->Scheduler()->LoadingTaskRunner() is ...
3 years, 8 months ago (2017-04-14 04:09:25 UTC) #43
kinuko
(Just noting what we discussed offline) Will take further look. https://codereview.chromium.org/2804843005/diff/180001/content/browser/renderer_host/render_process_host_impl.cc File content/browser/renderer_host/render_process_host_impl.cc (right): https://codereview.chromium.org/2804843005/diff/180001/content/browser/renderer_host/render_process_host_impl.cc#newcode515 ...
3 years, 8 months ago (2017-04-14 05:38:12 UTC) #45
horo
https://codereview.chromium.org/2804843005/diff/180001/third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp File third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp (right): https://codereview.chromium.org/2804843005/diff/180001/third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp#newcode43 third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp:43: ->ToSingleThreadTaskRunner()); On 2017/04/14 04:09:24, nhiroki wrote: > FYI: CurrentThread()->Scheduler()->LoadingTaskRunner() ...
3 years, 8 months ago (2017-04-14 07:22:45 UTC) #48
kinuko
Some more comments. Overall this is looking good. (I think I'll probably have more comments ...
3 years, 8 months ago (2017-04-14 14:00:27 UTC) #51
horo
https://codereview.chromium.org/2804843005/diff/200001/content/browser/renderer_host/render_process_host_impl.cc File content/browser/renderer_host/render_process_host_impl.cc (right): https://codereview.chromium.org/2804843005/diff/200001/content/browser/renderer_host/render_process_host_impl.cc#newcode1352 content/browser/renderer_host/render_process_host_impl.cc:1352: service_worker_context)); On 2017/04/14 14:00:26, kinuko wrote: > nit: it ...
3 years, 8 months ago (2017-04-17 15:12:04 UTC) #59
nhiroki
https://codereview.chromium.org/2804843005/diff/280001/third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp File third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp (right): https://codereview.chromium.org/2804843005/diff/280001/third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp#newcode21 third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp:21: class WorkerFetchContextInfo final Can you add a class-level comment? ...
3 years, 8 months ago (2017-04-18 07:56:41 UTC) #66
kinuko
Sending some comments before I look at everything https://codereview.chromium.org/2804843005/diff/200001/content/common/worker_fetch_context_factory.mojom File content/common/worker_fetch_context_factory.mojom (right): https://codereview.chromium.org/2804843005/diff/200001/content/common/worker_fetch_context_factory.mojom#newcode25 content/common/worker_fetch_context_factory.mojom:25: int32 ...
3 years, 8 months ago (2017-04-18 08:00:32 UTC) #67
kinuko
Some more comments. (I'll also look into the entire patch again) https://codereview.chromium.org/2804843005/diff/280001/third_party/WebKit/Source/web/DedicatedWorkerMessagingProxyProviderImpl.cpp File third_party/WebKit/Source/web/DedicatedWorkerMessagingProxyProviderImpl.cpp (right): ...
3 years, 8 months ago (2017-04-18 08:35:37 UTC) #68
horo
https://codereview.chromium.org/2804843005/diff/200001/content/common/worker_fetch_context_factory.mojom File content/common/worker_fetch_context_factory.mojom (right): https://codereview.chromium.org/2804843005/diff/200001/content/common/worker_fetch_context_factory.mojom#newcode25 content/common/worker_fetch_context_factory.mojom:25: int32 service_worker_provider_id); On 2017/04/18 08:00:31, kinuko wrote: > On ...
3 years, 8 months ago (2017-04-18 12:53:35 UTC) #71
kinuko
(Responses only, and will take another shot of serious review.) https://codereview.chromium.org/2804843005/diff/200001/content/common/worker_fetch_context_factory.mojom File content/common/worker_fetch_context_factory.mojom (right): https://codereview.chromium.org/2804843005/diff/200001/content/common/worker_fetch_context_factory.mojom#newcode25 ...
3 years, 8 months ago (2017-04-19 03:24:49 UTC) #74
kinuko
https://codereview.chromium.org/2804843005/diff/280001/third_party/WebKit/public/platform/WebWorkerFetchContext.h File third_party/WebKit/public/platform/WebWorkerFetchContext.h (right): https://codereview.chromium.org/2804843005/diff/280001/third_party/WebKit/public/platform/WebWorkerFetchContext.h#newcode41 third_party/WebKit/public/platform/WebWorkerFetchContext.h:41: // the main thread. On 2017/04/18 12:53:35, horo wrote: ...
3 years, 8 months ago (2017-04-19 03:50:49 UTC) #75
horo
On 2017/04/19 03:50:49, kinuko wrote: > https://codereview.chromium.org/2804843005/diff/280001/third_party/WebKit/public/platform/WebWorkerFetchContext.h > File third_party/WebKit/public/platform/WebWorkerFetchContext.h (right): > > https://codereview.chromium.org/2804843005/diff/280001/third_party/WebKit/public/platform/WebWorkerFetchContext.h#newcode41 > ...
3 years, 8 months ago (2017-04-19 04:30:22 UTC) #76
kinuko
On 2017/04/19 04:30:22, horo wrote: > On 2017/04/19 03:50:49, kinuko wrote: > > > https://codereview.chromium.org/2804843005/diff/280001/third_party/WebKit/public/platform/WebWorkerFetchContext.h ...
3 years, 8 months ago (2017-04-19 05:19:27 UTC) #77
horo
On 2017/04/19 05:19:27, kinuko wrote: > On 2017/04/19 04:30:22, horo wrote: > > On 2017/04/19 ...
3 years, 8 months ago (2017-04-19 10:36:32 UTC) #81
kinuko
I think this is looking good now. +cc yhirano as we just discussed how we ...
3 years, 8 months ago (2017-04-19 11:08:14 UTC) #82
horo
On 2017/04/19 10:36:32, horo wrote: > On 2017/04/19 05:19:27, kinuko wrote: > > On 2017/04/19 ...
3 years, 8 months ago (2017-04-19 11:36:14 UTC) #83
horo
https://codereview.chromium.org/2804843005/diff/340001/content/browser/renderer_host/render_process_host_impl.cc File content/browser/renderer_host/render_process_host_impl.cc (right): https://codereview.chromium.org/2804843005/diff/340001/content/browser/renderer_host/render_process_host_impl.cc#newcode500 content/browser/renderer_host/render_process_host_impl.cc:500: void CreateWorkerFetchContext( On 2017/04/19 11:08:14, kinuko wrote: > I ...
3 years, 8 months ago (2017-04-19 13:52:27 UTC) #88
kinuko
I probably have some more comments but I think this basically lgtm, let's get this ...
3 years, 8 months ago (2017-04-20 04:08:14 UTC) #92
nhiroki
https://codereview.chromium.org/2804843005/diff/380001/content/renderer/render_frame_impl.cc File content/renderer/render_frame_impl.cc (right): https://codereview.chromium.org/2804843005/diff/380001/content/renderer/render_frame_impl.cc#newcode3008 content/renderer/render_frame_impl.cc:3008: if (!base::FeatureList::IsEnabled(features::kOffMainThreadFetch)) DCHECK(base::FeatureList::IsEnabled(features::kOffMainThreadFetch)); ? https://codereview.chromium.org/2804843005/diff/380001/content/renderer/service_worker/service_worker_context_client.cc File content/renderer/service_worker/service_worker_context_client.cc (right): https://codereview.chromium.org/2804843005/diff/380001/content/renderer/service_worker/service_worker_context_client.cc#newcode1011 ...
3 years, 8 months ago (2017-04-20 04:16:07 UTC) #93
nhiroki
lgtm from worker infra pov https://codereview.chromium.org/2804843005/diff/380001/third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp File third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp (right): https://codereview.chromium.org/2804843005/diff/380001/third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp#newcode72 third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp:72: web_context_->InitializeOnWorkerThread(Platform::Current() On 2017/04/20 04:16:07, ...
3 years, 8 months ago (2017-04-20 04:17:33 UTC) #94
yhirano
You're adding a code path to create a resource dispatcher in a non-main thread, so ...
3 years, 8 months ago (2017-04-20 04:27:00 UTC) #96
horo
https://codereview.chromium.org/2804843005/diff/380001/content/renderer/render_frame_impl.cc File content/renderer/render_frame_impl.cc (right): https://codereview.chromium.org/2804843005/diff/380001/content/renderer/render_frame_impl.cc#newcode3008 content/renderer/render_frame_impl.cc:3008: if (!base::FeatureList::IsEnabled(features::kOffMainThreadFetch)) On 2017/04/20 04:16:06, nhiroki wrote: > DCHECK(base::FeatureList::IsEnabled(features::kOffMainThreadFetch)); ...
3 years, 8 months ago (2017-04-20 08:35:44 UTC) #103
yhirano
https://codereview.chromium.org/2804843005/diff/440001/content/browser/renderer_host/render_process_host_impl.cc File content/browser/renderer_host/render_process_host_impl.cc (right): https://codereview.chromium.org/2804843005/diff/440001/content/browser/renderer_host/render_process_host_impl.cc#newcode485 content/browser/renderer_host/render_process_host_impl.cc:485: mojo::MakeStrongBinding( When is this destructed? https://codereview.chromium.org/2804843005/diff/440001/content/browser/service_worker/service_worker_provider_host.cc File content/browser/service_worker/service_worker_provider_host.cc (right): ...
3 years, 8 months ago (2017-04-21 09:18:41 UTC) #110
yhirano
lgtm https://codereview.chromium.org/2804843005/diff/440001/content/browser/renderer_host/render_process_host_impl.cc File content/browser/renderer_host/render_process_host_impl.cc (right): https://codereview.chromium.org/2804843005/diff/440001/content/browser/renderer_host/render_process_host_impl.cc#newcode485 content/browser/renderer_host/render_process_host_impl.cc:485: mojo::MakeStrongBinding( On 2017/04/21 09:18:41, yhirano wrote: > When ...
3 years, 8 months ago (2017-04-21 09:30:44 UTC) #111
horo
Thank you! https://codereview.chromium.org/2804843005/diff/440001/content/browser/service_worker/service_worker_provider_host.cc File content/browser/service_worker/service_worker_provider_host.cc (right): https://codereview.chromium.org/2804843005/diff/440001/content/browser/service_worker/service_worker_provider_host.cc#newcode277 content/browser/service_worker/service_worker_provider_host.cc:277: for (auto it = worker_clients_.begin(); it != ...
3 years, 8 months ago (2017-04-21 09:42:32 UTC) #114
horo
estark@ Could you please review .mojom files and content_browser_manifest.json?
3 years, 8 months ago (2017-04-21 09:44:39 UTC) #116
estark
The mojom changes mostly lg with a question... but TBH I've been staring at this ...
3 years, 8 months ago (2017-04-25 06:41:08 UTC) #120
horo
https://codereview.chromium.org/2804843005/diff/460001/content/renderer/service_worker/worker_fetch_context_impl.cc File content/renderer/service_worker/worker_fetch_context_impl.cc (right): https://codereview.chromium.org/2804843005/diff/460001/content/renderer/service_worker/worker_fetch_context_impl.cc#newcode71 content/renderer/service_worker/worker_fetch_context_impl.cc:71: controller_version_id_ = controller_version_id; On 2017/04/25 06:41:08, estark wrote: > ...
3 years, 8 months ago (2017-04-25 08:43:45 UTC) #121
estark
https://codereview.chromium.org/2804843005/diff/460001/content/renderer/service_worker/worker_fetch_context_impl.cc File content/renderer/service_worker/worker_fetch_context_impl.cc (right): https://codereview.chromium.org/2804843005/diff/460001/content/renderer/service_worker/worker_fetch_context_impl.cc#newcode71 content/renderer/service_worker/worker_fetch_context_impl.cc:71: controller_version_id_ = controller_version_id; On 2017/04/25 08:43:44, horo wrote: > ...
3 years, 7 months ago (2017-04-25 20:26:56 UTC) #122
kenrb
lgtm I'm not especially familiar with web/service worker implementations, but I don't see any problems ...
3 years, 7 months ago (2017-04-25 21:11:34 UTC) #123
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2804843005/460001
3 years, 7 months ago (2017-04-25 21:17:47 UTC) #126
commit-bot: I haz the power
Try jobs failed on following builders: android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_clang_dbg_recipe/builds/255913) chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, ...
3 years, 7 months ago (2017-04-25 21:23:32 UTC) #128
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2804843005/480001
3 years, 7 months ago (2017-04-25 22:04:14 UTC) #131
commit-bot: I haz the power
Try jobs failed on following builders: cast_shell_android on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_android/builds/255105)
3 years, 7 months ago (2017-04-25 22:26:19 UTC) #133
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2804843005/500001
3 years, 7 months ago (2017-04-25 22:57:23 UTC) #136
commit-bot: I haz the power
3 years, 7 months ago (2017-04-26 01:49:47 UTC) #139
Message was sent while issue was closed.
Committed patchset #16 (id:500001) as
https://chromium.googlesource.com/chromium/src/+/e612058238bb3f549a917d55f94b...

Powered by Google App Engine
This is Rietveld 408576698