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

[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 120316: patch 5
https://bugs.webkit.org/attachment.cgi?id=120316&action=review

------- Additional Comments from Per-Erik Brodin <per-erik.brodin at ericsson.com>
> (From update of attachment 111298 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=111298&action=review
> 
> Looks like the spec changed substantially since the patch was posted.
> 
> > Source/WebCore/page/EventSource.cpp:119
> >	 options.allowCredentials = AllowStoredCredentials;
> 
> The spec says that credentials should not be sent cross-origin unless
constructor is called with a second argument telling it to allow credentials:

Fixed.

> There should also be a readonly withCredentials attribute on EventSource
object.

Fixed.

> > Source/WebCore/page/EventSource.cpp:256
> > +	 bool failedResourceSharingCheck = error.domain() ==
errorDomainWebKitInternal;
> 
> That's terribly fragile. We need a better way to communicate this to
EventSource code.

Yes, that doesn't even work anymore since now errorDomainWebKitInternal is used
for other error conditions as well.
Fixed.