Bug 160399 - DocumentThreadableLoader should pass the fetch mode to underlying loader code
Summary: DocumentThreadableLoader should pass the fetch mode to underlying loader code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on:
Blocks: 151937
  Show dependency treegraph
 
Reported: 2016-08-01 02:26 PDT by youenn fablet
Modified: 2017-05-25 10:37 PDT (History)
7 users (show)

See Also:


Attachments
Patch (53.00 KB, patch)
2016-08-01 02:44 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Rebasing (52.87 KB, patch)
2016-08-01 03:35 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Adding more tests (68.40 KB, patch)
2016-08-03 09:22 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-yosemite (745.96 KB, application/zip)
2016-08-03 10:10 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews105 for mac-yosemite-wk2 (926.21 KB, application/zip)
2016-08-03 10:13 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (639.72 KB, application/zip)
2016-08-03 10:19 PDT, Build Bot
no flags Details
Patch for landing (87.46 KB, patch)
2016-08-04 00:28 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.