Bug 178839 – messageEvent.source can also be a ServiceWorker

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

messageEvent.source can also be a ServiceWorker:
- https://html.spec.whatwg.org/multipage/comms.html#messageeventsource

[Chris Dumez]

Created attachment 324933 [details]
Patch

[Brady Eidson]

Comment on attachment 324933 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=324933&action=review

> Source/WebCore/dom/MessageEvent.h:46
> -using MessageEventSource = Variant<RefPtr<DOMWindow>, RefPtr<MessagePort>>;
> +using MessageEventSource = Variant<RefPtr<DOMWindow>, RefPtr<MessagePort>
> +#if ENABLE(SERVICE_WORKER)
> +    , RefPtr<ServiceWorker>
> +#endif
> +    >;

This is gross. I'd almost prefer a single line duplicating all 3 possibilities for the #if ENABLE() case.

> Source/WebCore/dom/MessageEvent.idl:33
> +#if defined(ENABLE_SERVICE_WORKER) && ENABLE_SERVICE_WORKER
> +typedef (DOMWindow or MessagePort or ServiceWorker) MessageEventSource;
> +#else
> +typedef (DOMWindow or MessagePort) MessageEventSource;
> +#endif

Like this is what I was alluding to up there ^^^

[Chris Dumez]

Created attachment 324944 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 324944 [details]
Patch

Clearing flags on attachment: 324944

Committed r224000: <https://trac.webkit.org/changeset/224000>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Sam Weinig]

:(. I was getting so close to getting rid of the preprocessor for IDL files. 

Would an alternative approach, where in non-ENABLE(SERVICE_WORKER) we have a dummy ServiceWorker class work in this case (I am considering this for the non-ENABLE(VIDEO) case). Without a way to get a ServiceWorker from script, the fact that it is included in the union should not be observable.

[Radar WebKit Bug Importer]

<rdar://problem/35568076>