As per https://fetch.spec.whatwg.org/#cors-preflight-fetch, computation of this value should be done before Origin header is set. It should also remove safe request headers from the list and sort the unsafe headers.
Created attachment 284692 [details] Patch
Comment on attachment 284692 [details] Patch Attachment 284692 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1761974 New failing tests: imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight.html
Created attachment 284696 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.5
Created attachment 284702 [details] Patch
Comment on attachment 284702 [details] Patch Rejecting attachment 284702 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 284702, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in LayoutTests/ChangeLog contains OOPS!. Full output: http://webkit-queues.webkit.org/results/1762526
Comment on attachment 284702 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284702&action=review > Source/WebCore/loader/DocumentThreadableLoader.cpp:121 > + preflightSuccess(ResourceRequest(request)); Is it really impossible to make request a ResourceRequest&&? This copy shouldn't be necessary. > Source/WebCore/platform/network/HTTPParsers.cpp:770 > + return value == "application/x-www-form-urlencoded" || value == "multipart/form-data" || value == "text/plain"; These should probably be case-insensitive comparisons and tested as such. tExT/pLaIn should be safe, right?
Comment on attachment 284702 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284702&action=review >> Source/WebCore/loader/DocumentThreadableLoader.cpp:121 >> + preflightSuccess(ResourceRequest(request)); > > Is it really impossible to make request a ResourceRequest&&? This copy shouldn't be necessary. request is const and preflightSuccess will add the origin in the headers, so we need to make a copy. Or we should make DocumentThreadableLoader constructor take a ResourceRequest&&. >> Source/WebCore/platform/network/HTTPParsers.cpp:770 >> + return value == "application/x-www-form-urlencoded" || value == "multipart/form-data" || value == "text/plain"; > > These should probably be case-insensitive comparisons and tested as such. tExT/pLaIn should be safe, right? I need to fix that obviously, thanks!
Created attachment 284761 [details] Fixing case comparison and beefing up tests
Created attachment 284764 [details] Fixing case comparison and beefing up tests
I added rebasing of some worker tests (currently skipped). As can be seen, referrer is set for workers before preflighting, in WorkerThreadableLoader probably. Like we did for Origin, setting referrer should be done at a lower loader code level.
> Or we should make DocumentThreadableLoader constructor take a > ResourceRequest&&. Yes. Let's do that. That's better than making a copy.
Comment on attachment 284764 [details] Fixing case comparison and beefing up tests r=me other than that.
(In reply to comment #11) > > Or we should make DocumentThreadableLoader constructor take a > > ResourceRequest&&. > Yes. Let's do that. That's better than making a copy. OK, I'll prepare patches to optimize ResourceResponse and ResourceRequest transfer around DocumentThreadableLoader. As for ResourceRequest, note that DocumentThreadableLoader is already creating a request copy when creating its CachedResourceRequest. We could probably optimize this already without changing its clients. That should go back to the previous number of ResourceRequest copies.
Comment on attachment 284764 [details] Fixing case comparison and beefing up tests Clearing flags on attachment: 284764 Committed r203899: <http://trac.webkit.org/changeset/203899>
All reviewed patches have been landed. Closing bug.