Preconnect to server as early as possible in WebPage::LoadRequest. This avoids delaying the connection to the server until after the policy check and is a ~1.3% progression on PLT5 on both macOS and iOS.
Created attachment 385108 [details] Patch
This change looks like a good technical change. It looks like it sacrifices user privacy at the cost of improving perceived load speed: apps and Safari can no longer prevent the preconnect from hitting the server with credentials. Code with is used by all page loads, right? Even programmatic? My strong opinion, weakly held, is that this change should be guarded by a user configurable setting (initially off by default) because there are significant privacy implications.
(In reply to Daniel Bates from comment #2) > This change looks like a good technical change. It looks like it sacrifices > user privacy at the cost of improving perceived load speed: apps and Safari > can no longer prevent the preconnect from hitting the server with > credentials. Code with is used by all page loads, right? Even programmatic? > My strong opinion, weakly held, is that this change should be guarded by a > user configurable setting (initially off by default) because there are > significant privacy implications. It is indeed important not to sacrifice privacy. I don’t think I am though but I may be missing something. I am not sure what you mean by programmatic loads, could mean JS or native. Loads triggered by JS or even user clicks do not use this code path. Only loads requested by the client app (native) using [WKWebView loadRequest:] are affected. Since those loads are requested by the client app, and the client app is in charge of the decidePolicyForNavigationAction, I did not feel connecting to the server early sacrificed privacy. Does this address your concerns?
Comment on attachment 385108 [details] Patch Looks like there are API test failures that need investigating. Clearing review flag while I do so.
Created attachment 385113 [details] Patch
Comment on attachment 385113 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=385113&action=review > Source/WebKit/WebProcess/WebPage/WebPage.cpp:1531 > + WebProcess::singleton().webLoaderStrategy().preconnectTo(ResourceRequest { loadParameters.request }, *this, *m_mainFrame, StoredCredentialsPolicy::Use, [](const ResourceError&) { }); WTF::Function is a nullable type. Another option would be to make preconnectTo handle a null callback function and remove the need to construct a no-op lambda.
Created attachment 385125 [details] Patch
(In reply to Antti Koivisto from comment #6) > Comment on attachment 385113 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=385113&action=review > > > Source/WebKit/WebProcess/WebPage/WebPage.cpp:1531 > > + WebProcess::singleton().webLoaderStrategy().preconnectTo(ResourceRequest { loadParameters.request }, *this, *m_mainFrame, StoredCredentialsPolicy::Use, [](const ResourceError&) { }); > > WTF::Function is a nullable type. Another option would be to make > preconnectTo handle a null callback function and remove the need to > construct a no-op lambda. Will look into this in a follow-up.
Comment on attachment 385125 [details] Patch Clearing flags on attachment: 385125 Committed r253277: <https://trac.webkit.org/changeset/253277>
All reviewed patches have been landed. Closing bug.
<rdar://problem/57735901>
Breaks some ApplePay API tests on iOS: https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.ApplePay.CanMakePaymentsWithActiveCardBlocksUserAgentScripts
(In reply to Jonathan Bedard from comment #12) > Breaks some ApplePay API tests on iOS: > > https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.ApplePay. > CanMakePaymentsWithActiveCardBlocksUserAgentScripts Looking now.
(In reply to Chris Dumez from comment #13) > (In reply to Jonathan Bedard from comment #12) > > Breaks some ApplePay API tests on iOS: > > > > https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.ApplePay. > > CanMakePaymentsWithActiveCardBlocksUserAgentScripts > > Looking now. Follow-up fix landed in <https://trac.webkit.org/changeset/253291>.