Chromium Code Reviews Chromium Code Reviews
Issue
2074203002:
Reland of Making cookies eviction quotas match spec (Closed)
DescriptionReland of Making cookies eviction quotas match spec (patchset #1 id:1 of https://codereview.chromium.org/2071593002/ )
Reason for revert:
Relanding. Reverting this did not fix the performance regression.
BUG=618679, 609550
Original issue's description:
> Revert of Making cookies eviction quotas match spec (patchset #5 id:180001 of https://codereview.chromium.org/1976073002/ )
>
> Reason for revert:
> Experimentally reverting, seems to have possibly caused a perf regression. See https://crbug.com/618679
>
> Original issue's description:
> > Making cookies eviction quotas match spec
> >
> > According to the section 3 and section 5 in
> > https://tools.ietf.org/html/draft-west-cookie-priority-
> > 00#section-3, the old cookie monster implementation doesn't
> > maintain priority quotas correctly.
> >
> > This CL makes sure there will always be at least 30 low
> > priority cookies, 50 mid priority and 70 high priority
> > cookies if there had been enough of them before the eviction.
> > Please note, secure cookies are more important than non-secure
> > per https://tools.ietf.org/html/draft-west-leave-secure-cookies-alone.
> >
> > A small example: if there are 70 cookies:
> > 37 non-secure low priority and 33 secure low priority
> > cookies, 37 non-secure cookies are deleted during the first
> > round and other 3 secure low priority cookies are deleted
> > during the following round preserving 30 low priority
> > cookies according to the quota of those specific cookies.
> > For a bigger example that fully complies with the
> > implementation, check the unittests.
> >
> > Before the fix, the unittests were just adjusted to the
> > behavior of cookies eviction.
> >
> > For example, if we take the following unittest:
> > Round 1 => 10L; round 2 => 11M, 10L; round 3 => none.
> > TestPriorityCookieCase(cm.get(), "11HN 10MN 20LN 110MN 20LN
> > 10HN", 20U,109U, 21U, 150U, 0U);
> > The problem here was that there were only 40 low priority
> > cookies, but the quota was not preserved for those cookies.
> > First, 10 low priority cookies were deleted and then more 10
> > low priority cookies were deleted leaving only 20 of them,
> > which was less than the quota (30 low priority cookies).
> > It happened because the eviction algorithm didn't know how
> > many cookies of a specific priority were deleted and
> > it had always started to delete all the cookies from the
> > beginning of the container removing even those cookies
> > that shouldn't have been deleted.
> >
> > After we land this CL, we can have cookies in any order and
> > high priority cookies will be eventually deleted in order
> > to avoid excess of high priority cookies by some
> > applications within the same domain. Thus, after the
> > eviction algorithm runs, we should have at least 30 low, 50
> > mid and 70 high priority cookies if we had sufficient
> > amount of them in the beginning.
> >
> > BUG=609550
> >
> > Committed: https://crrev.com/fe1fea6df040fff38a3a4567e20a3d5847e83170
> > Cr-Commit-Position: refs/heads/master@{#398049}
>
> TBR=jww@chromium.org,mkwst@chromium.org,maksim.sisov@intel.com
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=609550
>
> Committed: https://crrev.com/b89069d3f1167269aaf9fd117e743858475c3a37
> Cr-Commit-Position: refs/heads/master@{#399971}
TBR=jww@chromium.org,mkwst@chromium.org,maksim.sisov@intel.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=609550
Committed: https://crrev.com/645ca677647620b4c6425cf5f4c509f64e818157
Cr-Commit-Position: refs/heads/master@{#400469}
Patch Set 1 #
Messages
Total messages: 7 (3 generated)
|
||||||||||||||||||||||||||||
