Bug 163189 – MessageEvent.ports should be nullable

• Status: RESOLVED
• Resolution: WONTFIX
• Component: DOM

[Chris Dumez]

MessageEvent.ports should be nullable:
- https://html.spec.whatwg.org/multipage/comms.html#the-messageevent-interfaces

Firefox and Chrome agree with the specification.

[Chris Dumez]

Created attachment 291059 [details]
Patch

[Sam Weinig]

Comment on attachment 291059 [details]
Patch

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

> Source/WebCore/dom/MessageEvent.h:44
> +    std::unique_ptr<MessagePortArray> ports;

Can this be Optional<MessagePortArray> instead? Do you have any memory of why this is a unique_ptr in MessageEvent itself? (Doesn't seem like the overhead of an empty Vector would worth it for a transient event).

[Chris Dumez]

(In reply to comment #2)
> Comment on attachment 291059 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=291059&action=review
> 
> > Source/WebCore/dom/MessageEvent.h:44
> > +    std::unique_ptr<MessagePortArray> ports;
> 
> Can this be Optional<MessagePortArray> instead? Do you have any memory of
> why this is a unique_ptr in MessageEvent itself? (Doesn't seem like the
> overhead of an empty Vector would worth it for a transient event).

I do not know why the implementation is using unique_ptr<>. I guess it is possible we did not have Optional<> at the time. Anyway, I'll look into using Optional<> tomorrow. Hopefully this is not too much refactoring.

[Darin Adler]

Comment on attachment 291059 [details]
Patch

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

> Source/WebCore/bindings/js/JSMessageEventCustom.cpp:79
>          if (RefPtr<SerializedScriptValue> serializedValue = event.dataAsSerializedScriptValue()) {

Should use auto.

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:5442
> +    Ref<${interfaceName}> event = ${interfaceName}::createForBindings(eventType, WTFMove(eventInit));

Should use auto.

[Chris Dumez]

I filed a bug against the spec to see if it would be possible to make the attribute non-nullable in the specification instead:
- https://github.com/whatwg/html/issues/1882

Making this attribute nullable is not really useful (except for aligning with other browsers).

[Chris Dumez]

I updated the specification instead via https://github.com/whatwg/html/commit/df2c0c448612d5b8ab85ad3c6ce0255ee11c0b01