[webkit-reviews] review cancelled: [Bug 17331] Change postMessage/MessageEvent to match HTML5 wrt. exposing origin vs. domain/uri : [Attachment 19338] Updated patch that addresses aroben's comments

[bugzilla-daemon at webkit.org]

Collin Jackson <collinj-webkit at collinjackson.com> has cancelled Collin Jackson
<collinj-webkit at collinjackson.com>'s request for review:
Bug 17331: Change postMessage/MessageEvent to match HTML5 wrt. exposing origin
vs. domain/uri
http://bugs.webkit.org/show_bug.cgi?id=17331

Attachment 19338: Updated patch that addresses aroben's comments
http://bugs.webkit.org/attachment.cgi?id=19338&action=edit

------- Additional Comments from Collin Jackson
<collinj-webkit at collinjackson.com>
(In reply to comment #25)
> > 1) equalIgnoringDomain compares hosts, and is used for postMessage
> 
> Should we just call this isSameSchemeHostPort to match HTML 5's terminology?

Done.

> > 2) equal compares all aspects of the security origin, and is used for hash
keys
> 
> Maybe we should get rid of this method entirely and put the implementation in

> SecurityOriginHash. It seems like it will only confuse people to have this
> public method on SecurityOrigin that you're never supposed to call.

I am open to the idea of removing equal(), but it seems like it might be useful
for something. Sam and I were discussing whether it might make sense to use it
for isSecureTransitionTo. I think it might be better to leave it in for the
next few weeks until Sam has had a chance to implement data: URLs with GUIDs. I
added a FIXME comment describing your suggestion, but I can refactor it out now
in this patch if you prefer.

> > We changed SecurityOrigin::toString() and
SecurityOrigin::stringIdentifier() to
> > generate identifiers that are suitable for being used as a MessageEvent's
> > origin property.
> 
> Will these changes interfere with any existing uses of
SecurityOrigin::toString
> or SecurityOrigin::stringIdentifier?

The main consequence of this change will be that we lose track of any
pre-existing storage databases. Since there doesn't seem to be anyone shipping
a version of WebKit that supports databases yet, I don't think this change will
have a major impact. The advantage is that we remove redundant code: HTML 5
requires the scheme://host:port way of representing origins for postMessage, so
there's no reason to have another way of doing it as well.