Port MessagePort to the HTML5 event loop

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Chris Dumez]

Port MessagePort to the HTML5 event loop.

[Chris Dumez]

Created attachment 383055 [details]
Patch

[Ryosuke Niwa]

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

[Chris Dumez]

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?

[Ryosuke Niwa]

(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.

[Chris Dumez]

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

[Radar WebKit Bug Importer]

<rdar://problem/57000643>

[Aakash Jain]

(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

[Truitt Savell]

(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