The "ports" attribute in MessageEvent.idl is currently using a CustomGetter. The attribute is of type "MessagePortArray", which the JSC bindings generator has support for. Therefore, we should be able to get rid of the custom code with minor generator tweaks.
Created attachment 202062 [details] Patch
Comment on attachment 202062 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=202062&action=review > Source/WebCore/dom/MessageEvent.idl:37 > + [InitializedByEventConstructor] readonly attribute MessagePortArray ports; Per the spec, this should be 'MessagePort[]?'. Would you fix it in a follow-up patch? http://dev.w3.org/html5/postmsg/#event-definitions
Comment on attachment 202062 [details] Patch Clearing flags on attachment: 202062 Committed r150249: <http://trac.webkit.org/changeset/150249>
All reviewed patches have been landed. Closing bug.
(In reply to comment #2) > (From update of attachment 202062 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=202062&action=review > > > Source/WebCore/dom/MessageEvent.idl:37 > > + [InitializedByEventConstructor] readonly attribute MessagePortArray ports; > > Per the spec, this should be 'MessagePort[]?'. Would you fix it in a follow-up patch? > > http://dev.w3.org/html5/postmsg/#event-definitions This is visibly a recent spec change as it wasn't nullable before. I don't know how other browsers behave so this would need to be checked. Do you know the reason behind this change? Having a nullable array makes life harder for JS developers and I'm not sure why having an empty array is not enough.
(In reply to comment #5) > This is visibly a recent spec change as it wasn't nullable before. I don't know how other browsers behave so this would need to be checked. Do you know the reason behind this change? Having a nullable array makes life harder for JS developers and I'm not sure why having an empty array is not enough. Yeah, needs verification of other browsers. I don't know about nullable, but replacing XXXArray with XXX[] looks like a recent trend.
Comment on attachment 202062 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=202062&action=review > Source/WebCore/bindings/scripts/test/JS/JSTestSerializedScriptValueInterface.cpp:182 > + JSValue result = impl->ports() ? jsArray(exec, castedThis->globalObject(), *impl->ports()) : constructEmptyArray(exec, 0, castedThis->globalObject()); It seems unfortunate that we call into DOM twice now. Why can't the result of impl->ports() be cached?
(In reply to comment #7) > (From update of attachment 202062 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=202062&action=review > > > Source/WebCore/bindings/scripts/test/JS/JSTestSerializedScriptValueInterface.cpp:182 > > + JSValue result = impl->ports() ? jsArray(exec, castedThis->globalObject(), *impl->ports()) : constructEmptyArray(exec, 0, castedThis->globalObject()); > > It seems unfortunate that we call into DOM twice now. Why can't the result of impl->ports() be cached? I don't think it is a big issue currently because this code is only used for MessageEvent::ports() currently, which is inlined. However, you have a point that we should cache it to be safe. I will look into it, thanks.
Addressing ap and Kentaro's feedback in Bug 116653.