Bug 160399

Summary: DocumentThreadableLoader should pass the fetch mode to underlying loader code
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebCore Misc.Assignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, cdumez, commit-queue, darin, japhet, mkwst, rniwa
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=172578
Bug Depends on:    
Bug Blocks: 151937    
Attachments:
Description Flags
Patch
none
Rebasing
none
Adding more tests
none
Archive of layout-test-results from ews102 for mac-yosemite
none
Archive of layout-test-results from ews105 for mac-yosemite-wk2
none
Archive of layout-test-results from ews121 for ios-simulator-wk2
none
Patch for landing none

Description youenn fablet 2016-08-01 02:26:41 PDT
Currently, it sets the fetch mode to NoCors and does the fetch mode additional checks (CORS typically) on its own.
Comment 1 youenn fablet 2016-08-01 02:44:32 PDT
Created attachment 284997 [details]
Patch
Comment 2 youenn fablet 2016-08-01 03:35:36 PDT
Created attachment 285001 [details]
Rebasing
Comment 3 Alex Christensen 2016-08-02 10:55:49 PDT
Comment on attachment 285001 [details]
Rebasing

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

> LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects-async-expected.txt:18
> +FAIL: PASS: Cross-domain access allowed.

Is this a correct expectation?
Comment 4 youenn fablet 2016-08-02 11:49:30 PDT
Comment on attachment 285001 [details]
Rebasing

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

>> LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects-async-expected.txt:18
>> +FAIL: PASS: Cross-domain access allowed.
> 
> Is this a correct expectation?

The URL contains credentials so loading should stop as per the spec. But the underlying loading code is removing the credentials so loading continues but without any credential.

> LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects-async.html:57
> +// Underlying HTTP stack currently removes credentials from redirection URL, hence loading is successful.

Here is the comment related to the odd expectation
Comment 5 Darin Adler 2016-08-02 12:18:16 PDT
We should change the test to expect correct behavior.
Comment 6 youenn fablet 2016-08-03 09:22:36 PDT
Created attachment 285239 [details]
Adding more tests
Comment 7 Alex Christensen 2016-08-03 09:29:37 PDT
Comment on attachment 285239 [details]
Adding more tests

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

> Source/WebCore/loader/DocumentThreadableLoader.cpp:233
> +        request = ResourceRequest();

{ }

> LayoutTests/imported/w3c/ChangeLog:31
> +2016-08-03  Youenn Fablet  <youenn@apple.com>

two changelog entries :(
Comment 8 Build Bot 2016-08-03 10:10:22 PDT
Comment on attachment 285239 [details]
Adding more tests

Attachment 285239 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1805299

New failing tests:
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-location-worker.html
imported/w3c/web-platform-tests/fetch/api/cors/cors-redirect-credentials-worker.html
Comment 9 Build Bot 2016-08-03 10:10:26 PDT
Created attachment 285242 [details]
Archive of layout-test-results from ews102 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 10 Build Bot 2016-08-03 10:13:18 PDT
Comment on attachment 285239 [details]
Adding more tests

Attachment 285239 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1805304

New failing tests:
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-location-worker.html
imported/w3c/web-platform-tests/fetch/api/cors/cors-redirect-credentials-worker.html
Comment 11 Build Bot 2016-08-03 10:13:22 PDT
Created attachment 285244 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 12 Build Bot 2016-08-03 10:19:29 PDT
Comment on attachment 285239 [details]
Adding more tests

Attachment 285239 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/1805305

New failing tests:
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-location-worker.html
imported/w3c/web-platform-tests/fetch/api/cors/cors-redirect-credentials-worker.html
Comment 13 Build Bot 2016-08-03 10:19:33 PDT
Created attachment 285245 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.5
Comment 14 youenn fablet 2016-08-03 23:42:56 PDT
(In reply to comment #12)
> Comment on attachment 285239 [details]
> Adding more tests
> 
> Attachment 285239 [details] did not pass ios-sim-ews (ios-simulator-wk2):
> Output: http://webkit-queues.webkit.org/results/1805305
> 
> New failing tests:
> imported/w3c/web-platform-tests/fetch/api/redirect/redirect-location-worker.
> html
> imported/w3c/web-platform-tests/fetch/api/cors/cors-redirect-credentials-
> worker.html

These tests need to be rebased with the changes to the redirect.py script that is now correctly generating redirection URLs.
Comment 15 youenn fablet 2016-08-04 00:28:08 PDT
Created attachment 285306 [details]
Patch for landing
Comment 16 WebKit Commit Bot 2016-08-04 00:57:58 PDT
Comment on attachment 285306 [details]
Patch for landing

Clearing flags on attachment: 285306

Committed r204117: <http://trac.webkit.org/changeset/204117>
Comment 17 WebKit Commit Bot 2016-08-04 00:58:04 PDT
All reviewed patches have been landed.  Closing bug.