Send cookieAcceptPolicy to newly created processes

• Component: New Bugs
• Status: RESOLVED FIXED

[Alex Christensen]

Send cookieAcceptPolicy to newly created processes

[Alex Christensen]

Created attachment 282615 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 282615 [details]
Patch

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

> Source/WebKit2/ChangeLog:3
> +        Send cookieAcceptPolicy to newly created processes

I do not think that this is the correct fix.

Have you verified that all processes are actually using the same cookie storage on disk? The observed issue would be easiest to explain if WebKit failed to share the storage (see rdar://problem/20572830 for some details). Another potential explanation is that the fix for this very issue in rdar://problem/20904218 didn't quite work.

> Source/WebKit2/UIProcess/WebCookieManagerProxy.cpp:-163
> -    // - When not testing, Cocoa has the policy persisted, and thus new processes use it (even for ephemeral sessions).

What this comment says.

> Source/WebKit2/UIProcess/mac/WebCookieManagerProxyMac.mm:34
> +    // This will probably eventually be stored on the disk if this process stays alive long enough, but probably not immediately.
> +    // Do not rely on this immediately changing anything other than a value in memory in the current process.

Please see rdar://problem/20904218 for details on the expected behavior.

[Alex Christensen]

I am aware of these problems and their solutions, and I still think this is the correct solution to this problem.  The case I have observed is when the UIProcess calls setHTTPCookieAcceptPolicy and then immediately creates its first WebProcess and NetworkProcess before CFNetwork has persisted the change to disk.  The NetworkProcess and WebProcess now have the old policy, and they then persist the old policy to disk.  The solution is simply not to rely on CFNetwork having persisted anything to disk, unless there is a guarantee that it will happen immediately and synchronously, which there is not.

[Alex Christensen]

Comment on attachment 282615 [details]
Patch

Noting that Alexey initially gave this patch an r- and re-requesting review.

[Alexey Proskuryakov]

I still don't like the idea of multiple processes racing to write the policy to disk.

[Alex Christensen]

Created attachment 282812 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 282812 [details]
Patch

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

> Source/WebKit2/ChangeLog:12
> +        * UIProcess/mac/WebCookieManagerProxyMac.mm:

One day we should rename the ones that are also used on iOS.

[WebKit Commit Bot]

Comment on attachment 282812 [details]
Patch

Clearing flags on attachment: 282812

Committed r202834: <http://trac.webkit.org/changeset/202834>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.