Exempt Save-Data header from CORS preflight request check

Exempt Save-Data header from CORS preflight request check

Save-Data header is added by Chrome when Data saver feature is enabled.
This header should be treated as standard header and should be exempted
from CORS checks to trigger preflight request.

BUG=584889
Committed: https://crrev.com/a7d87a39dd8cddc64ead9ffa1f60f79ec6776bd9
Cr-Commit-Position: refs/heads/master@{#376410}

[Raj, 2016-02-16 08:36:39]

Description was changed from

==========
Exempt Save-Data header from CORS preflight request check

Save-Data header is added by Chrome when Data saver feature is enabled.
This header should be treated as standard header and should be exempted
from CORS checks to trigger preflight request.

BUG=584889
==========

to

==========
Exempt Save-Data header from CORS preflight request check

Save-Data header is added by Chrome when Data saver feature is enabled.
This header should be treated as standard header and should be exempted
from CORS checks to trigger preflight request.

BUG=584889
==========

[Raj, 2016-02-16 08:36:39]

rajendrant@chromium.org changed reviewers:
+ japhet@chromium.org

[Raj, 2016-02-16 08:37:34]

rajendrant@chromium.org changed reviewers:
+ yhirano@chromium.org

[Raj, 2016-02-16 08:39:24]

ptal, thanks

yhirano: FetchHeaderList.cpp
japhet: DocumentThreadableLoader.cpp

[yhirano, 2016-02-16 19:42:36]

Can you add any layout tests?

I think DocumentThreadableLoader is not the right place to fix as there are
other paths using CORS (CORS enabled images, for example).
How about adding "save-data" to the simple headers definition?

[yhirano, 2016-02-16 19:43:11]

yhirano@chromium.org changed reviewers:
+ mkwst@chromium.org

[yhirano, 2016-02-16 19:43:11]

+mkwst@

[Raj, 2016-02-16 21:03:05]

Patchset #2 (id:20001) has been deleted

[Raj, 2016-02-16 21:08:08]

On 2016/02/16 19:43:11, yhirano wrote:
> +mkwst@

Thanks yhirano for looking into it immediately.
ptal asap, since we want to merge this to M-49.

I have added 'save-data' to isSimpleHeader().
I will work on adding layout tests.

[Raj, 2016-02-18 05:59:48]

ptal, Thanks

[yhirano, 2016-02-18 17:47:16]

lgtm

[Mike West, 2016-02-19 06:42:57]

This LGTM to land as a temporary fix, but please add a TODO to revisit this
based on the resolution of
https://github.com/whatwg/fetch/issues/52#issuecomment-185575815.

It's not clear to me why Blink knows anything about this header, however; it
seems like something that should be added higher up the stack when we're
actually making network requests.

The argument that the Service Worker should know the state of the request is a
little strange; couldn't we expose a JavaScript API to give the worker that
information? I'm sure there's a good reason for the requirement that I'm
missing...

[Raj, 2016-02-19 06:46:27]

The CQ bit was checked by rajendrant@chromium.org

[commit-bot: I haz the power, 2016-02-19 06:46:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1698973002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1698973002/60001

[commit-bot: I haz the power, 2016-02-19 06:53:01]

Description was changed from

==========
Exempt Save-Data header from CORS preflight request check

Save-Data header is added by Chrome when Data saver feature is enabled.
This header should be treated as standard header and should be exempted
from CORS checks to trigger preflight request.

BUG=584889
==========

to

==========
Exempt Save-Data header from CORS preflight request check

Save-Data header is added by Chrome when Data saver feature is enabled.
This header should be treated as standard header and should be exempted
from CORS checks to trigger preflight request.

BUG=584889
==========

[commit-bot: I haz the power, 2016-02-19 06:53:02]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-02-19 06:53:54]

Description was changed from

==========
Exempt Save-Data header from CORS preflight request check

Save-Data header is added by Chrome when Data saver feature is enabled.
This header should be treated as standard header and should be exempted
from CORS checks to trigger preflight request.

BUG=584889
==========

to

==========
Exempt Save-Data header from CORS preflight request check

Save-Data header is added by Chrome when Data saver feature is enabled.
This header should be treated as standard header and should be exempted
from CORS checks to trigger preflight request.

BUG=584889

Committed: https://crrev.com/a7d87a39dd8cddc64ead9ffa1f60f79ec6776bd9
Cr-Commit-Position: refs/heads/master@{#376410}
==========

[commit-bot: I haz the power, 2016-02-19 06:53:56]

Patchset 3 (id:??) landed as
https://crrev.com/a7d87a39dd8cddc64ead9ffa1f60f79ec6776bd9
Cr-Commit-Position: refs/heads/master@{#376410}

[Raj, 2016-02-20 01:32:15]

On 2016/02/19 06:42:57, Mike West wrote:
> This LGTM to land as a temporary fix, but please add a TODO to revisit this
> based on the resolution of
> https://github.com/whatwg/fetch/issues/52#issuecomment-185575815.
> 
> It's not clear to me why Blink knows anything about this header, however; it
> seems like something that should be added higher up the stack when we're
> actually making network requests.
> 
> The argument that the Service Worker should know the state of the request is a
> little strange; couldn't we expose a JavaScript API to give the worker that
> information? I'm sure there's a good reason for the requirement that I'm
> missing...

Thanks Mike,
Filed http://crbug.com/588371 for the TODO, after there is some consensus.