This is the final change for issue 8850, changing the expiry behavior
of cookies to match Firefox in keeping around cookies that have been
accessed within the last 30 days.
More detailed summary of changes:
* Key scheme specification expanded to include expiry scheme (old vs. new).
* GC code refactored to make stats collection and variations in GC simpler.
* New tests written for probing GC.
BUG=8850
TEST=All known cookie tests. Several new tests written for this change.
Review URL: http://codereview.chromium.org/3323025
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@61189 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc
index f9ec08fe..a75420f 100644
--- a/net/base/cookie_monster.cc
+++ b/net/base/cookie_monster.cc
@@ -76,10 +76,11 @@
// See comments at declaration of these variables in cookie_monster.h
// for details.
-const size_t CookieMonster::kDomainMaxCookies = 180;
-const size_t CookieMonster::kDomainPurgeCookies = 30;
-const size_t CookieMonster::kMaxCookies = 3300;
-const size_t CookieMonster::kPurgeCookies = 300;
+const size_t CookieMonster::kDomainMaxCookies = 180;
+const size_t CookieMonster::kDomainPurgeCookies = 30;
+const size_t CookieMonster::kMaxCookies = 3300;
+const size_t CookieMonster::kPurgeCookies = 300;
+const int CookieMonster::kSafeFromGlobalPurgeDays = 30;
namespace {
@@ -108,7 +109,7 @@
CookieMonster::CookieMonster(PersistentCookieStore* store, Delegate* delegate)
: initialized_(false),
- use_effective_domain_key_scheme_(use_effective_domain_key_default_),
+ expiry_and_key_scheme_(expiry_and_key_default_),
store_(store),
last_access_threshold_(
TimeDelta::FromSeconds(kDefaultAccessUpdateThresholdSeconds)),
@@ -180,7 +181,7 @@
// From UMA_HISTOGRAM_ENUMERATION
histogram_cookie_deletion_cause_ = LinearHistogram::FactoryGet(
"Cookie.DeletionCause", 1,
- DELETE_COOKIE_LAST_ENTRY, DELETE_COOKIE_LAST_ENTRY + 1,
+ DELETE_COOKIE_LAST_ENTRY - 1, DELETE_COOKIE_LAST_ENTRY,
Histogram::kUmaTargetedHistogramFlag);
// From UMA_HISTOGRAM_{CUSTOM_,}TIMES
@@ -210,12 +211,20 @@
// that way we don't have to worry about what sections of code are safe
// to call while it's in that state.
std::set<int64> creation_times;
+
+ // Presumably later than any access time in the store.
+ Time earliest_access_time;
+
for (std::vector<CanonicalCookie*>::const_iterator it = cookies.begin();
it != cookies.end(); ++it) {
int64 cookie_creation_time = (*it)->CreationDate().ToInternalValue();
if (creation_times.insert(cookie_creation_time).second) {
InternalInsertCookie(GetKey((*it)->Domain()), *it, false);
+ const Time cookie_access_time((*it)->LastAccessDate());
+ if (earliest_access_time.is_null() ||
+ cookie_access_time < earliest_access_time)
+ earliest_access_time = cookie_access_time;
} else {
LOG(ERROR) << base::StringPrintf("Found cookies with duplicate creation "
"times in backing store: "
@@ -228,6 +237,7 @@
delete (*it);
}
}
+ earliest_access_time_= earliest_access_time;
// After importing cookies from the PersistentCookieStore, verify that
// none of our other constraints are violated.
@@ -391,9 +401,9 @@
SetCookieableSchemes(kDefaultCookieableSchemes, num_schemes);
}
-void CookieMonster::SetKeyScheme(bool use_effective_domain_key) {
+void CookieMonster::SetExpiryAndKeyScheme(ExpiryAndKeyScheme key_scheme) {
DCHECK(!initialized_);
- use_effective_domain_key_scheme_ = use_effective_domain_key;
+ expiry_and_key_scheme_ = key_scheme;
}
// The system resolution is not high enough, so we can have multiple
@@ -563,7 +573,7 @@
// be worth it, but is still too much trouble to solve what is currently a
// non-problem).
std::string CookieMonster::GetKey(const std::string& domain) const {
- if (!use_effective_domain_key_scheme_)
+ if (expiry_and_key_scheme_ == EKS_DISCARD_RECENT_AND_PURGE_DOMAIN)
return domain;
std::string effective_domain(
@@ -943,6 +953,72 @@
return skipped_httponly;
}
+static bool LRUCookieSorter(const CookieMonster::CookieMap::iterator& it1,
+ const CookieMonster::CookieMap::iterator& it2) {
+ // Cookies accessed less recently should be deleted first.
+ if (it1->second->LastAccessDate() != it2->second->LastAccessDate())
+ return it1->second->LastAccessDate() < it2->second->LastAccessDate();
+
+ // In rare cases we might have two cookies with identical last access times.
+ // To preserve the stability of the sort, in these cases prefer to delete
+ // older cookies over newer ones. CreationDate() is guaranteed to be unique.
+ return it1->second->CreationDate() < it2->second->CreationDate();
+}
+
+// Helper for GarbageCollection. If |cookie_its->size() > num_max|, remove the
+// |num_max - num_purge| most recently accessed cookies from cookie_its.
+// (In other words, leave the entries that are candidates for
+// eviction in cookie_its.) The cookies returned will be in order sorted by
+// access time, least recently accessed first. The access time of the least
+// recently accessed entry not returned will be placed in
+// |*lra_removed| if that pointer is set. FindLeastRecentlyAccessed
+// returns false if no manipulation is done (because the list size is less
+// than num_max), true otherwise.
+static bool FindLeastRecentlyAccessed(
+ size_t num_max,
+ size_t num_purge,
+ Time* lra_removed,
+ std::vector<CookieMonster::CookieMap::iterator>* cookie_its) {
+ DCHECK_LE(num_purge, num_max);
+ if (cookie_its->size() > num_max) {
+ COOKIE_DLOG(INFO) << "FindLeastRecentlyAccessed() Deep Garbage Collect.";
+ num_purge += cookie_its->size() - num_max;
+ DCHECK_GT(cookie_its->size(), num_purge);
+
+ // Add 1 so that we can get the last time left in the store.
+ std::partial_sort(cookie_its->begin(), cookie_its->begin() + num_purge + 1,
+ cookie_its->end(), LRUCookieSorter);
+ *lra_removed =
+ (*(cookie_its->begin() + num_purge))->second->LastAccessDate();
+ cookie_its->erase(cookie_its->begin() + num_purge, cookie_its->end());
+ return true;
+ }
+ return false;
+}
+
+int CookieMonster::GarbageCollectDeleteList(
+ const Time& current,
+ const Time& keep_accessed_after,
+ DeletionCause cause,
+ std::vector<CookieMap::iterator>& cookie_its) {
+ int num_deleted = 0;
+ for (std::vector<CookieMap::iterator>::iterator it = cookie_its.begin();
+ it != cookie_its.end(); it++) {
+ if ((*it)->second->LastAccessDate() < keep_accessed_after) {
+ histogram_evicted_last_access_minutes_->Add(
+ (current - (*it)->second->LastAccessDate()).InMinutes());
+ InternalDeleteCookie((*it), true, cause);
+ num_deleted++;
+ }
+ }
+ return num_deleted;
+}
+
+// Domain expiry behavior is unchanged by key/expiry scheme (the
+// meaning of the key is different, but that's not visible to this
+// routine). Global garbage collection is dependent on key/expiry
+// scheme in that recently touched cookies are not saved if
+// expiry_and_key_scheme_ == EKS_DISCARD_RECENT_AND_PURGE_DOMAIN.
int CookieMonster::GarbageCollect(const Time& current,
const std::string& key) {
lock_.AssertAcquired();
@@ -954,40 +1030,74 @@
COOKIE_DLOG(INFO) << "GarbageCollect() key: " << key;
std::vector<CookieMap::iterator> cookie_its;
- num_deleted += GarbageCollectExpired(current, cookies_.equal_range(key),
- &cookie_its);
- num_deleted += GarbageCollectEvict(
- current, kDomainMaxCookies, kDomainPurgeCookies,
- DELETE_COOKIE_EVICTED_DOMAIN, &cookie_its);
+ num_deleted += GarbageCollectExpired(
+ current, cookies_.equal_range(key), &cookie_its);
+ base::Time oldest_removed;
+ if (FindLeastRecentlyAccessed(kDomainMaxCookies, kDomainPurgeCookies,
+ &oldest_removed, &cookie_its)) {
+ // Delete in two passes so we can figure out what we're nuking
+ // that would be kept at the global level.
+ int num_subject_to_global_purge =
+ GarbageCollectDeleteList(
+ current,
+ Time::Now() - TimeDelta::FromDays(kSafeFromGlobalPurgeDays),
+ DELETE_COOKIE_EVICTED_DOMAIN_PRE_SAFE,
+ cookie_its);
+ num_deleted += num_subject_to_global_purge;
+ // Correct because FindLeastRecentlyAccessed returns a sorted list.
+ cookie_its.erase(cookie_its.begin(),
+ cookie_its.begin() + num_subject_to_global_purge);
+ num_deleted +=
+ GarbageCollectDeleteList(
+ current,
+ Time::Now(),
+ DELETE_COOKIE_EVICTED_DOMAIN_POST_SAFE,
+ cookie_its);
+ }
}
- // Collect garbage for everything.
- if (cookies_.size() > kMaxCookies) {
+ // Collect garbage for everything. With firefox style we want to
+ // preserve cookies touched in kSafeFromGlobalPurgeDays, otherwise
+ // not.
+ if (cookies_.size() > kMaxCookies &&
+ (expiry_and_key_scheme_ == EKS_DISCARD_RECENT_AND_PURGE_DOMAIN ||
+ earliest_access_time_ <
+ Time::Now() - TimeDelta::FromDays(kSafeFromGlobalPurgeDays))) {
COOKIE_DLOG(INFO) << "GarbageCollect() everything";
std::vector<CookieMap::iterator> cookie_its;
+ base::Time oldest_left;
num_deleted += GarbageCollectExpired(
current, CookieMapItPair(cookies_.begin(), cookies_.end()),
&cookie_its);
- num_deleted += GarbageCollectEvict(
- current, kMaxCookies, kPurgeCookies,
- DELETE_COOKIE_EVICTED_GLOBAL, &cookie_its);
+ if (FindLeastRecentlyAccessed(kMaxCookies, kPurgeCookies,
+ &oldest_left, &cookie_its)) {
+ Time oldest_safe_cookie(
+ expiry_and_key_scheme_ == EKS_KEEP_RECENT_AND_PURGE_ETLDP1 ?
+ (Time::Now() - TimeDelta::FromDays(kSafeFromGlobalPurgeDays)) :
+ Time::Now());
+ int num_evicted = GarbageCollectDeleteList(
+ current,
+ oldest_safe_cookie,
+ DELETE_COOKIE_EVICTED_GLOBAL,
+ cookie_its);
+
+ // If no cookies were preserved by the time limit, the global last
+ // access is set to the value returned from FindLeastRecentlyAccessed.
+ // If the time limit preserved some cookies, we use the last access of
+ // the oldest preserved cookie.
+ if (num_evicted == static_cast<int>(cookie_its.size())) {
+ earliest_access_time_ = oldest_left;
+ } else {
+ earliest_access_time_ =
+ (*(cookie_its.begin() + num_evicted))->second->LastAccessDate();
+ }
+ num_deleted += num_evicted;
+ }
}
return num_deleted;
}
-static bool LRUCookieSorter(const CookieMonster::CookieMap::iterator& it1,
- const CookieMonster::CookieMap::iterator& it2) {
- // Cookies accessed less recently should be deleted first.
- if (it1->second->LastAccessDate() != it2->second->LastAccessDate())
- return it1->second->LastAccessDate() < it2->second->LastAccessDate();
-
- // In rare cases we might have two cookies with identical last access times.
- // To preserve the stability of the sort, in these cases prefer to delete
- // older cookies over newer ones. CreationDate() is guaranteed to be unique.
- return it1->second->CreationDate() < it2->second->CreationDate();
-}
-
int CookieMonster::GarbageCollectExpired(
const Time& current,
const CookieMapItPair& itpair,
@@ -1010,32 +1120,6 @@
return num_deleted;
}
-int CookieMonster::GarbageCollectEvict(
- const Time& current,
- size_t num_max,
- size_t num_purge,
- DeletionCause cause,
- std::vector<CookieMap::iterator>* cookie_its) {
- if (cookie_its->size() <= num_max) {
- return 0;
- }
-
- COOKIE_DLOG(INFO) << "GarbageCollectEvict() Deep Garbage Collect.";
- // Purge down to (|num_max| - |num_purge|) total cookies.
- DCHECK_LE(num_purge, num_max);
- num_purge += cookie_its->size() - num_max;
-
- std::partial_sort(cookie_its->begin(), cookie_its->begin() + num_purge,
- cookie_its->end(), LRUCookieSorter);
- for (size_t i = 0; i < num_purge; ++i) {
- // See InitializeHistograms() for details.
- histogram_evicted_last_access_minutes_->Add(
- (current - (*cookie_its)[i]->second->LastAccessDate()).InMinutes());
- InternalDeleteCookie((*cookie_its)[i], true, cause);
- }
- return num_purge;
-}
-
int CookieMonster::DeleteAll(bool sync_to_store) {
AutoLock autolock(lock_);
InitIfNecessary();
@@ -1285,7 +1369,7 @@
// want to collect statistics whenever the browser's being used.
RecordPeriodicStats(current_time);
- if (use_effective_domain_key_scheme_) {
+ if (expiry_and_key_scheme_ == EKS_DISCARD_RECENT_AND_PURGE_DOMAIN) {
// Can just dispatch to FindCookiesForKey
const std::string key(GetKey(url.host()));
FindCookiesForKey(key, url, options, current_time,
@@ -1355,7 +1439,8 @@
continue;
// Filter out cookies that don't apply to this domain.
- if (use_effective_domain_key_scheme_ && !cc->IsDomainMatch(scheme, host))
+ if (expiry_and_key_scheme_ == EKS_KEEP_RECENT_AND_PURGE_ETLDP1
+ && !cc->IsDomainMatch(scheme, host))
continue;
if (!cc->IsOnPath(url.path()))
@@ -1756,12 +1841,12 @@
Delegate* delegate,
int last_access_threshold_milliseconds)
: initialized_(false),
- use_effective_domain_key_scheme_(use_effective_domain_key_default_),
- store_(store),
- last_access_threshold_(base::TimeDelta::FromMilliseconds(
- last_access_threshold_milliseconds)),
- delegate_(delegate),
- last_statistic_record_time_(base::Time::Now()) {
+ expiry_and_key_scheme_(expiry_and_key_default_),
+ store_(store),
+ last_access_threshold_(base::TimeDelta::FromMilliseconds(
+ last_access_threshold_milliseconds)),
+ delegate_(delegate),
+ last_statistic_record_time_(base::Time::Now()) {
InitializeHistograms();
SetDefaultCookieableSchemes();
}
diff --git a/net/base/cookie_monster.h b/net/base/cookie_monster.h
index 80638764..80535c2 100644
--- a/net/base/cookie_monster.h
+++ b/net/base/cookie_monster.h
@@ -64,20 +64,21 @@
// structures (the data structures are owned by the CookieMonster
// and must be destroyed when removed from the map). There are two
// possible keys for the map, controlled on a per-CookieMonster basis
- // by use_effective_domain_key_scheme_/SetKeyScheme()
- // (defaulted by use_effective_domain_key_default_):
+ // by expiry_and_key_scheme_/SetExpiryAndKeyScheme()
+ // (defaulted by expiry_and_key_default_):
- // If use_effective_domain_key_scheme_ is true (default), then the key is
- // based on the effective domain of the cookies. If the domain
- // of the cookie has an eTLD+1, that is the key for the map. If the
- // domain of the cookie does not have an eTLD+1, the key of the map
- // is the host the cookie applies to (it is not legal to have domain
- // cookies without an eTLD+1). This rule excludes cookies for,
- // e.g, ".com", ".co.uk", or ".internalnetwork".
+ // If expiry_and_key_scheme_ is EKS_KEEP_RECENT_AND_PURGE_ETLDP1
+ // (default), then the key is based on the effective domain of the
+ // cookies. If the domain of the cookie has an eTLD+1, that is the
+ // key for the map. If the domain of the cookie does not have an eTLD+1,
+ // the key of the map is the host the cookie applies to (it is not
+ // legal to have domain cookies without an eTLD+1). This rule
+ // excludes cookies for, e.g, ".com", ".co.uk", or ".internalnetwork".
+ // This behavior is the same as the behavior in Firefox v 3.6.10.
- // If use_effective_domain_key_scheme_ is false, then the key is
- // just the domain of the cookie. Eventually, this option will be
- // removed.
+ // If use_effective_domain_key_scheme_ is EKS_DISCARD_RECENT_AND_PURGE_DOMAIN,
+ // then the key is just the domain of the cookie. Eventually, this
+ // option will be removed.
// NOTE(deanm):
// I benchmarked hash_multimap vs multimap. We're going to be query-heavy
@@ -92,6 +93,18 @@
typedef std::pair<CookieMap::iterator, CookieMap::iterator> CookieMapItPair;
typedef std::vector<CanonicalCookie> CookieList;
+ // The key and expiry scheme to be used by the monster.
+ // EKS_KEEP_RECENT_AND_PURGE_ETLDP1 means to use
+ // the new key scheme based on effective domain and save recent cookies
+ // in global garbage collection. EKS_DISCARD_RECENT_AND_PURGE_DOMAIN
+ // means to use the old key scheme based on full domain and be ruthless
+ // about purging.
+ enum ExpiryAndKeyScheme {
+ EKS_KEEP_RECENT_AND_PURGE_ETLDP1,
+ EKS_DISCARD_RECENT_AND_PURGE_DOMAIN,
+ EKS_LAST_ENTRY
+ };
+
// The store passed in should not have had Init() called on it yet. This
// class will take care of initializing it. The backing store is NOT owned by
// this class, but it must remain valid for the duration of the cookie
@@ -185,9 +198,10 @@
// the instance (i.e. as part of the instance initialization process).
void SetCookieableSchemes(const char* schemes[], size_t num_schemes);
- // Overrides the default key scheme. This function must be called
- // before initialization.
- void SetKeyScheme(bool use_effective_domain_key);
+ // Overrides the default key and expiry scheme. See comments
+ // before CookieMap and Garbage collection constants for details. This
+ // function must be called before initialization.
+ void SetExpiryAndKeyScheme(ExpiryAndKeyScheme key_scheme);
// There are some unknowns about how to correctly handle file:// cookies,
// and our implementation for this is not robust enough. This allows you
@@ -209,6 +223,8 @@
// For gargage collection constants.
FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest, TestHostGarbageCollection);
FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest, TestTotalGarbageCollection);
+ FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest, GarbageCollectionTriggers);
+ FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest, TestGCTimes);
// For validation of key values.
FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest, TestDomainTree);
@@ -227,6 +243,12 @@
// CookieMap typedef. So, e.g., the maximum number of cookies allowed for
// google.com and all of its subdomains will be 150-180.
//
+ // If the expiry and key scheme follows firefox standards (default,
+ // set by SetExpiryAndKeyScheme()), any cookies accessed more recently
+ // than kSafeFromGlobalPurgeDays will not be evicted by global garbage
+ // collection, even if we have more than kMaxCookies. This does not affect
+ // domain garbage collection.
+ //
// Present in .h file to make accessible to tests through FRIEND_TEST.
// Actual definitions are in cookie_monster.cc.
static const size_t kDomainMaxCookies;
@@ -234,10 +256,13 @@
static const size_t kMaxCookies;
static const size_t kPurgeCookies;
- // Default value for key scheme. true means to use the new
- // key scheme based on effective domain; false to use the
- // old key scheme based on full domain.
- static const bool use_effective_domain_key_default_ = true;
+ // The number of days since last access that cookies will not be subject
+ // to global garbage collection.
+ static const int kSafeFromGlobalPurgeDays;
+
+ // Default value for key and expiry scheme scheme.
+ static const ExpiryAndKeyScheme expiry_and_key_default_ =
+ EKS_KEEP_RECENT_AND_PURGE_ETLDP1;
bool SetCookieWithCreationTime(const GURL& url,
const std::string& cookie_line,
@@ -326,7 +351,17 @@
DELETE_COOKIE_DONT_RECORD, // e.g. For final cleanup after flush to store.
DELETE_COOKIE_EVICTED_DOMAIN,
DELETE_COOKIE_EVICTED_GLOBAL,
- DELETE_COOKIE_LAST_ENTRY = DELETE_COOKIE_EVICTED_GLOBAL
+
+ // Cookies evicted during domain level garbage collection that
+ // were accessed longer ago than kSafeFromGlobalPurgeDays
+ DELETE_COOKIE_EVICTED_DOMAIN_PRE_SAFE,
+
+ // Cookies evicted during domain level garbage collection that
+ // were accessed more rencelyt than kSafeFromGlobalPurgeDays
+ // (and thus would have been preserved by global garbage collection).
+ DELETE_COOKIE_EVICTED_DOMAIN_POST_SAFE,
+
+ DELETE_COOKIE_LAST_ENTRY
};
// |deletion_cause| argument is for collecting statistics.
@@ -335,12 +370,13 @@
// If the number of cookies for CookieMap key |key|, or globally, are
// over the preset maximums above, garbage collect, first for the host and
- // then globally, as described by GarbageCollectRange().
+ // then globally. See comments above garbage collection threshold
+ // constants for details.
//
// Returns the number of cookies deleted (useful for debugging).
int GarbageCollect(const base::Time& current, const std::string& key);
- // Helper for GarbageCollectRange(); can be called directly as well. Deletes
+ // Helper for GarbageCollect(); can be called directly as well. Deletes
// all expired cookies in |itpair|. If |cookie_its| is non-NULL, it is
// populated with all the non-expired cookies from |itpair|.
//
@@ -349,14 +385,13 @@
const CookieMapItPair& itpair,
std::vector<CookieMap::iterator>* cookie_its);
- // If needed, evicts least recently accessed cookies in iterator
- // list until (|num_max| - |num_purge|) cookies remain.
- int GarbageCollectEvict(
- const base::Time& current,
- size_t num_max,
- size_t num_purge,
- DeletionCause cause,
- std::vector<CookieMap::iterator>* cookie_its);
+ // Helper for GarbageCollect(). Deletes all cookies in the list
+ // that were accessed before |keep_accessed_after|, using DeletionCause
+ // |cause|. Returns the number of cookies deleted.
+ int GarbageCollectDeleteList(const base::Time& current,
+ const base::Time& keep_accessed_after,
+ DeletionCause cause,
+ std::vector<CookieMap::iterator>& cookie_its);
// Find the key (for lookup in cookies_) based on the given domain.
// See comment on keys before the CookieMap typedef.
@@ -398,7 +433,7 @@
// Indicates whether this cookie monster uses the new effective domain
// key scheme or not.
- bool use_effective_domain_key_scheme_;
+ ExpiryAndKeyScheme expiry_and_key_scheme_;
scoped_refptr<PersistentCookieStore> store_;
@@ -411,6 +446,17 @@
// update it again.
const base::TimeDelta last_access_threshold_;
+ // Approximate date of access time of least recently accessed cookie
+ // in |cookies_|. Note that this is not guaranteed to be accurate, only a)
+ // to be before or equal to the actual time, and b) to be accurate
+ // immediately after a garbage collection that scans through all the cookies.
+ // This value is used to determine whether global garbage collection might
+ // find cookies to purge.
+ // Note: The default Time() constructor will create a value that compares
+ // earlier than any other time value, which is is wanted. Thus this
+ // value is not initialized.
+ base::Time earliest_access_time_;
+
std::vector<std::string> cookieable_schemes_;
scoped_refptr<Delegate> delegate_;
diff --git a/net/base/cookie_monster_perftest.cc b/net/base/cookie_monster_perftest.cc
index 5326654..c0061287 100644
--- a/net/base/cookie_monster_perftest.cc
+++ b/net/base/cookie_monster_perftest.cc
@@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "net/base/cookie_monster.h"
+
#include "base/perftimer.h"
#include "base/string_util.h"
#include "base/stringprintf.h"
@@ -238,4 +240,64 @@
timer.Done();
}
+// This test is probing for whether garbage collection happens when it
+// shouldn't. This will not in general be visible functionally, since
+// if GC runs twice in a row without any change to the store, the second
+// GC run will not do anything the first one didn't. That's why this is
+// a performance test. The test should be considered to pass if all the
+// times reported are approximately the same--this indicates that no GC
+// happened repeatedly for any case.
+TEST(CookieMonsterTest, TestGCTimes) {
+ const struct TestCase {
+ const char* name;
+ int num_cookies;
+ int num_old_cookies;
+ } test_cases[] = {
+ {
+ // A whole lot of recent cookies; gc shouldn't happen.
+ "all_recent",
+ CookieMonster::kMaxCookies * 2,
+ 0,
+ }, {
+ // Some old cookies, but still overflowing max.
+ "mostly_recent",
+ CookieMonster::kMaxCookies * 2,
+ CookieMonster::kMaxCookies / 2,
+ }, {
+ // Old cookies enough to bring us right down to our purge line.
+ "balanced",
+ CookieMonster::kMaxCookies * 2,
+ CookieMonster::kMaxCookies + CookieMonster::kPurgeCookies + 1,
+ }, {
+ "mostly_old",
+ // Old cookies enough to bring below our purge line (which we
+ // shouldn't do).
+ CookieMonster::kMaxCookies * 2,
+ CookieMonster::kMaxCookies * 3 / 4,
+ }, {
+ "less_than_gc_thresh",
+ // Few enough cookies that gc shouldn't happen at all.
+ CookieMonster::kMaxCookies - 5,
+ 0,
+ },
+ };
+ for (int ci = 0; ci < static_cast<int>(ARRAYSIZE_UNSAFE(test_cases)); ++ci) {
+ const TestCase& test_case(test_cases[ci]);
+ scoped_refptr<CookieMonster> cm =
+ CreateMonsterFromStoreForGC(
+ test_case.num_cookies, test_case.num_old_cookies,
+ CookieMonster::kSafeFromGlobalPurgeDays * 2);
+
+ GURL gurl("http://google.com");
+ std::string cookie_line("z=3");
+ // Trigger the Garbage collection we're allowed.
+ EXPECT_TRUE(cm->SetCookie(gurl, cookie_line));
+
+ PerfTimeLogger timer((std::string("GC_") + test_case.name).c_str());
+ for (int i = 0; i < kNumCookies; i++)
+ EXPECT_TRUE(cm->SetCookie(gurl, cookie_line));
+ timer.Done();
+ }
+}
+
} // namespace
diff --git a/net/base/cookie_monster_store_test.h b/net/base/cookie_monster_store_test.h
index 79d919f..c54e1c13 100644
--- a/net/base/cookie_monster_store_test.h
+++ b/net/base/cookie_monster_store_test.h
@@ -8,6 +8,7 @@
// It should only be included by test code.
#include "base/time.h"
+#include "net/base/cookie_monster.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace {
@@ -144,4 +145,91 @@
out_list->push_back(cookie.release());
}
+// Just act like a backing database. Keep cookie information from
+// Add/Update/Delete and regurgitate it when Load is called.
+class MockSimplePersistentCookieStore
+ : public net::CookieMonster::PersistentCookieStore {
+ public:
+ typedef std::vector<net::CookieMonster::CanonicalCookie>
+ CanonicalCookieVector;
+
+ virtual bool Load(
+ std::vector<net::CookieMonster::CanonicalCookie*>* out_cookies) {
+ for (CanonicalCookieVector::const_iterator it = cookies_.begin();
+ it != cookies_.end(); it++)
+ out_cookies->push_back(new net::CookieMonster::CanonicalCookie(*it));
+ return true;
+ }
+
+ virtual void AddCookie(
+ const net::CookieMonster::CanonicalCookie& cookie) {
+ for (CanonicalCookieVector::const_iterator it = cookies_.begin();
+ it != cookies_.end(); it++) {
+ // Caller must assure creation dates unique.
+ EXPECT_NE(cookie.CreationDate().ToInternalValue(),
+ it->CreationDate().ToInternalValue());
+ }
+ cookies_.push_back(cookie); // Copies.
+ }
+
+ virtual void UpdateCookieAccessTime(
+ const net::CookieMonster::CanonicalCookie& cookie) {
+ for (CanonicalCookieVector::iterator it = cookies_.begin();
+ it != cookies_.end(); it++) {
+ if (it->CreationDate() == cookie.CreationDate())
+ it->SetLastAccessDate(base::Time::Now());
+ }
+ }
+
+ virtual void DeleteCookie(
+ const net::CookieMonster::CanonicalCookie& cookie) {
+ for (CanonicalCookieVector::iterator it = cookies_.begin();
+ it != cookies_.end(); it++) {
+ if (it->CreationDate() == cookie.CreationDate()) {
+ cookies_.erase(it);
+ return;
+ }
+ }
+ }
+
+ private:
+ std::vector<net::CookieMonster::CanonicalCookie> cookies_;
+};
+
+// Helper function for creating a CookieMonster backed by a
+// MockSimplePersistentCookieStore for garbage collection testing.
+//
+// Fill the store through import with |num_cookies| cookies, |num_old_cookies|
+// with access time Now()-days_old, the rest with access time Now().
+// Do two SetCookies(). Return whether each of the two SetCookies() took
+// longer than |gc_perf_micros| to complete, and how many cookie were
+// left in the store afterwards.
+static net::CookieMonster* CreateMonsterFromStoreForGC(
+ int num_cookies,
+ int num_old_cookies,
+ int days_old) {
+ base::Time current(base::Time::Now());
+ base::Time past_creation(base::Time::Now() - base::TimeDelta::FromDays(1000));
+ scoped_refptr<MockSimplePersistentCookieStore> store(
+ new MockSimplePersistentCookieStore);
+ // Must expire to be persistent
+ for (int i = 0; i < num_old_cookies; i++) {
+ net::CookieMonster::CanonicalCookie cc(
+ "a", "1", StringPrintf("h%05d.izzle", i), "/path", false, false,
+ past_creation + base::TimeDelta::FromMicroseconds(i),
+ current - base::TimeDelta::FromDays(days_old),
+ true, current + base::TimeDelta::FromDays(30));
+ store->AddCookie(cc);
+ }
+ for (int i = num_old_cookies; i < num_cookies; i++) {
+ net::CookieMonster::CanonicalCookie cc(
+ "a", "1", StringPrintf("h%05d.izzle", i), "/path", false, false,
+ past_creation + base::TimeDelta::FromMicroseconds(i), current,
+ true, current + base::TimeDelta::FromDays(30));
+ store->AddCookie(cc);
+ }
+
+ return new net::CookieMonster(store, NULL);
+}
+
} // namespace
diff --git a/net/base/cookie_monster_unittest.cc b/net/base/cookie_monster_unittest.cc
index 83c7f7d3b..34a9635c 100644
--- a/net/base/cookie_monster_unittest.cc
+++ b/net/base/cookie_monster_unittest.cc
@@ -1032,16 +1032,17 @@
return std::count(str.begin(), str.end(), c);
}
-static void TestHostGarbageCollectHelper(int domain_max_cookies,
- int domain_purge_cookies,
- bool new_key_scheme) {
+static void TestHostGarbageCollectHelper(
+ int domain_max_cookies,
+ int domain_purge_cookies,
+ CookieMonster::ExpiryAndKeyScheme key_scheme) {
GURL url_google(kUrlGoogle);
const int more_than_enough_cookies =
(domain_max_cookies + domain_purge_cookies) * 2;
// Add a bunch of cookies on a single host, should purge them.
{
scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL, NULL));
- cm->SetKeyScheme(new_key_scheme);
+ cm->SetExpiryAndKeyScheme(key_scheme);
for (int i = 0; i < more_than_enough_cookies; ++i) {
std::string cookie = base::StringPrintf("a%03d=b", i);
EXPECT_TRUE(cm->SetCookie(url_google, cookie));
@@ -1055,14 +1056,15 @@
// Add a bunch of cookies on multiple hosts within a single eTLD.
// Should keep at least kDomainMaxCookies - kDomainPurgeCookies
- // between them. If we are using the (new) effective domain keying system
- // we shouldn't go above kDomainMaxCookies for both together.
- // If we're using the (old) domain keying system, each individual
- // domain shouldn't go above kDomainMaxCookies.
+ // between them. If we are using the effective domain keying system
+ // (EKS_KEEP_RECENT_AND_PURGE_ETLDP1) we shouldn't go above
+ // kDomainMaxCookies for both together. If we're using the domain
+ // keying system (EKS_DISCARD_RECENT_AND_PURGE_DOMAIN), each
+ // individual domain shouldn't go above kDomainMaxCookies.
GURL url_google_specific(kUrlGoogleSpecific);
{
scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL, NULL));
- cm->SetKeyScheme(new_key_scheme);
+ cm->SetExpiryAndKeyScheme(key_scheme);
for (int i = 0; i < more_than_enough_cookies; ++i) {
std::string cookie_general = base::StringPrintf("a%03d=b", i);
EXPECT_TRUE(cm->SetCookie(url_google, cookie_general));
@@ -1072,7 +1074,7 @@
EXPECT_NE(cookies_general.find(cookie_general), std::string::npos);
std::string cookies_specific = cm->GetCookies(url_google_specific);
EXPECT_NE(cookies_specific.find(cookie_specific), std::string::npos);
- if (new_key_scheme) {
+ if (key_scheme == CookieMonster::EKS_KEEP_RECENT_AND_PURGE_ETLDP1) {
EXPECT_LE((CountInString(cookies_general, '=') +
CountInString(cookies_specific, '=')),
domain_max_cookies);
@@ -1085,7 +1087,7 @@
// kDomainMaxCookies - kDomainPurgeCookies for both URLs.
std::string cookies_general = cm->GetCookies(url_google);
std::string cookies_specific = cm->GetCookies(url_google_specific);
- if (new_key_scheme) {
+ if (key_scheme == CookieMonster::EKS_KEEP_RECENT_AND_PURGE_ETLDP1) {
int total_cookies = (CountInString(cookies_general, '=') +
CountInString(cookies_specific, '='));
EXPECT_GE(total_cookies,
@@ -1105,43 +1107,12 @@
}
TEST(CookieMonsterTest, TestHostGarbageCollection) {
- TestHostGarbageCollectHelper(CookieMonster::kDomainMaxCookies,
- CookieMonster::kDomainPurgeCookies, false);
- TestHostGarbageCollectHelper(CookieMonster::kDomainMaxCookies,
- CookieMonster::kDomainPurgeCookies, true);
-}
-
-TEST(CookieMonsterTest, TestTotalGarbageCollection) {
- scoped_refptr<net::CookieMonster> cm(
- new net::CookieMonster(NULL, NULL, kLastAccessThresholdMilliseconds));
-
- // Add a bunch of cookies on a bunch of host, some should get purged.
- const GURL sticky_cookie("http://a0000.izzle");
- for (int i = 0; i < 4000; ++i) {
- GURL url(base::StringPrintf("http://a%04d.izzle", i));
- EXPECT_TRUE(cm->SetCookie(url, "a=b"));
- EXPECT_EQ("a=b", cm->GetCookies(url));
-
- // Keep touching the first cookie to ensure it's not purged (since it will
- // always have the most recent access time).
- if (!(i % 500)) {
- // Ensure the timestamps will be different enough to update.
- PlatformThread::Sleep(kLastAccessThresholdMilliseconds + 20);
- EXPECT_EQ("a=b", cm->GetCookies(sticky_cookie));
- }
- }
-
- // Check that cookies that still exist.
- for (int i = 0; i < 4000; ++i) {
- GURL url(base::StringPrintf("http://a%04d.izzle", i));
- if ((i == 0) || (i > 1001)) {
- // Cookies should still be around.
- EXPECT_FALSE(cm->GetCookies(url).empty());
- } else if (i < 701) {
- // Cookies should have gotten purged.
- EXPECT_TRUE(cm->GetCookies(url).empty());
- }
- }
+ TestHostGarbageCollectHelper(
+ CookieMonster::kDomainMaxCookies, CookieMonster::kDomainPurgeCookies,
+ CookieMonster::EKS_KEEP_RECENT_AND_PURGE_ETLDP1);
+ TestHostGarbageCollectHelper(
+ CookieMonster::kDomainMaxCookies, CookieMonster::kDomainPurgeCookies,
+ CookieMonster::EKS_DISCARD_RECENT_AND_PURGE_DOMAIN);
}
// Formerly NetUtilTest.CookieTest back when we used wininet's cookie handling.
@@ -1827,7 +1798,8 @@
scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL, NULL));
// This test is really only interesting if GetKey() actually does something.
- if (cm->use_effective_domain_key_scheme_) {
+ if (cm->expiry_and_key_scheme_ ==
+ CookieMonster::EKS_KEEP_RECENT_AND_PURGE_ETLDP1) {
EXPECT_EQ("google.com", cm->GetKey("www.google.com"));
EXPECT_EQ("google.izzie", cm->GetKey("www.google.izzie"));
EXPECT_EQ("google.izzie", cm->GetKey(".google.izzie"));
@@ -1847,78 +1819,6 @@
}
}
-// Just act like a backing database. Keep cookie information from
-// Add/Update/Delete and regurgitate it when Load is called.
-class MockSimplePersistentCookieStore
- : public CookieMonster::PersistentCookieStore {
- public:
- typedef std::vector<CookieMonster::CanonicalCookie> CanonicalCookieVector;
-
- virtual bool Load(std::vector<CookieMonster::CanonicalCookie*>* out_cookies);
-
- virtual void AddCookie(const net::CookieMonster::CanonicalCookie& cookie);
-
- virtual void UpdateCookieAccessTime(
- const CookieMonster::CanonicalCookie& cookie);
-
- virtual void DeleteCookie(const CookieMonster::CanonicalCookie& cookie);
-
- private:
- std::vector<CookieMonster::CanonicalCookie> cookies_;
-};
-
-bool MockSimplePersistentCookieStore::Load(
- std::vector<CookieMonster::CanonicalCookie*>* out_cookies) {
- for (CanonicalCookieVector::const_iterator it = cookies_.begin();
- it != cookies_.end(); it++)
- out_cookies->push_back(new CookieMonster::CanonicalCookie(*it));
- return true;
-}
-
-void MockSimplePersistentCookieStore::AddCookie(
- const net::CookieMonster::CanonicalCookie& cookie) {
- for (CanonicalCookieVector::const_iterator it = cookies_.begin();
- it != cookies_.end(); it++) {
- // Caller must assure creation dates unique.
- EXPECT_NE(cookie.CreationDate().ToInternalValue(),
- it->CreationDate().ToInternalValue());
- }
- cookies_.push_back(cookie);
-}
-
-void MockSimplePersistentCookieStore::UpdateCookieAccessTime(
- const CookieMonster::CanonicalCookie& cookie) {
- for (CanonicalCookieVector::iterator it = cookies_.begin();
- it != cookies_.end(); it++) {
- if (it->CreationDate() == cookie.CreationDate())
- it->SetLastAccessDate(base::Time::Now());
- }
-}
-
-void MockSimplePersistentCookieStore::DeleteCookie(
- const CookieMonster::CanonicalCookie& cookie) {
- for (CanonicalCookieVector::iterator it = cookies_.begin();
- it != cookies_.end(); it++) {
- if (it->CreationDate() == cookie.CreationDate()) {
- cookies_.erase(it);
- return;
- }
- }
-}
-
-// Helper type for BackingStoreCommunication; defined outside of the
-// function so that arraysize() macro can beu used.
-struct CookiesInputInfo {
- std::string gurl;
- std::string name;
- std::string value;
- std::string domain;
- std::string path;
- base::Time expires;
- bool secure;
- bool http_only;
-};
-
// Test that cookies transfer from/to the backing store correctly.
TEST(CookieMonsterTest, BackingStoreCommunication) {
// Store details for cookies transforming through the backing store interface.
@@ -1929,6 +1829,16 @@
base::Time new_access_time;
base::Time expires(base::Time::Now() + base::TimeDelta::FromSeconds(100));
+ struct CookiesInputInfo {
+ std::string gurl;
+ std::string name;
+ std::string value;
+ std::string domain;
+ std::string path;
+ base::Time expires;
+ bool secure;
+ bool http_only;
+ };
const CookiesInputInfo input_info[] = {
{"http://a.b.google.com", "a", "1", "", "/path/to/cookie", expires,
false, false},
@@ -1944,7 +1854,7 @@
{
scoped_refptr<net::CookieMonster> cmout = new CookieMonster(store, NULL);
for (const CookiesInputInfo* p = input_info;
- p < &input_info[arraysize(input_info)]; p++) {
+ p < &input_info[ARRAYSIZE_UNSAFE(input_info)]; p++) {
EXPECT_TRUE(cmout->SetCookieWithDetails(GURL(p->gurl), p->name, p->value,
p->domain, p->path, p->expires,
p->secure, p->http_only));
@@ -2027,4 +1937,100 @@
}
}
-} // namespace
+
+// Function for creating a CM with a number of cookies in it,
+// no store (and hence no ability to affect access time).
+static net::CookieMonster* CreateMonsterForGC(int num_cookies) {
+ net::CookieMonster* cm(new net::CookieMonster(NULL, NULL));
+ for (int i = 0; i < num_cookies; i++)
+ cm->SetCookie(GURL(StringPrintf("http://h%05d.izzle", i)), "a=1");
+ return cm;
+}
+
+// This test and CookieMonstertest.TestGCTimes (in cookie_monster_perftest.cc)
+// are somewhat complementary twins. This test is probing for whether
+// garbage collection always happens when it should (i.e. that we actually
+// get rid of cookies when we should). The perftest is probing for
+// whether garbage collection happens when it shouldn't. See comments
+// before that test for more details.
+TEST(CookieMonsterTest, GarbageCollectionTriggers) {
+ // First we check to make sure that a whole lot of recent cookies
+ // doesn't get rid of anything after garbage collection is checked for.
+ {
+ scoped_refptr<net::CookieMonster> cm =
+ CreateMonsterForGC(CookieMonster::kMaxCookies * 2);
+ EXPECT_EQ(CookieMonster::kMaxCookies * 2, cm->GetAllCookies().size());
+ cm->SetCookie(GURL("http://newdomain.com"), "b=2");
+ EXPECT_EQ(CookieMonster::kMaxCookies * 2 + 1, cm->GetAllCookies().size());
+ }
+
+ // Now we explore a series of relationships between cookie last access
+ // time and size of store to make sure we only get rid of cookies when
+ // we really should.
+ const struct TestCase {
+ int num_cookies;
+ int num_old_cookies;
+ int expected_initial_cookies;
+ // Indexed by ExpiryAndKeyScheme
+ int expected_cookies_after_set[CookieMonster::EKS_LAST_ENTRY];
+ } test_cases[] = {
+ {
+ // A whole lot of recent cookies; gc shouldn't happen.
+ CookieMonster::kMaxCookies * 2,
+ 0,
+ CookieMonster::kMaxCookies * 2,
+ { CookieMonster::kMaxCookies * 2 + 1,
+ CookieMonster::kMaxCookies - CookieMonster::kPurgeCookies }
+ }, {
+ // Some old cookies, but still overflowing max.
+ CookieMonster::kMaxCookies * 2,
+ CookieMonster::kMaxCookies / 2,
+ CookieMonster::kMaxCookies * 2,
+ {CookieMonster::kMaxCookies * 2 - CookieMonster::kMaxCookies / 2 + 1,
+ CookieMonster::kMaxCookies - CookieMonster::kPurgeCookies}
+ }, {
+ // Old cookies enough to bring us right down to our purge line.
+ CookieMonster::kMaxCookies * 2,
+ CookieMonster::kMaxCookies + CookieMonster::kPurgeCookies + 1,
+ CookieMonster::kMaxCookies * 2,
+ {CookieMonster::kMaxCookies - CookieMonster::kPurgeCookies,
+ CookieMonster::kMaxCookies - CookieMonster::kPurgeCookies}
+ }, {
+ // Old cookies enough to bring below our purge line (which we
+ // shouldn't do).
+ CookieMonster::kMaxCookies * 2,
+ CookieMonster::kMaxCookies * 3 / 2,
+ CookieMonster::kMaxCookies * 2,
+ {CookieMonster::kMaxCookies - CookieMonster::kPurgeCookies,
+ CookieMonster::kMaxCookies - CookieMonster::kPurgeCookies}
+ }
+ };
+ const CookieMonster::ExpiryAndKeyScheme schemes[] = {
+ CookieMonster::EKS_KEEP_RECENT_AND_PURGE_ETLDP1,
+ CookieMonster::EKS_DISCARD_RECENT_AND_PURGE_DOMAIN,
+ };
+
+ for (int ci = 0; ci < static_cast<int>(ARRAYSIZE_UNSAFE(test_cases)); ++ci) {
+ // Test both old and new key and expiry schemes.
+ for (int recent_scheme = 0;
+ recent_scheme < static_cast<int>(ARRAYSIZE_UNSAFE(schemes));
+ recent_scheme++) {
+ const TestCase *test_case = &test_cases[ci];
+ scoped_refptr<net::CookieMonster> cm =
+ CreateMonsterFromStoreForGC(
+ test_case->num_cookies, test_case->num_old_cookies,
+ CookieMonster::kSafeFromGlobalPurgeDays * 2);
+ cm->SetExpiryAndKeyScheme(schemes[recent_scheme]);
+ EXPECT_EQ(test_case->expected_initial_cookies,
+ static_cast<int>(cm->GetAllCookies().size()))
+ << "For test case " << ci;
+ // Will trigger GC
+ cm->SetCookie(GURL("http://newdomain.com"), "b=2");
+ EXPECT_EQ(test_case->expected_cookies_after_set[recent_scheme],
+ static_cast<int>((cm->GetAllCookies().size())))
+ << "For test case (" << ci << ", " << recent_scheme << ")";
+ }
+ }
+}
+
+} // namespace
