[Webkit-unassigned] [Bug 17331] Change postMessage/MessageEvent to match HTML5 wrt. exposing origin vs. domain/uri

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=17331





------- Comment #37 from collinj-webkit at collinjackson.com  2008-04-20 12:54 PDT -------
Created an attachment (id=20703)
 --> (http://bugs.webkit.org/attachment.cgi?id=20703&action=view)
Updated patch to address aroben's comments

(In reply to comment #36)
> (From update of attachment 20631 [edit])
> r31975 added protocolHostAndPortAreEqual to KURL.h. Should
> we use that instead of adding SecurityOrigin::isSameSchemeHostPort?

What is protocolHostAndPortAreEqual supposed to be used for? Right now it
doesn't seem to be called anywhere. It seems better to have origin-related
security checks be done by the SecurityOrigin class.

> +    // Sender is not allowed to see exceptions other than syntax errors
> +    ExceptionCode ec; 
> +    document()->dispatchEvent(new MessageEvent(message, sourceOrigin, source),
> ec, true);
> 
> Should ec be initialized to 0?

Since it's just a placeholder for a return value that we throw away, I don't
think it needs to be initialized. There are other examples of this pattern in
WebKit.

> +        [DoNotCheckDomainSecurity, Custom] void postMessage(in DOMString
> message, in [Optional] DOMString origin)
> 
> Let's call it targetOrigin here as well.

Fixed.

> +    if (m_port) {
> +      append(result, ":");
> +      append(result, String::number(m_port));
> +    }
> 
> Looks like the indentation got messed up here.

Fixed.

> +        // Okay deleted value because "invalid-protocol" is not a valid
> protocol.
> 
> What makes it invalid?

Only convention. We tightened up the interface to make it hard to construct
nonsense security origins, but SecurityOriginHash wants a nonsense security
origin anyway. The code was relying on a different convention (file URLs don't
have non-zero ports) before, but it's no longer possible to construct that
security origin, so we went with a nonsense scheme instead.

I've changed the comment to the following: 
// Ok deleted value assuming "invalid-protocol" is not a valid protocol

> You should use svn cp when renaming the tests so that the history is preserved
> (maybe  you already did this and it didn't come through in the diff).

Fixed.

I think this patch is ready to land.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.