Chromium Code Reviews
Help | Chromium Project | Sign in
(16)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 weeks, 1 day ago by horo(ooo until May 8)
Modified:
3 days, 13 hours 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
Commit queue not available (can’t edit this change).

Messages

Total messages: 139 (101 generated)
horo(ooo until May 8)
kinuko@ Could you please review this?
2 weeks, 2 days 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 ...
2 weeks, 1 day 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 ...
2 weeks, 1 day 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 ...
2 weeks, 1 day ago (2017-04-14 05:38:12 UTC) #45
horo(ooo until May 8)
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() ...
2 weeks, 1 day 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 ...
2 weeks, 1 day ago (2017-04-14 14:00:27 UTC) #51
horo(ooo until May 8)
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 ...
1 week, 5 days 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? ...
1 week, 4 days 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 ...
1 week, 4 days 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): ...
1 week, 4 days ago (2017-04-18 08:35:37 UTC) #68
horo(ooo until May 8)
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 ...
1 week, 4 days 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 ...
1 week, 3 days 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: ...
1 week, 3 days ago (2017-04-19 03:50:49 UTC) #75
horo(ooo until May 8)
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 > ...
1 week, 3 days 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 ...
1 week, 3 days ago (2017-04-19 05:19:27 UTC) #77
horo(ooo until May 8)
On 2017/04/19 05:19:27, kinuko wrote: > On 2017/04/19 04:30:22, horo wrote: > > On 2017/04/19 ...
1 week, 3 days 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 ...
1 week, 3 days ago (2017-04-19 11:08:14 UTC) #82
horo(ooo until May 8)
On 2017/04/19 10:36:32, horo wrote: > On 2017/04/19 05:19:27, kinuko wrote: > > On 2017/04/19 ...
1 week, 3 days ago (2017-04-19 11:36:14 UTC) #83
horo(ooo until May 8)
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 ...
1 week, 3 days 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 ...
1 week, 2 days 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 ...
1 week, 2 days 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, ...
1 week, 2 days 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 ...
1 week, 2 days ago (2017-04-20 04:27:00 UTC) #96
horo(ooo until May 8)
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)); ...
1 week, 2 days 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): ...
1 week, 1 day 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 ...
1 week, 1 day ago (2017-04-21 09:30:44 UTC) #111
horo(ooo until May 8)
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 != ...
1 week, 1 day ago (2017-04-21 09:42:32 UTC) #114
horo(ooo until May 8)
estark@ Could you please review .mojom files and content_browser_manifest.json?
1 week, 1 day 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 ...
4 days, 8 hours ago (2017-04-25 06:41:08 UTC) #120
horo(ooo until May 8)
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: > ...
4 days, 6 hours 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 days, 19 hours 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 days, 18 hours 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 days, 18 hours 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 days, 18 hours 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 days, 17 hours 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 days, 17 hours 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 days, 16 hours ago (2017-04-25 22:57:23 UTC) #136
commit-bot: I haz the power
3 days, 13 hours 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46