Now that service worker registrations are in the network process, we could consider serve service worker content through the regular network process loading code path. The first step is to make ServiceWorkerClientFetch closer to WebResourceLoader.
Created attachment 362019 [details] Patch
Created attachment 362067 [details] Patch
Comment on attachment 362067 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=362067&action=review > Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.cpp:220 > + }, WorkerRunLoop::defaultMode()); Could we make this a default parameter? > Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.h:65 > + void didReceiveRedirectResponse(WebCore::ResourceResponse&&); Do we want to just inform that a redirect happened, or will we in the future have a need for the service worker to be able to reject the redirect? > Source/WebKit/WebProcess/Storage/WebServiceWorkerFetchTaskClient.cpp:201 > + auto formData = WTFMove(m_formData); > + didReceiveFormDataAndFinish(formData.releaseNonNull()); Could we just call m_formData.releaseNonNull() without WTFMove? > Source/WebKit/WebProcess/Storage/WebServiceWorkerFetchTaskClient.h:86 > + RefPtr<WebCore::SharedBuffer> m_buffer; > + RefPtr<FormData> m_formData; Can these be a Variant, or could we have more than one of these? > Source/WebKit/WebProcess/Storage/WebServiceWorkerFetchTaskClient.h:88 > + Optional<ResourceError> m_error; Making this a std::unique_ptr would save memory in the common case.
Created attachment 362140 [details] Patch for landing
Thanks for the review. (In reply to Alex Christensen from comment #3) > Comment on attachment 362067 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=362067&action=review > > > Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.cpp:220 > > + }, WorkerRunLoop::defaultMode()); > > Could we make this a default parameter? Sure, could be done as a separate patch. > > Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.h:65 > > + void didReceiveRedirectResponse(WebCore::ResourceResponse&&); > > Do we want to just inform that a redirect happened, or will we in the future > have a need for the service worker to be able to reject the redirect? When fetch event respondWith is called, there is no way back. A DocumentLoader may trigger another service worker fetch which will be treated as another fetch event. > > Source/WebKit/WebProcess/Storage/WebServiceWorkerFetchTaskClient.cpp:201 > > + auto formData = WTFMove(m_formData); > > + didReceiveFormDataAndFinish(formData.releaseNonNull()); > > Could we just call m_formData.releaseNonNull() without WTFMove? > > > Source/WebKit/WebProcess/Storage/WebServiceWorkerFetchTaskClient.h:86 > > + RefPtr<WebCore::SharedBuffer> m_buffer; > > + RefPtr<FormData> m_formData; > > Can these be a Variant, or could we have more than one of these? > > > Source/WebKit/WebProcess/Storage/WebServiceWorkerFetchTaskClient.h:88 > > + Optional<ResourceError> m_error; > > Making this a std::unique_ptr would save memory in the common case. I updated the patch to make all of these a Variant with proper Ref<>, UniqueRef<>.
Comment on attachment 362140 [details] Patch for landing Clearing flags on attachment: 362140 Committed r241603: <https://trac.webkit.org/changeset/241603>
All reviewed patches have been landed. Closing bug.
<rdar://problem/48120172>
Reopening to attach new patch.
Created attachment 362146 [details] Patch
Comment on attachment 362146 [details] Patch Clearing flags on attachment: 362146 Committed r241609: <https://trac.webkit.org/changeset/241609>