Bug 203960

Created attachment 383055 [details] Patch

Comment on attachment 383055 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383055&action=review > Source/WebCore/dom/MessagePort.cpp:278 > - m_eventQueue->enqueueEvent(MessageEvent::create(WTFMove(ports), message.message.releaseNonNull())); > + queueTaskToDispatchEvent(*this, TaskSource::PostedMessage, MessageEvent::create(WTFMove(ports), message.message.releaseNonNull())); Hm... the posted message task source is only used for window post message steps: https://html.spec.whatwg.org/multipage/web-messaging.html#window-post-message-steps https://html.spec.whatwg.org/multipage/web-messaging.html#posted-message-task-source Shouldn't this be port message queue or unshipped port message queue? https://html.spec.whatwg.org/multipage/web-messaging.html#port-message-queue

Comment on attachment 383055 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383055&action=review >> Source/WebCore/dom/MessagePort.cpp:278 >> + queueTaskToDispatchEvent(*this, TaskSource::PostedMessage, MessageEvent::create(WTFMove(ports), message.message.releaseNonNull())); > > Hm... the posted message task source is only used for window post message steps: > https://html.spec.whatwg.org/multipage/web-messaging.html#window-post-message-steps > https://html.spec.whatwg.org/multipage/web-messaging.html#posted-message-task-source > > Shouldn't this be port message queue or unshipped port message queue? > https://html.spec.whatwg.org/multipage/web-messaging.html#port-message-queue Looks like you're right, the spec says: "Add a task that runs the following steps to the port message queue of targetPort:" How do you propose I match the spec though?

(In reply to Chris Dumez from comment #3) > Comment on attachment 383055 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=383055&action=review > > >> Source/WebCore/dom/MessagePort.cpp:278 > >> + queueTaskToDispatchEvent(*this, TaskSource::PostedMessage, MessageEvent::create(WTFMove(ports), message.message.releaseNonNull())); > > > > Hm... the posted message task source is only used for window post message steps: > > https://html.spec.whatwg.org/multipage/web-messaging.html#window-post-message-steps > > https://html.spec.whatwg.org/multipage/web-messaging.html#posted-message-task-source > > > > Shouldn't this be port message queue or unshipped port message queue? > > https://html.spec.whatwg.org/multipage/web-messaging.html#port-message-queue > > Looks like you're right, the spec says: > "Add a task that runs the following steps to the port message queue of > targetPort:" > > How do you propose I match the spec though? I think we just add TaskSource type of PostMessageQueue. Maybe add a comment saying that the spec says each port has its own task source but we coalesce into one. Because HTML5 event loop spec allows whatever task source to be picked, this is okay for our implementation.

Committed r252209: <https://trac.webkit.org/changeset/252209>

<rdar://problem/57000643>

(In reply to Chris Dumez from comment #5) > Committed r252209: <https://trac.webkit.org/changeset/252209> This likely caused layout Test fast/workers/worker-close-more.html to fail. Tracked in https://bugs.webkit.org/show_bug.cgi?id=203995

(In reply to Aakash Jain from comment #7) > (In reply to Chris Dumez from comment #5) > > Committed r252209: <https://trac.webkit.org/changeset/252209> > This likely caused layout Test fast/workers/worker-close-more.html to fail. > Tracked in https://bugs.webkit.org/show_bug.cgi?id=203995 I was able to reproduce this by just running the test. It fails on 252209 but not on 252208