REOPENED 230015
Replace PrivateClickMeasurementNetworkLoader::start with an implementation that doesn't need NetworkSession
https://bugs.webkit.org/show_bug.cgi?id=230015
Summary Replace PrivateClickMeasurementNetworkLoader::start with an implementation th...
Alex Christensen
Reported 2021-09-07 12:40:46 PDT
Replace PrivateClickMeasurementNetworkLoader::start with an implementation that doesn't need NetworkSession
Attachments
Patch (33.82 KB, patch)
2021-09-07 12:44 PDT, Alex Christensen
no flags
Patch (28.57 KB, patch)
2021-09-07 12:53 PDT, Alex Christensen
ews-feeder: commit-queue-
Patch (33.88 KB, patch)
2021-09-07 13:01 PDT, Alex Christensen
wilander: review+
ews-feeder: commit-queue-
Patch (33.89 KB, patch)
2021-09-07 13:17 PDT, Alex Christensen
no flags
Patch (33.94 KB, patch)
2021-09-07 15:10 PDT, Alex Christensen
no flags
Patch (4.37 KB, patch)
2021-09-07 18:35 PDT, Alex Christensen
ews-feeder: commit-queue-
Alex Christensen
Comment 1 2021-09-07 12:44:31 PDT
Alex Christensen
Comment 2 2021-09-07 12:53:54 PDT
Alex Christensen
Comment 3 2021-09-07 13:01:43 PDT
Alex Christensen
Comment 4 2021-09-07 13:17:01 PDT
John Wilander
Comment 5 2021-09-07 13:27:22 PDT
Comment on attachment 437546 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437546&action=review We should check that we're not leaving behind an ephemeral, stateless session that is no longer used. I believe we use it for more things but we should make sure. See inline questions. r=me as long as EWS is happy. > Source/WebKit/ChangeLog:8 > + This is needed to run this code in a process that doesn't have NetworkSession, which is a class tied closely to WKWebsiteDataStore. With this change, do we still have guarantees that PCM will not be enabled in ephemeral sessions? > Source/WebKit/ChangeLog:11 > + 2. Does not allow redirects. Did we have this configuration before? I'm for it until we are proven otherwise but I want to keep track of any functional changes. > Source/WebKit/NetworkProcess/PrivateClickMeasurement/cocoa/PrivateClickMeasurementNetworkLoaderCocoa.mm:88 > + configuration._shouldSkipPreferredClientCertificateLookup = YES; What does this imply?
Alex Christensen
Comment 6 2021-09-07 13:32:10 PDT
(In reply to John Wilander from comment #5) > Comment on attachment 437546 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=437546&action=review > > We should check that we're not leaving behind an ephemeral, stateless > session that is no longer used. I believe we use it for more things but we > should make sure. > > See inline questions. r=me as long as EWS is happy. > > > Source/WebKit/ChangeLog:8 > > + This is needed to run this code in a process that doesn't have NetworkSession, which is a class tied closely to WKWebsiteDataStore. > > With this change, do we still have guarantees that PCM will not be enabled > in ephemeral sessions? Yes, that is currently done in a function named pcmStoreDirectory. > > Source/WebKit/ChangeLog:11 > > + 2. Does not allow redirects. > > Did we have this configuration before? I'm for it until we are proven > otherwise but I want to keep track of any functional changes. Yes, PrivateClickMeasurementNetworkLoader::willSendRedirectedRequest called cancel. > > Source/WebKit/NetworkProcess/PrivateClickMeasurement/cocoa/PrivateClickMeasurementNetworkLoaderCocoa.mm:88 > > + configuration._shouldSkipPreferredClientCertificateLookup = YES; > > What does this imply? This is what we do for all our sessions in WebKit now. It makes it so that CFNetwork doesn't look in the keychain for client certificates if the server requests one.
Alex Christensen
Comment 7 2021-09-07 15:10:36 PDT
Alex Christensen
Comment 8 2021-09-07 15:14:39 PDT
Radar WebKit Bug Importer
Comment 9 2021-09-07 15:15:54 PDT
Alex Christensen
Comment 10 2021-09-07 18:35:43 PDT
Reopening to attach new patch.
Alex Christensen
Comment 11 2021-09-07 18:35:44 PDT
Alex Christensen
Comment 12 2021-09-07 18:42:38 PDT
Removed extra "static" and committed to r282121
Note You need to log in before you can comment on or make changes to this bug.