Bug 192970

Summary: [curl] Move cookiePersistentStorageFile from NetworkProcessCreationParameters to NetworkSessionCreationParameters
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: PlatformAssignee: Fujii Hironori <Hironori.Fujii>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, Basuke.Suzuki, commit-queue, don.olmstead, ross.kirsling, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
WIP Patch none

Description Fujii Hironori 2018-12-20 19:14:13 PST
[curl] Move cookiePersistentStorageFile from NetworkProcessCreationParameters to NetworkSessionCreationParameters

[webkit-dev] Reducing globals
https://lists.webkit.org/pipermail/webkit-dev/2018-December/030267.html
Comment 1 Fujii Hironori 2018-12-20 20:00:45 PST
Created attachment 357921 [details]
Patch
Comment 2 Alex Christensen 2018-12-21 09:44:38 PST
Comment on attachment 357921 [details]
Patch

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

r=me.  Thanks

> Source/WebKit/NetworkProcess/curl/NetworkSessionCurl.cpp:43
> +        NetworkProcess::singleton().supplement<WebCookieManager>()->setCookiePersistentStorage(parameters.cookiePersistentStorageFile);

This is a great step in the right direction.  Thanks!  The next step would be to stop using the NetworkProcess::singleton() for this.
Comment 3 Alex Christensen 2018-12-21 10:17:00 PST
Comment on attachment 357921 [details]
Patch

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

>> Source/WebKit/NetworkProcess/curl/NetworkSessionCurl.cpp:43
>> +        NetworkProcess::singleton().supplement<WebCookieManager>()->setCookiePersistentStorage(parameters.cookiePersistentStorageFile);
> 
> This is a great step in the right direction.  Thanks!  The next step would be to stop using the NetworkProcess::singleton() for this.

Instead, this should probably be a member of the NetworkSessionCurl or even better, the NetworkStorageSession.
Comment 4 Fujii Hironori 2018-12-24 19:32:27 PST
Comment on attachment 357921 [details]
Patch

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

Thank you for the review.

>>> Source/WebKit/NetworkProcess/curl/NetworkSessionCurl.cpp:43
>>> +        NetworkProcess::singleton().supplement<WebCookieManager>()->setCookiePersistentStorage(parameters.cookiePersistentStorageFile);
>> 
>> This is a great step in the right direction.  Thanks!  The next step would be to stop using the NetworkProcess::singleton() for this.
> 
> Instead, this should probably be a member of the NetworkSessionCurl or even better, the NetworkStorageSession.

I get the idea. I will revise this patch.
Comment 5 Fujii Hironori 2018-12-24 19:49:49 PST
NetworkProcess::setNetworkProxySettings also should be removed?
https://trac.webkit.org/browser/webkit/trunk/Source/WebKit/NetworkProcess/NetworkProcess.h?rev=239474#L297
Comment 6 Fujii Hironori 2018-12-24 19:54:15 PST
Created attachment 358051 [details]
WIP Patch
Comment 7 Alex Christensen 2019-01-03 09:45:49 PST
Comment on attachment 358051 [details]
WIP Patch

Looks good to me.
Comment 8 WebKit Commit Bot 2019-01-04 11:07:23 PST
Comment on attachment 358051 [details]
WIP Patch

Clearing flags on attachment: 358051

Committed r239624: <https://trac.webkit.org/changeset/239624>
Comment 9 WebKit Commit Bot 2019-01-04 11:07:25 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2019-01-04 11:08:49 PST
<rdar://problem/47052236>