Bug 227992 - Add SPI to prevent preconnect
Summary: Add SPI to prevent preconnect
Status: RESOLVED DUPLICATE of bug 228044
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-15 10:48 PDT by Alex Christensen
Modified: 2021-07-16 17:21 PDT (History)
2 users (show)

See Also:


Attachments
Patch (13.33 KB, patch)
2021-07-15 10:49 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (11.07 KB, patch)
2021-07-15 15:50 PDT, Alex Christensen
ggaren: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2021-07-15 10:48:35 PDT
Add SPI to prevent preconnect
Comment 1 Alex Christensen 2021-07-15 10:49:49 PDT
Created attachment 433596 [details]
Patch
Comment 2 Alex Christensen 2021-07-15 10:49:52 PDT
<rdar://problem/72995136>
Comment 3 Geoffrey Garen 2021-07-15 11:13:16 PDT
Given the problem description that "the resource load delegate is not being called for any pre-connections", did you consider changing pre-connect loads to consult the resource load delegate? If so, can you share your thinking for why a separate setting for pre-connect loads is preferable?
Comment 4 Alex Christensen 2021-07-15 15:32:52 PDT
Comment on attachment 433596 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433596&action=review

> Tools/TestWebKitAPI/Tests/WebKitCocoa/LoadInvalidURLRequest.mm:140
> +TEST(WebKit, LoadNSURLRequestWithProperty)

Oops.  This should not have been included here.  Please review as if it weren't here, and it'll be removed before landing.
Comment 5 Alex Christensen 2021-07-15 15:34:03 PDT
Geoff I'm not sure what you're saying.  This has nothing to do with a resource load delegate.  This adds a way to prevent preconnecting.
Comment 6 Alex Christensen 2021-07-15 15:50:42 PDT
Created attachment 433630 [details]
Patch
Comment 7 Alex Christensen 2021-07-15 15:53:49 PDT
Ah, I read the radar and now I know what you're saying.  The initial solution attempt was to do this through WKBundlePageResourceLoadClient, but I don't think we should solve this problem that way because we are trying to decrease use of the injected bundle, this solves the problem just as well, and this has no compatibility issues with existing users of WKBundlePageResourceLoadClient.
Comment 8 Geoffrey Garen 2021-07-16 12:38:08 PDT
Two thoughts:

1. Does WKWebViewConfiguration._loadsFromNetwork prevent preconnect? (Seems like it should.)

2. If WKWebViewConfiguration._loadsFromNetwork prevents preconnect, is there any specific use case for this SPI, or should we just recommend adopting WKWebViewConfiguration._loadsFromNetwork instead?

In my view, WKWebViewConfiguration._loadsFromNetwork is the better SPI, and something we should consider promoting to API.

I'm open to practicality arguments about why we need an SPI specific to preconnect for the time being for some reason. But if WKWebViewConfiguration._loadsFromNetwork gets the job done, I'd prefer going down that route, either right now, or at least in the longer term.

So, I'll say r+ here because it's just SPI and it's not harmful or anything, bug please consider WKWebViewConfiguration._loadsFromNetwork as an alternative before landing.
Comment 9 Alex Christensen 2021-07-16 17:21:26 PDT

*** This bug has been marked as a duplicate of bug 228044 ***