New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 591686 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Buried. Ping if important.
Closed: Oct 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocked on:
issue 591720



Sign in to add a comment

Tweak cookie eviction algorithm.

Project Member Reported by mkwst@chromium.org, Mar 3 2016

Issue description

On `google.com`, it's easy to get into a state where you're over the cookie limit for the origin, and cookies begin to be discarded. The login folks have a single non-secure login cookie that they need to retain, and it's getting dropped on the floor due to our work in https://tools.ietf.org/html/draft-ietf-httpbis-cookie-alone.

Their proposal is that we begin treating non-secure, high-priority cookies as being more important than secure, low-priority cookies. This weakens the guarantee that we'd like to make for secure cookies, but since it's an opt-in that the origin specific chooses, it's worth experimenting with.
 
Is this using Priority attribute or inferring priority in another way?

Comment 2 by mkwst@chromium.org, Mar 3 2016

Using the priority attribute. Which I hope to actually bring to the IETF in the near future. :/

Comment 3 by mkwst@chromium.org, Mar 3 2016

So, this is a mess.

I updated the eviction algorithm with our initial pass at a merger between leave-secure-cookies-along and cookie-priorities, and neglected to ensure that that new algorithm was safely locked up behind the "strict secure cookies" experiment. That patch is in M50.

I think the right thing to do is to revert https://codereview.chromium.org/1705753002 on the M50 branch so that we retain the status quo behavior for beta. That means that we're going to have to wait a bit longer to start ratcheting down on cookies, which is unfortunate, but the safe choice this late in the game.

I'll file a separate bug to ask the release managers for permission.
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 11 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e079ac4170556a1a8eb498acef6b02a461be678f

commit e079ac4170556a1a8eb498acef6b02a461be678f
Author: mkwst <mkwst@chromium.org>
Date: Fri Mar 11 09:04:06 2016

Rewrite the cookie eviction algorithm.

The current code is very complicated, creating an array of iterators and
using them to demarcate sorted boundaries within a vector. This patch
should have no web-visible impact on eviction behavior, but changes the
implementation to be substantially less confusing.

BUG= 591686 

Review URL: https://codereview.chromium.org/1767513004

Cr-Commit-Position: refs/heads/master@{#380583}

[modify] https://crrev.com/e079ac4170556a1a8eb498acef6b02a461be678f/net/cookies/cookie_monster.cc
[modify] https://crrev.com/e079ac4170556a1a8eb498acef6b02a461be678f/net/cookies/cookie_monster.h
[modify] https://crrev.com/e079ac4170556a1a8eb498acef6b02a461be678f/net/cookies/cookie_monster_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, May 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c00ac71cdb3caff6710e3c35c0b23493c3cde0d3

commit c00ac71cdb3caff6710e3c35c0b23493c3cde0d3
Author: jww <jww@chromium.org>
Date: Thu May 05 22:21:44 2016

Strict Secure Cookies re-implemented to be priority aware

This implements Strict Secure Cookies to be priority-aware. Modifies the
cookie eviction and garbage collection algorithms to evict low priority
cookies first, then low priority secure cookies, followed by all other
non-secure cookies, and finally all secure cookies.

Our original Strict Secure Cookies implementation ignored priorities
completely, which led to some surprising interactions with applications.
This solution maintains a basic level of control over eviction order,
while maintaining that, overall, secure cookies should be lowest on the
eviction ladder (or highest: whichever you think means 'evicted later').

BUG= 591686 , 546820 
R=mkwst@chromium.org

Review-Url: https://codereview.chromium.org/1939623002
Cr-Commit-Position: refs/heads/master@{#391926}

[modify] https://crrev.com/c00ac71cdb3caff6710e3c35c0b23493c3cde0d3/net/cookies/cookie_monster.cc
[modify] https://crrev.com/c00ac71cdb3caff6710e3c35c0b23493c3cde0d3/net/cookies/cookie_monster.h
[modify] https://crrev.com/c00ac71cdb3caff6710e3c35c0b23493c3cde0d3/net/cookies/cookie_monster_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment