[webkit-reviews] review requested: [Bug 61862] EventSource should support CORS : [Attachment 104203] updated patch

[bugzilla-daemon at webkit.org]

Per-Erik Brodin <per-erik.brodin at ericsson.com> has asked  for review:
Bug 61862: EventSource should support CORS
https://bugs.webkit.org/show_bug.cgi?id=61862

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

------- Additional Comments from Per-Erik Brodin <per-erik.brodin at ericsson.com>
(In reply to comment #13)
> (From update of attachment 97728 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=97728&action=review
> 
> > Source/WebCore/loader/CrossOriginAccessControl.cpp:52
> >	 if (equalIgnoringCase(name, "accept")
> >	     || equalIgnoringCase(name, "accept-language")
> > +	     || equalIgnoringCase(name, "cache-control")
> >	     || equalIgnoringCase(name, "content-language")
> > +	     || equalIgnoringCase(name, "last-event-id")
> >	     || equalIgnoringCase(name, "origin"))
> >	     return true;
> 
> This list comes from the CORS spec.  It's not clear we should change it
unilaterally.

This change is no longer needed since the check is skipped altogether.

(In reply to comment #17)
> > This is the origin set on the dispatched message events, "the origin of the
event stream's URL" in the spec. I should probably rename this to
m_eventStreamOrigin or similar. This was the same as the script's origin when
only same origin requests were supported.
> 
> Makes sense, but don't we need to get that value from the final response in
the redirect chain rather than the first?  In either case, having a clearer
name would be very helpful.

m_origin renamed to m_eventStreamOrigin and now set from the response URL.
(makes no difference currently since it seems that WebKit doesn't support CORS
redirects)