Created attachment 383014 [details] Patch

Comment on attachment 383014 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383014&action=review > Source/WebCore/workers/Worker.cpp:208 > + queueTaskToDispatchEvent(*this, TaskSource::DOMManipulation, Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::Yes)); Shouldn't this be Networking? > Source/WebCore/workers/WorkerMessagingProxy.cpp:107 > - workerObject->enqueueEvent(MessageEvent::create(WTFMove(ports), message.message.releaseNonNull())); > + ActiveDOMObject::queueTaskToDispatchEvent(*workerObject, TaskSource::DOMManipulation, MessageEvent::create(WTFMove(ports), message.message.releaseNonNull())); Shouldn't this be unshipped port message queue? https://html.spec.whatwg.org/multipage/web-messaging.html#message-port-post-message-steps > Source/WebCore/workers/WorkerMessagingProxy.cpp:180 > - workerObject->enqueueEvent(ErrorEvent::create(errorMessage, sourceURL, lineNumber, columnNumber, { })); > + ActiveDOMObject::queueTaskToDispatchEvent(*workerObject, TaskSource::DOMManipulation, ErrorEvent::create(errorMessage, sourceURL, lineNumber, columnNumber, { })); Ditto.

Comment on attachment 383014 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383014&action=review >> Source/WebCore/workers/Worker.cpp:208 >> + queueTaskToDispatchEvent(*this, TaskSource::DOMManipulation, Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::Yes)); > > Shouldn't this be Networking? The spec says: "The task source for the tasks mentioned above is the DOM manipulation task source."

Comment on attachment 383014 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383014&action=review >> Source/WebCore/workers/WorkerMessagingProxy.cpp:107 >> + ActiveDOMObject::queueTaskToDispatchEvent(*workerObject, TaskSource::DOMManipulation, MessageEvent::create(WTFMove(ports), message.message.releaseNonNull())); > > Shouldn't this be unshipped port message queue? > https://html.spec.whatwg.org/multipage/web-messaging.html#message-port-post-message-steps We don't exactly match the spec here indeed but I am unclear how to exactly match the spec. Would you like me to add a FIXME comment?

(In reply to Chris Dumez from comment #4) > Comment on attachment 383014 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=383014&action=review > > >> Source/WebCore/workers/WorkerMessagingProxy.cpp:107 > >> + ActiveDOMObject::queueTaskToDispatchEvent(*workerObject, TaskSource::DOMManipulation, MessageEvent::create(WTFMove(ports), message.message.releaseNonNull())); > > > > Shouldn't this be unshipped port message queue? > > https://html.spec.whatwg.org/multipage/web-messaging.html#message-port-post-message-steps > > We don't exactly match the spec here indeed but I am unclear how to exactly > match the spec. > Would you like me to add a FIXME comment? I think using DOM manipulation task source is super confusing here because it clearly isn't that. I think the closest thing is https://html.spec.whatwg.org/multipage/web-messaging.html#port-message-queue See https://html.spec.whatwg.org/multipage/web-messaging.html#message-port-post-message-steps, which is invoked by postMessage on Worker, which uses the implicitly defined MessagePort of Worker.

Comment on attachment 383014 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383014&action=review >>>> Source/WebCore/workers/WorkerMessagingProxy.cpp:107 >>>> + ActiveDOMObject::queueTaskToDispatchEvent(*workerObject, TaskSource::DOMManipulation, MessageEvent::create(WTFMove(ports), message.message.releaseNonNull())); >>> >>> Shouldn't this be unshipped port message queue? >>> https://html.spec.whatwg.org/multipage/web-messaging.html#message-port-post-message-steps >> >> We don't exactly match the spec here indeed but I am unclear how to exactly match the spec. >> Would you like me to add a FIXME comment? > > I think using DOM manipulation task source is super confusing here because it clearly isn't that. I think the closest thing is https://html.spec.whatwg.org/multipage/web-messaging.html#port-message-queue > > See https://html.spec.whatwg.org/multipage/web-messaging.html#message-port-post-message-steps, which is invoked by postMessage on Worker, which uses the implicitly defined MessagePort of Worker. I used DOMManipulation for consistency with the rest of the spec. Yes, the spec is using DOMManipulation for things that are not DOMManipulation. See also: https://w3c.github.io/ServiceWorker/#dom-serviceworker-postmessage-message-options Even in service workers, the spec says to use DOMManipulation for postMessage.

(In reply to Chris Dumez from comment #6) > Comment on attachment 383014 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=383014&action=review > > >>>> Source/WebCore/workers/WorkerMessagingProxy.cpp:107 > >>>> + ActiveDOMObject::queueTaskToDispatchEvent(*workerObject, TaskSource::DOMManipulation, MessageEvent::create(WTFMove(ports), message.message.releaseNonNull())); > >>> > >>> Shouldn't this be unshipped port message queue? > >>> https://html.spec.whatwg.org/multipage/web-messaging.html#message-port-post-message-steps > >> > >> We don't exactly match the spec here indeed but I am unclear how to exactly match the spec. > >> Would you like me to add a FIXME comment? > > > > I think using DOM manipulation task source is super confusing here because it clearly isn't that. I think the closest thing is https://html.spec.whatwg.org/multipage/web-messaging.html#port-message-queue > > > > See https://html.spec.whatwg.org/multipage/web-messaging.html#message-port-post-message-steps, which is invoked by postMessage on Worker, which uses the implicitly defined MessagePort of Worker. > > I used DOMManipulation for consistency with the rest of the spec. Yes, the > spec is using DOMManipulation for things that are not DOMManipulation. See > also: > https://w3c.github.io/ServiceWorker/#dom-serviceworker-postmessage-message- > options > > Even in service workers, the spec says to use DOMManipulation for > postMessage. That's very strange. Perhaps we need a spec bug to clarify what task source is used for messaging. I'd much prefer having a single task source for all messaging related APIs than ramp it together with DOM manipulation.

Committed r252212: <https://trac.webkit.org/changeset/252212>

<rdar://problem/57000977>