Preconnect to server as early as possible in WebPage::LoadRequest

• Component: Page Loading
• Status: RESOLVED FIXED

[Chris Dumez]

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.

[Chris Dumez]

Created attachment 385108 [details]
Patch

[Daniel Bates]

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.

[Chris Dumez]

(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?

[Chris Dumez]

Comment on attachment 385108 [details]
Patch

Looks like there are API test failures that need investigating. Clearing review flag while I do so.

[Chris Dumez]

Created attachment 385113 [details]
Patch

[Antti Koivisto]

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.

[Chris Dumez]

Created attachment 385125 [details]
Patch

[Chris Dumez]

(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.

[WebKit Commit Bot]

Comment on attachment 385125 [details]
Patch

Clearing flags on attachment: 385125

Committed r253277: <https://trac.webkit.org/changeset/253277>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/57735901>

[Jonathan Bedard]

Breaks some ApplePay API tests on iOS:

https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.ApplePay.CanMakePaymentsWithActiveCardBlocksUserAgentScripts

[Chris Dumez]

(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.

[Chris Dumez]

(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>.