Update WorkletGlobalScope.addModule() to actually fetch the module script.
Created attachment 410357 [details] Patch
Created attachment 410360 [details] Patch
Created attachment 410365 [details] Patch
Created attachment 410392 [details] Patch
Comment on attachment 410392 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=410392&action=review > Source/WebCore/loader/ThreadableLoader.cpp:115 > + if (is<WorkerGlobalScope>(context) || (is<WorkletGlobalScope>(context) && downcast<WorkletGlobalScope>(context).workerOrWorkletThread())) > + return WorkerThreadableLoader::create(static_cast<WorkerOrWorkletGlobalScope&>(context), client, WorkerRunLoop::defaultMode(), WTFMove(request), options, WTFMove(referrer)); > + > + Document* document = nullptr; > + if (is<WorkletGlobalScope>(context)) > + document = downcast<WorkletGlobalScope>(context).responsibleDocument(); > + else > + document = &downcast<Document>(context); > > - return DocumentThreadableLoader::create(downcast<Document>(context), client, WTFMove(request), options, WTFMove(referrer)); > + return DocumentThreadableLoader::create(*document, client, WTFMove(request), options, WTFMove(referrer)); I don't quite understand this logic. When do you have a WorkletGlobalScope but not a workerOrWorkletThread?
(In reply to Sam Weinig from comment #5) > Comment on attachment 410392 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=410392&action=review > > > Source/WebCore/loader/ThreadableLoader.cpp:115 > > + if (is<WorkerGlobalScope>(context) || (is<WorkletGlobalScope>(context) && downcast<WorkletGlobalScope>(context).workerOrWorkletThread())) > > + return WorkerThreadableLoader::create(static_cast<WorkerOrWorkletGlobalScope&>(context), client, WorkerRunLoop::defaultMode(), WTFMove(request), options, WTFMove(referrer)); > > + > > + Document* document = nullptr; > > + if (is<WorkletGlobalScope>(context)) > > + document = downcast<WorkletGlobalScope>(context).responsibleDocument(); > > + else > > + document = &downcast<Document>(context); > > > > - return DocumentThreadableLoader::create(downcast<Document>(context), client, WTFMove(request), options, WTFMove(referrer)); > > + return DocumentThreadableLoader::create(*document, client, WTFMove(request), options, WTFMove(referrer)); > > I don't quite understand this logic. When do you have a WorkletGlobalScope > but not a workerOrWorkletThread? A Worklet does not necessarily use threading. PaintWorklets do not use threading, AudioWorklets do.
(In reply to Chris Dumez from comment #6) > (In reply to Sam Weinig from comment #5) > > Comment on attachment 410392 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=410392&action=review > > > > > Source/WebCore/loader/ThreadableLoader.cpp:115 > > > + if (is<WorkerGlobalScope>(context) || (is<WorkletGlobalScope>(context) && downcast<WorkletGlobalScope>(context).workerOrWorkletThread())) > > > + return WorkerThreadableLoader::create(static_cast<WorkerOrWorkletGlobalScope&>(context), client, WorkerRunLoop::defaultMode(), WTFMove(request), options, WTFMove(referrer)); > > > + > > > + Document* document = nullptr; > > > + if (is<WorkletGlobalScope>(context)) > > > + document = downcast<WorkletGlobalScope>(context).responsibleDocument(); > > > + else > > > + document = &downcast<Document>(context); > > > > > > - return DocumentThreadableLoader::create(downcast<Document>(context), client, WTFMove(request), options, WTFMove(referrer)); > > > + return DocumentThreadableLoader::create(*document, client, WTFMove(request), options, WTFMove(referrer)); > > > > I don't quite understand this logic. When do you have a WorkletGlobalScope > > but not a workerOrWorkletThread? > > A Worklet does not necessarily use threading. PaintWorklets do not use > threading, AudioWorklets do. Oh, that's not confusing at all!
(In reply to Sam Weinig from comment #7) > (In reply to Chris Dumez from comment #6) > > (In reply to Sam Weinig from comment #5) > > > Comment on attachment 410392 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=410392&action=review > > > > > > > Source/WebCore/loader/ThreadableLoader.cpp:115 > > > > + if (is<WorkerGlobalScope>(context) || (is<WorkletGlobalScope>(context) && downcast<WorkletGlobalScope>(context).workerOrWorkletThread())) > > > > + return WorkerThreadableLoader::create(static_cast<WorkerOrWorkletGlobalScope&>(context), client, WorkerRunLoop::defaultMode(), WTFMove(request), options, WTFMove(referrer)); > > > > + > > > > + Document* document = nullptr; > > > > + if (is<WorkletGlobalScope>(context)) > > > > + document = downcast<WorkletGlobalScope>(context).responsibleDocument(); > > > > + else > > > > + document = &downcast<Document>(context); > > > > > > > > - return DocumentThreadableLoader::create(downcast<Document>(context), client, WTFMove(request), options, WTFMove(referrer)); > > > > + return DocumentThreadableLoader::create(*document, client, WTFMove(request), options, WTFMove(referrer)); > > > > > > I don't quite understand this logic. When do you have a WorkletGlobalScope > > > but not a workerOrWorkletThread? > > > > A Worklet does not necessarily use threading. PaintWorklets do not use > > threading, AudioWorklets do. > > Oh, that's not confusing at all! :) Per spec, Workets are completely thread-agnostic [1] and our current implementation of paint Worklets does not use threading at all. I am trying to add support for threading for AudioWorklets without breaking support for using the main thread so that I don't break our paint Worklets. [1] https://drafts.css-houdini.org/worklets/#motivations
(In reply to Chris Dumez from comment #8) > (In reply to Sam Weinig from comment #7) > > (In reply to Chris Dumez from comment #6) > > > (In reply to Sam Weinig from comment #5) > > > > Comment on attachment 410392 [details] > > > > Patch > > > > > > > > View in context: > > > > https://bugs.webkit.org/attachment.cgi?id=410392&action=review > > > > > > > > > Source/WebCore/loader/ThreadableLoader.cpp:115 > > > > > + if (is<WorkerGlobalScope>(context) || (is<WorkletGlobalScope>(context) && downcast<WorkletGlobalScope>(context).workerOrWorkletThread())) > > > > > + return WorkerThreadableLoader::create(static_cast<WorkerOrWorkletGlobalScope&>(context), client, WorkerRunLoop::defaultMode(), WTFMove(request), options, WTFMove(referrer)); > > > > > + > > > > > + Document* document = nullptr; > > > > > + if (is<WorkletGlobalScope>(context)) > > > > > + document = downcast<WorkletGlobalScope>(context).responsibleDocument(); > > > > > + else > > > > > + document = &downcast<Document>(context); > > > > > > > > > > - return DocumentThreadableLoader::create(downcast<Document>(context), client, WTFMove(request), options, WTFMove(referrer)); > > > > > + return DocumentThreadableLoader::create(*document, client, WTFMove(request), options, WTFMove(referrer)); > > > > > > > > I don't quite understand this logic. When do you have a WorkletGlobalScope > > > > but not a workerOrWorkletThread? > > > > > > A Worklet does not necessarily use threading. PaintWorklets do not use > > > threading, AudioWorklets do. > > > > Oh, that's not confusing at all! > > :) Per spec, Workets are completely thread-agnostic [1] and our current > implementation of paint Worklets does not use threading at all. I am trying > to add support for threading for AudioWorklets without breaking support for > using the main thread so that I don't break our paint Worklets. > > [1] https://drafts.css-houdini.org/worklets/#motivations I would never doubt that a Houdini spec use confusing naming :).
Comment on attachment 410392 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=410392&action=review r=me > Source/WebCore/Modules/webaudio/AudioWorkletGlobalScope.h:56 > + AudioWorkletThread* workerOrWorkletThread() final { return m_thread.ptr(); } I would just call this "thread". But if you prefer to call it "workerOrWorkletThread", let's rename the data member to match: m_workerOrWorkletThread. > Source/WebCore/Modules/webaudio/AudioWorkletThread.h:66 > + Ref<AudioWorkletMessagingProxy> m_messagingProxy; It seems like AudioWorkletMessagingProxy and AudioWorkletThread ref each other. Is this a cycle?
Comment on attachment 410392 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=410392&action=review > Source/WebCore/workers/WorkerGlobalScope.h:94 > + WorkerThread* workerOrWorkletThread() final { return &m_thread; } The reason I did not call this one thread() is because workerOrWorkletThread() can return null... > Source/WebCore/workers/WorkerGlobalScope.h:95 > WorkerThread& thread() const { return m_thread; } ... and WorkerGlobalScope already has a thread() function which guarantees it returns non-null.
Comment on attachment 410392 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=410392&action=review >> Source/WebCore/Modules/webaudio/AudioWorkletThread.h:66 >> + Ref<AudioWorkletMessagingProxy> m_messagingProxy; > > It seems like AudioWorkletMessagingProxy and AudioWorkletThread ref each other. Is this a cycle? Looks like it, good catch. Will fix.
Comment on attachment 410392 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=410392&action=review > Source/WebCore/ChangeLog:11 > + The WorkletGlobalScope maintains a queue a fetch requests (basically a URL and a completion handler) and uses Nit. s/a queue a fetch/a queue of fetch/
Created attachment 410647 [details] Patch
Created attachment 410648 [details] Patch
Committed r268057: <https://trac.webkit.org/changeset/268057> All reviewed patches have been landed. Closing bug and clearing flags on attachment 410648 [details].
<rdar://problem/70005642>