WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
185849
NetworkLoadChecker should check cached redirections
https://bugs.webkit.org/show_bug.cgi?id=185849
Summary
NetworkLoadChecker should check cached redirections
youenn fablet
Reported
2018-05-21 16:12:48 PDT
NetworkLoadChecker should check cached redirections
Attachments
Patch
(16.04 KB, patch)
2018-05-21 16:18 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews106 for mac-sierra-wk2
(3.29 MB, application/zip)
2018-05-21 17:41 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews124 for ios-simulator-wk2
(2.38 MB, application/zip)
2018-05-21 17:58 PDT
,
EWS Watchlist
no flags
Details
Patch
(25.38 KB, patch)
2018-05-21 20:22 PDT
,
youenn fablet
ews-watchlist
: commit-queue-
Details
Formatted Diff
Diff
Archive of layout-test-results from ews106 for mac-sierra-wk2
(2.84 MB, application/zip)
2018-05-21 21:12 PDT
,
EWS Watchlist
no flags
Details
Patch
(32.51 KB, patch)
2018-05-22 10:12 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch
(34.71 KB, patch)
2018-05-22 11:10 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch
(32.77 KB, patch)
2018-05-22 14:15 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch
(32.86 KB, patch)
2018-05-22 17:24 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Remove setting some metric values
(32.65 KB, patch)
2018-05-23 10:58 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
youenn fablet
Comment 1
2018-05-21 16:18:53 PDT
Created
attachment 340918
[details]
Patch
EWS Watchlist
Comment 2
2018-05-21 17:41:42 PDT
Comment on
attachment 340918
[details]
Patch
Attachment 340918
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/7759068
New failing tests: http/tests/cache/disk-cache/redirect-chain-limits.html http/tests/fetch/redirectmode-and-preload.html
EWS Watchlist
Comment 3
2018-05-21 17:41:43 PDT
Created
attachment 340931
[details]
Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
EWS Watchlist
Comment 4
2018-05-21 17:58:17 PDT
Comment on
attachment 340918
[details]
Patch
Attachment 340918
[details]
did not pass ios-sim-ews (ios-simulator-wk2): Output:
http://webkit-queues.webkit.org/results/7759015
New failing tests: http/tests/cache/disk-cache/redirect-chain-limits.html http/tests/fetch/redirectmode-and-preload.html
EWS Watchlist
Comment 5
2018-05-21 17:58:18 PDT
Created
attachment 340934
[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
Comment 6
2018-05-21 20:22:27 PDT
Created
attachment 340949
[details]
Patch
EWS Watchlist
Comment 7
2018-05-21 21:12:32 PDT
Comment on
attachment 340949
[details]
Patch
Attachment 340949
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/7760808
New failing tests: imported/w3c/web-platform-tests/service-workers/service-worker/redirected-response.https.html http/tests/cache/disk-cache/redirect-chain-limits.html
EWS Watchlist
Comment 8
2018-05-21 21:12:33 PDT
Created
attachment 340951
[details]
Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
youenn fablet
Comment 9
2018-05-22 10:12:39 PDT
Created
attachment 340991
[details]
Patch
youenn fablet
Comment 10
2018-05-22 11:10:03 PDT
Created
attachment 340994
[details]
Patch
youenn fablet
Comment 11
2018-05-22 14:15:32 PDT
Created
attachment 341022
[details]
Patch
Chris Dumez
Comment 12
2018-05-22 16:25:40 PDT
Comment on
attachment 341022
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=341022&action=review
> Source/WebCore/loader/SubresourceLoader.cpp:212 > + ResourceError error { errorDomainWebKitInternal, 0, request().url(), makeString("Load cannot follow redirection for ", request().url().string()), ResourceError::Type::AccessControl };
"redirection to" sounds better than "redirection for". Also, the previous text was clear that it was a security error by saying "not allowed". The new message merely says it cannot follow the redirection, without giving the reason.
> Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:108 > + handler(accessControlErrorForValidationHandler(makeString("Load cannot follow redirection for ", redirectResponse.url().string())));
ditto.
youenn fablet
Comment 13
2018-05-22 17:24:23 PDT
OK, will replace with the following: "Not allowed to follow a redirection while loading XXX" (In reply to Chris Dumez from
comment #12
)
> Comment on
attachment 341022
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=341022&action=review
> > > Source/WebCore/loader/SubresourceLoader.cpp:212 > > + ResourceError error { errorDomainWebKitInternal, 0, request().url(), makeString("Load cannot follow redirection for ", request().url().string()), ResourceError::Type::AccessControl }; > > "redirection to" sounds better than "redirection for". Also, the previous > text was clear that it was a security error by saying "not allowed". The new > message merely says it cannot follow the redirection, without giving the > reason. > > > Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:108 > > + handler(accessControlErrorForValidationHandler(makeString("Load cannot follow redirection for ", redirectResponse.url().string()))); > > ditto.
youenn fablet
Comment 14
2018-05-22 17:24:37 PDT
Created
attachment 341050
[details]
Patch
Chris Dumez
Comment 15
2018-05-23 10:33:51 PDT
Comment on
attachment 341050
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=341050&action=review
> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:679 > + networkLoadMetrics.markComplete();
Why all this? The previous code was just using a default initialized NetworkLoadMetrics.
youenn fablet
Comment 16
2018-05-23 10:54:49 PDT
> > Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:679 > > + networkLoadMetrics.markComplete(); > > Why all this? The previous code was just using a default initialized > NetworkLoadMetrics.
Since we finish the load, we should be marking the metric as complete. We haven't received any body yet, so we can set 0 to body received/decoded. I will remove setting the other fields though since we do not have that information.
youenn fablet
Comment 17
2018-05-23 10:58:49 PDT
Created
attachment 341108
[details]
Remove setting some metric values
WebKit Commit Bot
Comment 18
2018-05-23 12:09:14 PDT
Comment on
attachment 341108
[details]
Remove setting some metric values Clearing flags on attachment: 341108 Committed
r232121
: <
https://trac.webkit.org/changeset/232121
>
WebKit Commit Bot
Comment 19
2018-05-23 12:09:16 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 20
2018-05-23 12:11:02 PDT
<
rdar://problem/40494164
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug