I'm concerned about races within a single renderer, for example a page executes the following code:

document.cookie = 'foo=bar';
fetch('subresource.json');

I think it's natural for page authors to expect to see the foo=bar cookie on the request for subresource.json. However, Mojo does not in general provide this guarantee: https://www.chromium.org/developers/design-documents/mojo/mojo-migration-guide#TOC-Dealing-with-message-ordering

+kinuko who is more knowledgeable about the interaction between message ordering and loading.

+rdsmith@, who's been working on the new mojo interface for Cookies right now. 
 Also +mmenke@ for his expertise.

The race described in #1 could happen with sync IPC if cookie IPC and URLLoaderFactor IPC don't use the same pipe (pointed by mmenke@ on some other issue), and that's one of the reasons we're holding off making URLLoaderFactory un-associated.

In general, I'd be thrilled if we could get rid of the sync IPC for `document.cookies`. That said, I agree with ricea@ about the potential races. If we could ensure that the `document.cookie` Mojo pipe would block subsequent loading requests, that would be excellent.

> However, it looks like Chrome limits a cookie to 4096 bytes in size,

Right. It'll end up being a little less than that, because the byte limit includes the whole cookie string, so also needs to fit all the attribute flags, etc.

>with a max of 180 per domain (not sure if this means origin or the FQDN?)

Neither! It ends up being ~180 per eTLD+1.

Site isolation has talked with several of the network service people, and the conclusion was that cookies are going to have to be associated somehow.

(In case it wasn't clear) note that my comment in #2 seems to imply that it'll be okay if we use the same pipe for cookie and URLLoaderFactory, at least for same-renderer races.

(Looks like I wrote the same thing as dcheng@ wrote :))

Another concern here is multi-hop races.  e.g., if I set a cookie and then
trigger a navigation, the cookie will go over a renderer->network service
pipe, while the navigation will go renderer->browser->network service, and
we have no guarantee of order.

Do we have uma for how long this ipc takes?

@mmenke: Note that SetCookie is already an async IPC, so we do in fact need to guarantee order. My understanding is both of them will be going over the same message pipe from renderer -> network (the browser will broker the message pipes, but not be involved from that point on)

@ojan: We don't today, but FB's tracing data (not sure if they're using chrome://tracing or something else) apparently highlighted this as a pain point. Probably worth adding though, so we can decide how bad the problem is.

@dcheng:  So at the moment (At least unless PlzNavigate broke it), order is guaranteed because they both use IPC and go over the browser's IO thread (Which is also, conveniently, where the cookie store lives).  But once either one of them starts using Mojo, that breaks.

Cookies are already on Mojo, not IPC (and associated with the legacy IPC channel). But I agree that we need to make sure loading is associated with cookie storage as we mojo-fy things; otherwise, it definitely will break. This is true whether or not we implement the async cookie getter.

(I've updated the bug title to indicate that this only changes the behavior of the getter)

+1 on adding metrics to understand the impact and prioritize this accordingly.

filed: https://bugs.chromium.org/p/chromium/issues/detail?id=753247 Is this a lot of work? Who can triage this further?

We've recently introduced RuntimeCallStats to Blink to get a better idea of where time is spent in Blink during JS execution. Some of the preliminary results for some real world websites (based on some local experiments I did) are here: https://docs.google.com/a/google.com/document/d/1WriPEIVme4qYr4qPmvhxNxsvQhRe21sK5X2fSRLFPyQ/edit?usp=sharing (Sorry, Google internal).

I think document.cookie (the getter in particular) accounts for a sizable portion of time spent for a large number of websites I experimented with. 'Blink_Document_cookie_Getter' is the counter tracking the time spent there (this is time spent by the entire blink callback and not just the IPC, although I think the IPC call is probably the largest contributor).

This graph also tracks the execution time spent in document.cookie getter in the v8.runtime_stats.top25 metric:  
https://chromeperf.appspot.com/report?sid=2267faec27f516c3b32aa681369217a45d9f6e9790cbdf0870025fa5b2da0534&start_rev=486218&end_rev=490494 

Just to elaborate on what I imagined for shared memory: I didn't imagine a cross-process mutex, but something like that just used shm to make reads (but not writes) faster, writable only by the browser process. e.g. one of the following:

1. Global generation ID: all renderers share a shm page with the browser that has a uint64_t. Every time cookies are updated, the browser process atomically increments the generation ID. If a renderer reads the generation ID and it is the same as the generation ID for the last time it read cookies for that origin, it reuses the previously read values. Otherwise, it does a sync IPC and updates its cache.

(Pros: simple. Cons: leaks info that some cookie has been changed to all renderers, may be too coarse.)

2. Per-renderer generation ID(s): similar, but share a shm region per renderer. Either have a single generation ID per renderer (bumped when an origin the renderer is authorized to read cookies for is changed, but not otherwise), or a per-origin generation ID (managing this is more complicated, however).

3. Per-renderer cookie jar: use a seqlock-like approach (caution: this is very tricky to do correctly in C++), and also have the cookie data in the shm. In very rough terms: renderer atomically reads generation number, reads data with atomic operations, confirms generation ID is unchanged. Retries if it has changed. No context switch is needed to read cookies, but managing this memory is quite complicated.

Any of these should be possible to do in a way that preserves the browser's single ordering of reads and writes, but if we can be looser/more asynchronous without breaking anything, there are probably simpler solutions available.

With a generation ID, certain races are still possible.

// This is async.
document.cookie = 'foo';
// Race with another renderer to set document.cookie
console.log(document.cookie);
// Logged value may be foo or an entirely different value, depending on who won the race.

It's true that we'll have this race in the browser process no matter what, but the renderer's view of the world should be somewhat consistent, IMO.

Ah, I didn't realize that we had the setter be async even though the getter is sync. (We could change that, assuming the setter is much less common, but I hadn't realized that.)

The race you describe is possible today, no?

document.cookie = 'foo';
// renderer sends IPC and then is descheduled
// other renderer updates cookie
// renderer is scheduled again
console.log(document.cookie);  // not foo

How can a renderer's view be made consistent in the presence of this race?

Ah good point: the race is possible today as well =)

If we cached locally and updated asynchronously via IPC, then this race would no longer be visible (modulo something interesting where we're pumping and handling IPC messages in a nested message loop).

cc-ing myself, as I'm implementing the Async Cookie API -- https://crbug.com/729800

Via that bug: apparently Facebook has also observed document.cookie jank:
https://bugs.chromium.org/p/chromium/issues/detail?id=729800#c4