Bug 169797 - Make HTTPCookieStorage operate in the UIProcess in absence of a WebProcessPool
Summary: Make HTTPCookieStorage operate in the UIProcess in absence of a WebProcessPool
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-03-16 16:29 PDT by Brady Eidson
Modified: 2017-03-17 16:23 PDT (History)
8 users (show)

See Also:


Attachments
Patch (17.23 KB, patch)
2017-03-17 13:48 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (17.23 KB, patch)
2017-03-17 15:42 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2017-03-16 16:29:01 PDT
Make HTTPCookieStorage operate in the UIProcess in absence of a WebProcessPool

Currently it creates transient process pools, just to create networking processes, just for cookie operations.

That's nuts - It should operate in the UIProcess and then just flush cookies if an associated WebProcessPool comes into existence.
Comment 1 Radar WebKit Bug Importer 2017-03-16 16:29:29 PDT
<rdar://problem/31102330>
Comment 2 Brady Eidson 2017-03-17 13:48:13 PDT
Created attachment 304819 [details]
Patch
Comment 3 Alex Christensen 2017-03-17 14:04:03 PDT
Comment on attachment 304819 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        No new tests (No behavior change).

I'm not entirely convinced.

> Source/WebKit2/UIProcess/API/APIHTTPCookieStore.cpp:63
> +        callOnMainThread([completionHandler = WTFMove(completionHandler), allCookies = WTFMove(allCookies)]() {

Cookies contain a lot of Strings.  Should we do an isolated copy here?
Comment 4 Brady Eidson 2017-03-17 14:09:26 PDT
(In reply to comment #3)
> Comment on attachment 304819 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=304819&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        No new tests (No behavior change).
> 
> I'm not entirely convinced.

I assure you - "Setting cookies without a process pool" didn't work before, and still doesn't work after this patch. :)

It just gets us closer. We can chat in person about what's left.

> 
> > Source/WebKit2/UIProcess/API/APIHTTPCookieStore.cpp:63
> > +        callOnMainThread([completionHandler = WTFMove(completionHandler), allCookies = WTFMove(allCookies)]() {
> 
> Cookies contain a lot of Strings.  Should we do an isolated copy here?

I think that'd be more expensive, right?

(I'm unsure why you're making the request)
Comment 5 Brady Eidson 2017-03-17 15:42:18 PDT
Created attachment 304829 [details]
Patch
Comment 6 WebKit Commit Bot 2017-03-17 16:23:42 PDT
Comment on attachment 304829 [details]
Patch

Clearing flags on attachment: 304829

Committed r214127: <http://trac.webkit.org/changeset/214127>
Comment 7 WebKit Commit Bot 2017-03-17 16:23:47 PDT
All reviewed patches have been landed.  Closing bug.