Nope, WorkerScriptLoader is also not currently acceptable. For now, using ThreadableLoader. Rename: Move ServiceWorkerJob from FetchLoader to ThreadableLoader

(In reply to Brady Eidson from comment #1) > Nope, WorkerScriptLoader is also not currently acceptable. What is the issue with WorkerScriptLoader? > For now, using ThreadableLoader. > > Rename: > Move ServiceWorkerJob from FetchLoader to ThreadableLoader That will not really help. Might be better to postpone this change until there is a good plan.

Created attachment 319632 [details] Patch

(In reply to youenn fablet from comment #2) > (In reply to Brady Eidson from comment #1) > > Nope, WorkerScriptLoader is also not currently acceptable. > > What is the issue with WorkerScriptLoader? > > > For now, using ThreadableLoader. > > > > Rename: > > Move ServiceWorkerJob from FetchLoader to ThreadableLoader > > That will not really help. > Might be better to postpone this change until there is a good plan. It gets us closer. It's definitely not right, but it's "less wrong" I definitely want to remove notions of using FetchLoader before I move on. :)

(In reply to youenn fablet from comment #2) > (In reply to Brady Eidson from comment #1) > > Nope, WorkerScriptLoader is also not currently acceptable. > > What is the issue with WorkerScriptLoader? You can only feed it a URL and can't get the data as it streams in. Additionally, it seems to be designed to be used *from Workers* but whatever we settle on needs to be used from both Documents and Workers.

Comment on attachment 319632 [details] Patch r=me. There is no need to create a FetchRequest object here so this is better like this for now. Since FetchLoader.h/FetchLoaderClient.h are no longer used in WebKit2, they could be reverted to not being "Private" headers. View in context: https://bugs.webkit.org/attachment.cgi?id=319632&action=review > Source/WebCore/loader/ThreadableLoader.h:46 > +enum PreflightPolicy { Would be nice as enum class, but hey...

Created attachment 319640 [details] Patch

Comment on attachment 319640 [details] Patch The iOS-sim failure is an infrastructure issue, not an issue with this patch

Comment on attachment 319640 [details] Patch Clearing flags on attachment: 319640 Committed r221495: <http://trac.webkit.org/changeset/221495>

All reviewed patches have been landed. Closing bug.

After this patch landed there were multiple Layout test crashing on El Capitan WK2: imported/w3c/web-platform-tests/FileAPI/idlharness.html imported/w3c/web-platform-tests/fetch/api/policies/referrer-no-referrer-worker.html imported/w3c/web-platform-tests/fetch/api/policies/referrer-origin-when-cross-origin-worker.html imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-storage-match.https.html imported/w3c/web-platform-tests/streams/piping/close-propagation-backward.sharedworker.html imported/w3c/web-platform-tests/streams/readable-streams/bad-underlying-sources.dedicatedworker.html https://build.webkit.org/builders/Apple%20El%20Capitan%20Debug%20WK2%20%28Tests%29/builds/2841 https://build.webkit.org/results/Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/r221495%20(2841)/results.html Crash: https://build.webkit.org/results/Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/r221495%20(2841)/imported/w3c/web-platform-tests/fetch/api/policies/referrer-no-referrer-worker-crash-log.txt

(In reply to Matt Lewis from comment #11) > After this patch landed there were multiple Layout test crashing on El > Capitan WK2: > imported/w3c/web-platform-tests/FileAPI/idlharness.html > > imported/w3c/web-platform-tests/fetch/api/policies/referrer-no-referrer- > worker.html > > imported/w3c/web-platform-tests/fetch/api/policies/referrer-origin-when- > cross-origin-worker.html > > imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/ > cache-storage-match.https.html > > imported/w3c/web-platform-tests/streams/piping/close-propagation-backward. > sharedworker.html > > imported/w3c/web-platform-tests/streams/readable-streams/bad-underlying- > sources.dedicatedworker.html > > https://build.webkit.org/builders/ > Apple%20El%20Capitan%20Debug%20WK2%20%28Tests%29/builds/2841 > https://build.webkit.org/results/ > Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/r221495%20(2841)/results.html > > Crash: > https://build.webkit.org/results/ > Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/r221495%20(2841)/imported/w3c/ > web-platform-tests/fetch/api/policies/referrer-no-referrer-worker-crash-log. > txt Specifically, this is an assertion failure: ASSERTION FAILED: m_loader /Volumes/Data/slave/elcapitan-debug/build/Source/WebCore/workers/service/ServiceWorkerJob.cpp(136) : virtual void WebCore::ServiceWorkerJob::didFail(const WebCore::ResourceError &) 1 0x1149ff8c0 WTFCrash

Reverted r221495 for reason: This change introduced assertion failures on El Capitan Debug WK2. Committed r221646: <http://trac.webkit.org/changeset/221646>