Update cookie expiry policy to attempt to avoid deleting "in-use" cookies. S...

net/base/cookie_monster.cc

Index: net/base/cookie_monster.cc
===================================================================
--- net/base/cookie_monster.cc	(revision 23545)
+++ net/base/cookie_monster.cc	(working copy)
@@ -68,15 +68,12 @@
 
 namespace net {
 
+
 // Cookie garbage collection thresholds.  Based off of the Mozilla defaults.
-// It might seem scary to have a high purge value, but really it's not.  You
-// just make sure that you increase the max to cover the increase in purge,
-// and we would have been purging the same amount of cookies.  We're just
-// going through the garbage collection process less often.
-static const size_t kNumCookiesPerHost      = 70;  // ~50 cookies
-static const size_t kNumCookiesPerHostPurge = 20;
-static const size_t kNumCookiesTotal        = 3300;  // ~3000 cookies
-static const size_t kNumCookiesTotalPurge   = 300;
+static const size_t kMaxCookiesPerHost = 50;
+static const size_t kMaxCookiesTotal = 2000;
+static const int kMaxCookieAgeSeconds = 30 * 24 * 60 * 60;  // 30 days
+static const double kPurgeOverageFactor = 1.1;
 
 // Default minimum delay after updating a cookie's LastAccessDate before we
 // will update it again.
@@ -118,9 +115,12 @@
   // and sync'd.
   std::vector<KeyedCanonicalCookie> cookies;
   // Reserve space for the maximum amount of cookies a database should have.
-  // This prevents multiple vector growth / copies as we append cookies.
-  cookies.reserve(kNumCookiesTotal);
-  store_->Load(&cookies);
+  // This prevents multiple vector growth / copies as we append cookies.  Note
+  // that if we have too many recent cookies, we'll still cross this limit;
+  // however, since std::vector<> grows by 50% when inserting a new element
+  // into a full vector, we should have few of these additional resizes.
+  cookies.reserve(static_cast<int>(kMaxCookiesTotal * kPurgeOverageFactor));
+  store_->Load(&cookies, &least_recent_access_);
   for (std::vector<KeyedCanonicalCookie>::const_iterator it = cookies.begin();
        it != cookies.end(); ++it) {
     InternalInsertCookie(it->first, it->second, false);
@@ -516,6 +516,9 @@
   if (cc->IsPersistent() && store_ && sync_to_store)
     store_->AddCookie(key, *cc);
   cookies_.insert(CookieMap::value_type(key, cc));
+  const Time last_access = cc->LastAccessDate();
+  if (least_recent_access_.is_null() || (last_access < least_recent_access_))
+    least_recent_access_ = last_access;
 }
 
 void CookieMonster::InternalUpdateCookieAccessTime(CanonicalCookie* cc) {
@@ -530,6 +533,8 @@
   cc->SetLastAccessDate(current);
   if (cc->IsPersistent() && store_)
     store_->UpdateCookieAccessTime(*cc);
+  if (least_recent_access_.is_null() || (current < least_recent_access_))
+    least_recent_access_ = current;
 }
 
 void CookieMonster::InternalDeleteCookie(CookieMap::iterator it,
@@ -579,18 +584,22 @@
   int num_deleted = 0;
 
   // Collect garbage for this key.
-  if (cookies_.count(key) > kNumCookiesPerHost) {
+  if (cookies_.count(key) >= (kMaxCookiesPerHost * kPurgeOverageFactor)) {
     COOKIE_DLOG(INFO) << "GarbageCollect() key: " << key;
     num_deleted += GarbageCollectRange(current, cookies_.equal_range(key),
-        kNumCookiesPerHost, kNumCookiesPerHostPurge);
+        kMaxCookiesPerHost, TimeDelta());
   }
 
   // Collect garbage for everything.
-  if (cookies_.size() > kNumCookiesTotal) {
+  if ((cookies_.size() >= (kMaxCookiesTotal * kPurgeOverageFactor)) &&
+      (least_recent_access_.is_null() ||
+       ((current - least_recent_access_) >=
+           TimeDelta::FromSeconds(static_cast<int>(kMaxCookieAgeSeconds *
+               kPurgeOverageFactor))))) {
     COOKIE_DLOG(INFO) << "GarbageCollect() everything";
     num_deleted += GarbageCollectRange(current,
-        CookieMapItPair(cookies_.begin(), cookies_.end()), kNumCookiesTotal,
-        kNumCookiesTotalPurge);
+        CookieMapItPair(cookies_.begin(), cookies_.end()), kMaxCookiesTotal,
+        TimeDelta::FromSeconds(kMaxCookieAgeSeconds));
   }
 
   return num_deleted;
@@ -610,24 +619,36 @@
 
 int CookieMonster::GarbageCollectRange(const Time& current,
                                        const CookieMapItPair& itpair,
-                                       size_t num_max,
-                                       size_t num_purge) {
+                                       size_t max_cookies,
+                                       const TimeDelta& purge_age) {
   // First, delete anything that's expired.
   std::vector<CookieMap::iterator> cookie_its;
   int num_deleted = GarbageCollectExpired(current, itpair, &cookie_its);
 
   // If the range still has too many cookies, delete the least recently used.
-  if (cookie_its.size() > num_max) {
+  if (cookie_its.size() > max_cookies) {
     COOKIE_DLOG(INFO) << "GarbageCollectRange() Deep Garbage Collect.";
-    // Purge down to (|num_max| - |num_purge|) total cookies.
-    DCHECK(num_purge <= num_max);
-    num_purge += cookie_its.size() - num_max;
+    // Purge down to |max_cookies| total cookies.
+    size_t num_purge = cookie_its.size() - max_cookies;
 
-    std::partial_sort(cookie_its.begin(), cookie_its.begin() + num_purge,
-                      cookie_its.end(), LRUCookieSorter);
-    for (size_t i = 0; i < num_purge; ++i)
+    std::partial_sort(cookie_its.begin(),
+        cookie_its.begin() + std::min(cookie_its.size(), num_purge + 1),
+        cookie_its.end(), LRUCookieSorter);
+    const bool only_purge_old_cookies = (purge_age != TimeDelta());
+    for (size_t i = 0; i < num_purge;) {
+      // Stop purging once we've run out of cookies that are "old enough".
+      if (only_purge_old_cookies &&
+          ((current - cookie_its[i]->second->LastAccessDate()) < purge_age))
+        return num_deleted + i;
+
       InternalDeleteCookie(cookie_its[i], true);
 
+      // Update least_recent_access_ now that we've deleted the LRU cookie.
+      ++i;
+      if (i < cookie_its.size())
+        least_recent_access_ = cookie_its[i]->second->LastAccessDate();
+    }
+
     num_deleted += num_purge;
   }
 
@@ -666,6 +687,8 @@
     ++num_deleted;
   }
 
+  least_recent_access_ = Time();
+
   return num_deleted;
 }