Bug 184741 – Use NetworkLoadChecker for XHR/fetch loads

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[youenn fablet]

Use NetworkLoadChecker for XHR/fetch loads.
This will allow us to handle CORS/CSP/ContentBlockers all in NetworkProcess.

[youenn fablet]

Created attachment 338230 [details]
Patch

[EWS Watchlist]

Attachment 338230 [details] did not pass style-queue:


ERROR: Source/WebKit/NetworkProcess/NetworkResourceLoader.h:152:  The parameter name "request" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 76 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 338230 [details]
Patch

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

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 338244 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6

[youenn fablet]

Created attachment 338900 [details]
Patch

[youenn fablet]

Created attachment 338908 [details]
Patch

[youenn fablet]

Created attachment 338911 [details]
Patch

[EWS Watchlist]

Comment on attachment 338911 [details]
Patch

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

New failing tests:
http/tests/security/contentSecurityPolicy/connect-src-xmlhttprequest-redirect-to-blocked.html
imported/w3c/web-platform-tests/cors/late-upload-events.htm

[EWS Watchlist]

Created attachment 338917 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 338911 [details]
Patch

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

New failing tests:
http/tests/quicklook/same-origin-xmlhttprequest-allowed.html

[EWS Watchlist]

Created attachment 338921 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4

[youenn fablet]

Created attachment 338925 [details]
Patch

[youenn fablet]

Created attachment 338931 [details]
Patch

[youenn fablet]

Created attachment 338938 [details]
Patch

[EWS Watchlist]

Comment on attachment 338938 [details]
Patch

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

New failing tests:
http/tests/quicklook/same-origin-xmlhttprequest-allowed.html

[EWS Watchlist]

Created attachment 338949 [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.13.4

[youenn fablet]

Created attachment 338961 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 338961 [details]
Patch

Clearing flags on attachment: 338961

Committed r231107: <https://trac.webkit.org/changeset/231107>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/39797746>

[youenn fablet]

*** Bug 184696 has been marked as a duplicate of this bug. ***

[Daniel Bates]

(In reply to Build Bot from comment #15)
> Comment on attachment 338938 [details]
> Patch
> 
> Attachment 338938 [details] did not pass ios-sim-ews (ios-simulator-wk2):
> Output: http://webkit-queues.webkit.org/results/7474737
> 
> New failing tests:
> http/tests/quicklook/same-origin-xmlhttprequest-allowed.html

This failure represents a change in behavior...

(In reply to youenn fablet from comment #17)
> Created attachment 338961 [details]
> Patch

...that was unfortunately incorporated into this patch without explanation and...

(In reply to WebKit Commit Bot from comment #18)
> Comment on attachment 338961 [details]
> Patch
> 
> Clearing flags on attachment: 338961
> 
> Committed r231107: <https://trac.webkit.org/changeset/231107>

landed in this commit :(

Filed bug #185807 to address this test regression.

[Daniel Bates]

(In reply to WebKit Commit Bot from comment #18)
> Comment on attachment 338961 [details]
> Patch
> 
> Clearing flags on attachment: 338961
> 
> Committed r231107: <https://trac.webkit.org/changeset/231107>

This change makes MoviStar+ launch to a blank black screen. See bug #188139 for more details.

[Daniel Bates]

(In reply to WebKit Commit Bot from comment #18)
> Comment on attachment 338961 [details]
> Patch
> 
> Clearing flags on attachment: 338961
> 
> Committed r231107: <https://trac.webkit.org/changeset/231107>

It is disingenuous that this change landed under the guise of using "NetworkLoadChecker for XHR/fetch loads" and was "covered by existing tests". Both statements are half-truths when viewed in the best light. This change does at least three things:

1. It refactors code in WebCore so as to support the kind of "hand-off" needed for the NetworkLoadChecker in the NetworkProcess to take responsibility for XHR/Fetch loads. This change is expected given the title of this bug, the description given in comment #0, and what little is explicitly written or can be inferred from the ChangeLog.

2. (Unexplained behavior change) Change in behavior with respect to Quick Look XHR caught by test failure http/tests/quicklook/same-origin-xmlhttprequest-allowed.html. See bug #185807 for more details.

3. (Unexplained behavior change) Preflight XHRs are blocked for non-HTTP(s) schemes. See bug #188139 for more details.