Bug 175443

<rdar://problem/33729002>

Created attachment 318298 [details] WIP Patch

Created attachment 318299 [details] WIP Patch

Comment on attachment 318299 [details] WIP Patch Attachment 318299 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4326946 New failing tests: http/tests/security/contentSecurityPolicy/user-style-sheet-font-crasher.php

Created attachment 318309 [details] Archive of layout-test-results from ews104 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6

Comment on attachment 318299 [details] WIP Patch Attachment 318299 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4326970 New failing tests: http/tests/security/contentSecurityPolicy/user-style-sheet-font-crasher.php

Created attachment 318311 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.5

Created attachment 318399 [details] WIP Patch

Created attachment 318442 [details] WIP Patch

Created attachment 318451 [details] Patch

Comment on attachment 318451 [details] Patch Looks good to me. Two questions though: What happens if the network process crashes? Is there a way to call the completion handlers so as to free WebProcess pings? Since we are having a completion handler, would it be worth passing an error if ping load fails? That might be helpful to do some logging or use this in Web Inspector.

(In reply to youenn fablet from comment #11) > Comment on attachment 318451 [details] > Patch > > Looks good to me. > Two questions though: > > What happens if the network process crashes? > Is there a way to call the completion handlers so as to free WebProcess > pings? I'll look into this. > > Since we are having a completion handler, would it be worth passing an error > if ping load fails? > That might be helpful to do some logging or use this in Web Inspector. For now this is not useful. It would be trivial to add it in a follow-up if we need it.

Created attachment 318457 [details] Patch

Comment on attachment 318457 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=318457&action=review > Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:355 > + for (auto& pingLoadCompletionHandler : pingLoadCompletionHandlers.values()) Now dealing with network process crashes better.

Comment on attachment 318457 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=318457&action=review > Source/WebKitLegacy/WebCoreSupport/WebResourceLoadScheduler.h:62 > + void startPingLoad(WebCore::NetworkingContext*, WebCore::ResourceRequest&, const WebCore::HTTPHeaderMap&, Ref<WebCore::SecurityOrigin>&& sourceOrigin, WebCore::ContentSecurityPolicy*, const WebCore::FetchOptions&, WTF::Function<void()>&& completionHandler) override; Should be final probably. Do we really need sourceOrigin parameter name here also? On another topic related to the handling of headers, do we have a test checking referrer value after a cross-origin redirection and with a referrer policy like origin-when-cross-origin? Looking at the code, I am not sure we are doing the right thing here.

> > Since we are having a completion handler, would it be worth passing an error > > if ping load fails? > > That might be helpful to do some logging or use this in Web Inspector. > > For now this is not useful. It would be trivial to add it in a follow-up if > we need it. Adding some web inspector console logging would be good when ping load fails. That might be handy for web developers to debug things like CORS checks, at least until web inspector fully supports ping loads in the network panel. This would also mirror what is done for fetch.

(In reply to youenn fablet from comment #16) > > > Since we are having a completion handler, would it be worth passing an error > > > if ping load fails? > > > That might be helpful to do some logging or use this in Web Inspector. > > > > For now this is not useful. It would be trivial to add it in a follow-up if > > we need it. > > Adding some web inspector console logging would be good when ping load fails. > That might be handy for web developers to debug things like CORS checks, at > least until web inspector fully supports ping loads in the network panel. > This would also mirror what is done for fetch. I started working on error reporting but this is a larger change. I'll upload a separate follow-up patch for it.

Created attachment 318516 [details] Patch

Comment on attachment 318516 [details] Patch Clearing flags on attachment: 318516 Committed r220922: <http://trac.webkit.org/changeset/220922>

All reviewed patches have been landed. Closing bug.