New issue
Advanced search Search tips

Issue 765103 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Remove Save-Data from external/wpt/XMLHttpRequest/access-control-basic-cors-safelisted-request-headers.htm after detailed investigation

Project Member Reported by tyoshino@chromium.org, Sep 14 2017

Issue description

Comment 1 by r.k...@samsung.com, Oct 24 2017

I'm working on this bug

Comment 2 by r.k...@samsung.com, Oct 24 2017

Dear Tyoshino,

I verified "Save-Data" header behaviour in detail in Chrome.
Below are the details:


"Save-Data" header is used to get the compressed content.
ref: http://httpwg.org/http-extensions/client-hints.html#save-data

Chrome uses "Save-Data" header whenever "DataSaver" feature is ON.
During reload, Chrome sets "Save-Data" header based on DataSaver feature is ON/OFF.
ref: https://cs.chromium.org/chromium/src/content/browser/frame_host/navigation_request.cc?type=cs&q=%22Save-Data%22&sq=package:chromium&l=153

Hence, even if "Save-Data" header is set in the test case explicitly, it will not be in added to the request headers unless "DataSaver" feature is ON.
IMHO, we can remove Save-Data from external/wpt/XMLHttpRequest/access-control-basic-cors-safelisted-request-headers.htm as you suggested.

Shall I go ahead with the patch, please let me know your opinion.

Thanks!
Sorry for delay. I should've responded to your suggestion earlier!

As I said in the comment for your patch, the test is a part of web-platform-tests. So, it should test behaviors specified by the standards. If Chromium doesn't match it, we should have an -expected.txt file, instead of modifying the test case.

Thanks for the investigation. But even though Chrome has the behavior that for reload cases it decides whether to put the header based only on the feature, for other cases, it's working to verify that Save-Data doesn't trigger the CORS preflight. So, it's not perfect but useful, and it isn't harmful. So, I think we can just keep the setRequestHeader() call there. What we agreed on in the comment was that we shouldn't have the echo back verification until we figure out what's standardized and what's not regarding what's emitted by a UA to the peer server.

FYI, the background of whitelisting Save-Data is to expose the header to a Service Worker to help it figure out what's the best thing to do while not causing the CORS preflight when the Service Worker forwards the Save-Data attached Request object to the network. So, actually it's more important to verify that it's whitelisted inside SW environment.
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/ccbf88794cd4bcbe330528c42e2ea959d8bb78c8

commit ccbf88794cd4bcbe330528c42e2ea959d8bb78c8
Author: Eric Foo <efoo@chromium.org>
Date: Thu Nov 16 01:13:13 2017

Add "waterfall view will be deprecated" header to waterfall template

Bug:765103
Change-Id: If5b51e012eef026ea2045ee419c438e342ee9256
Reviewed-on: https://chromium-review.googlesource.com/773579
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Eric Foo <efoo@chromium.org>

[delete] https://crrev.com/6beae0b12c70424a117bf463fc6d45a7a632c87a/masters/master.chromium.infra/templates/waterfall.html
[modify] https://crrev.com/ccbf88794cd4bcbe330528c42e2ea959d8bb78c8/masters/master.chromium/templates/waterfall.html

Project Member

Comment 6 by sheriffbot@chromium.org, Nov 16

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Recharge-Cold
Status: Available (was: Untriaged)
Every other browser is failing this test, probably because Save-Data is non-standard so it triggers a preflight.

http://w3c-test.org/xhr/access-control-basic-cors-safelisted-request-headers.htm

Should we test Save-Data in a chromium-specific layout test instead?
Save-Data has been specified as a CORS-safelisted header. https://fetch.spec.whatwg.org/#cors-safelisted-request-header

Sign in to add a comment