[webkit-reviews] review requested: [Bug 20879] Implement HTML5 channel messaging : [Attachment 23573] updated patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has asked  for review:
Bug 20879: Implement HTML5 channel messaging
https://bugs.webkit.org/show_bug.cgi?id=20879

Attachment 23573: updated patch
https://bugs.webkit.org/attachment.cgi?id=23573&action=edit

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
> We should use the lexicalGlobalObject.

Fixed.

> Also, maybe I missed it, but how to you keep the MessagePorts, etc, from
> holding on to pointers to Frames that have been deallocated?

Fixed this (it did crash), and several more instances of hanging references.
Added a test case.

(In reply to comment #4)
> 1) A parent frame grabs a closure from a child frame, navigates the child,
> waits for the navigation to finish, and then calls the closure (which
> uses/creates a channel).

Currently, messages sent through the created port have the new origin. More
precisely,

Subframe:
window.createClosure = function() {
    var MessageChannelConstructor = window.MessageChannel;
    return function() {
	var channel = new MessageChannelConstructor; // can't use
window.MessageChannel
	channel.port1.postMessage("ping");
	return channel.port2;
    }
}

Main frame:
var closure = window.frames[0].createClosure();
... navigate subframe from 127.0.0.1 to localhost
var mainPort = closure();
mainPort.onmessage = function(evt) {
   alert(evt.origin); // http://localhost:8000
}
mainPort.start();

Is this correct behavior?

> 2) A frame uses/creates a channel, synchronously navigates to a new security
> origin (see LayoutTests/http/tests/security/xss-DENIED-synchronous-form.html
> for an example of how to do this), and then uses/creates another channel.

I'm not sure what this means - what kinds of issues I can expect if a new
channel is created?

> In both cases, we should also test the situation in which the channel is
> created before the navigation and used after the navigation.

Noted, but haven't tried this yet, as I'm also not sure about expected
behavior.