Remove Save-Data from external/wpt/XMLHttpRequest/access-control-basic-cors-safelisted-request-headers.htm after detailed investigation |
|||
Issue description
,
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!
,
Oct 27 2017
Patch submitted for review: https://chromium-review.googlesource.com/c/chromium/src/+/741361
,
Nov 6 2017
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.
,
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
,
Nov 16
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
,
Nov 19
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?
,
Nov 21
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 |
|||
Comment 1 by r.k...@samsung.com
, Oct 24 2017