Bug 175628 – Cross origin Beacon requests with a ArrayBuffer / ArrayBufferView payload should not do a CORS preflight

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Chris Dumez]

Cross origin Beacon requests with a ArrayBuffer / ArrayBufferView payload should not do a CORS preflight.
Firefox and Chrome do not require a CORS preflight in this case. The reason it is not needed is because the request does not have a "Content-Type" header as per the specification.

[Radar WebKit Bug Importer]

<rdar://problem/33919278>

[Chris Dumez]

Created attachment 318290 [details]
Patch

[Geoffrey Garen]

Comment on attachment 318290 [details]
Patch

r=me

[youenn fablet]

Comment on attachment 318290 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=318290&action=review

> Source/WebCore/loader/cache/CachedResourceRequest.cpp:44
> +    , m_originalRequestHeaders(m_resourceRequest.httpHeaderFields())

This field seems unnecessary in CachedResourceRequest, as well as the header map copy for all requests.
Why not using an optional<header-map> in CachedResource and do the copy only for keep alive and/or beacon requests at CachedResourceLoader level?

[WebKit Commit Bot]

Comment on attachment 318290 [details]
Patch

Clearing flags on attachment: 318290

Committed r220817: <http://trac.webkit.org/changeset/220817>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

(In reply to youenn fablet from comment #4)
> Comment on attachment 318290 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=318290&action=review
> 
> > Source/WebCore/loader/cache/CachedResourceRequest.cpp:44
> > +    , m_originalRequestHeaders(m_resourceRequest.httpHeaderFields())
> 
> This field seems unnecessary in CachedResourceRequest, as well as the header
> map copy for all requests.
> Why not using an optional<header-map> in CachedResource and do the copy only
> for keep alive and/or beacon requests at CachedResourceLoader level?

I hesitated to do this, I was trying to avoid special casing keepalive/beacon too much. I can follow-up to see how it looks.

[youenn fablet]

We should try to remove the header map copy.
DTL is already doing this copy and other resources will not do preflights.

Ideally, this copy should be only visible/manipulated by CachedResourceLoader/CachedResource.
It might be easier to have CachedResourceRequest convey this information between the two but other users of CachedResourceRequest should not be tempted to start using it.

[Chris Dumez]

(In reply to Chris Dumez from comment #7)
> (In reply to youenn fablet from comment #4)
> > Comment on attachment 318290 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=318290&action=review
> > 
> > > Source/WebCore/loader/cache/CachedResourceRequest.cpp:44
> > > +    , m_originalRequestHeaders(m_resourceRequest.httpHeaderFields())
> > 
> > This field seems unnecessary in CachedResourceRequest, as well as the header
> > map copy for all requests.
> > Why not using an optional<header-map> in CachedResource and do the copy only
> > for keep alive and/or beacon requests at CachedResourceLoader level?
> 
> I hesitated to do this, I was trying to avoid special casing
> keepalive/beacon too much. I can follow-up to see how it looks.

https://bugs.webkit.org/show_bug.cgi?id=175679