Bug 66789

Created attachment 104877 [details] Patch

Comment on attachment 104877 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104877&action=review > LayoutTests/fast/workers/resources/worker-thread-multi-port.js:4 > + testPassed("event.ports is non-null and zero- ength when no port sent"); I think this is an editing mistake where you turned the "l" into a space instead of turning the"-" into a space.

Created attachment 104921 [details] Patch

Comment on attachment 104877 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104877&action=review > Source/WebCore/bindings/js/JSMessageEventCustom.cpp:49 > MessagePortArray* ports = static_cast<MessageEvent*>(impl())->ports(); > if (!ports || ports->isEmpty()) > - return jsNull(); > + return constructEmptyArray(exec, globalObject()); It seems there is no need to check ports->isEmpty() any more here, since empty is no longer a special case. Unless we want a performance optimization for the empty case. It also seems to me we could change things so a MessageEvent always has a MessagePortArray in it, and then we might not need a custom binding at all!

Comment on attachment 104921 [details] Patch I made a comment on the old patch that still applies. Seems to me that the “ports is an empty array instead of null” rule could be in the DOM instead of in the binding.

(In reply to comment #4) > (From update of attachment 104877 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=104877&action=review > > > Source/WebCore/bindings/js/JSMessageEventCustom.cpp:49 > > MessagePortArray* ports = static_cast<MessageEvent*>(impl())->ports(); > > if (!ports || ports->isEmpty()) > > - return jsNull(); > > + return constructEmptyArray(exec, globalObject()); > > It seems there is no need to check ports->isEmpty() any more here, since empty is no longer a special case. Unless we want a performance optimization for the empty case. Good point. I'll clean that up before check-in. > > It also seems to me we could change things so a MessageEvent always has a MessagePortArray in it, and then we might not need a custom binding at all! I tried this and failed -- probably something simple. (Along with a few other changes) I tried to express the array as follows - readonly attribute [CustomGetter] Array ports; + readonly attribute MessagePort[] ports; but that didn't work and I couldn't quickly figure out how to do the array, so I filed 66829 to do this later.

(In reply to comment #6) > I tried this and failed -- probably something simple. (Along with a few other changes) I tried to express the array as follows > - readonly attribute [CustomGetter] Array ports; > + readonly attribute MessagePort[] ports; > but that didn't work and I couldn't quickly figure out how to do the array, so I filed 66829 to do this later. I also tried readonly attribute Array ports; and had problems.

Committed as http://trac.webkit.org/changeset/93709