Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(649)

Issue 1762693002: Revert of Evict non-secure cookies less agressively. (Closed)

Created:
4 years, 9 months ago by Mike West
Modified:
4 years, 9 months ago
Reviewers:
jww, mmenke
CC:
cbentzel+watch_chromium.org, chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Evict non-secure cookies less agressively. (patchset #4 id:60001 of https://codereview.chromium.org/1705753002/ ) Reason for revert: Let's see if the automagical reversion thingie will work on a ~2 week old patch. ``` 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. ``` BUG=591720 Original issue's description: > Evict non-secure cookies less agressively. > > The current implementation of strict-secure cookies wipes all non-secure > cookies when an origin exceeds ~180. This is a bit agressive, and has > real impact on users. > > This patch softens that to remove only the number of non-secure cookies > necessary to get under the ~150 cap. If a user has 150 secure cookies, > we'll still wipe all non-secure cookies, but that seems less likely to > impact users than the current behavior. > > The patch is a bit more complicated than I expected due to interactions > with the Chrome-only "priority" feature we shipped back in 2013. In > short, we execute priority-driven removal of non-secure cookies first, > and only touch secure cookies if necessary. > > BUG=581588 > R=mmenke@chromium.org, jww@chromium.org > > Committed: https://crrev.com/162d27135f2ee44ae01341de055d1b827a930767 > Cr-Commit-Position: refs/heads/master@{#376204} TBR=jww@chromium.org,mmenke@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=581588 Committed: https://crrev.com/8773435ab4441ce8f78611b694e92659e9b50a63 Cr-Commit-Position: refs/heads/master@{#379030}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+160 lines, -422 lines) Patch
M net/cookies/cookie_monster.h View 3 chunks +3 lines, -28 lines 0 comments Download
M net/cookies/cookie_monster.cc View 5 chunks +84 lines, -174 lines 0 comments Download
M net/cookies/cookie_monster_unittest.cc View 8 chunks +73 lines, -220 lines 0 comments Download

Messages

Total messages: 8 (1 generated)
Mike West
Created Revert of Evict non-secure cookies less agressively.
4 years, 9 months ago (2016-03-03 16:10:21 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1762693002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1762693002/1
4 years, 9 months ago (2016-03-03 16:10:35 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1762693002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1762693002/1
4 years, 9 months ago (2016-03-03 16:31:44 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1762693002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1762693002/1
4 years, 9 months ago (2016-03-03 17:01:36 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1762693002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1762693002/1
4 years, 9 months ago (2016-03-03 17:31:32 UTC) #5
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 9 months ago (2016-03-03 17:36:31 UTC) #6
commit-bot: I haz the power
4 years, 9 months ago (2016-03-03 17:37:42 UTC) #8
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/8773435ab4441ce8f78611b694e92659e9b50a63
Cr-Commit-Position: refs/heads/master@{#379030}

Powered by Google App Engine
This is Rietveld 408576698