summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--chrome/common/net/cookie_monster_sqlite.cc75
-rw-r--r--chrome/common/net/cookie_monster_sqlite.h2
-rw-r--r--net/base/cookie_monster.cc51
-rw-r--r--net/base/cookie_monster.h33
-rw-r--r--net/base/cookie_monster_unittest.cc43
5 files changed, 174 insertions, 30 deletions
diff --git a/chrome/common/net/cookie_monster_sqlite.cc b/chrome/common/net/cookie_monster_sqlite.cc
index e4e4a79..11bb002 100644
--- a/chrome/common/net/cookie_monster_sqlite.cc
+++ b/chrome/common/net/cookie_monster_sqlite.cc
@@ -37,12 +37,17 @@ class SQLitePersistentCookieStore::Backend
DCHECK(num_pending_ == 0 && pending_.empty());
}
- // Batch a cookie add
+ // Batch a cookie addition.
void AddCookie(const std::string& key,
const net::CookieMonster::CanonicalCookie& cc);
- // Batch a cookie delete
+
+ // Batch a cookie access time update.
+ void UpdateCookieAccessTime(const net::CookieMonster::CanonicalCookie& cc);
+
+ // Batch a cookie deletion.
void DeleteCookie(const net::CookieMonster::CanonicalCookie& cc);
- // Commit and pending operations and close the database, must be called
+
+ // Commit any pending operations and close the database. This must be called
// before the object is destructed.
void Close();
@@ -51,6 +56,7 @@ class SQLitePersistentCookieStore::Backend
public:
typedef enum {
COOKIE_ADD,
+ COOKIE_UPDATEACCESS,
COOKIE_DELETE,
} OperationType;
@@ -97,6 +103,11 @@ void SQLitePersistentCookieStore::Backend::AddCookie(
BatchOperation(PendingOperation::COOKIE_ADD, key, cc);
}
+void SQLitePersistentCookieStore::Backend::UpdateCookieAccessTime(
+ const net::CookieMonster::CanonicalCookie& cc) {
+ BatchOperation(PendingOperation::COOKIE_UPDATEACCESS, std::string(), cc);
+}
+
void SQLitePersistentCookieStore::Backend::DeleteCookie(
const net::CookieMonster::CanonicalCookie& cc) {
BatchOperation(PendingOperation::COOKIE_DELETE, std::string(), cc);
@@ -150,13 +161,21 @@ void SQLitePersistentCookieStore::Backend::Commit() {
SQLITE_UNIQUE_STATEMENT(add_smt, *cache_,
"INSERT INTO cookies (creation_utc, host_key, name, value, path, "
- "expires_utc, secure, httponly) "
- "VALUES (?,?,?,?,?,?,?,?)");
+ "expires_utc, secure, httponly, last_access_utc) "
+ "VALUES (?,?,?,?,?,?,?,?,?)");
if (!add_smt.is_valid()) {
NOTREACHED();
return;
}
+ SQLITE_UNIQUE_STATEMENT(update_access_smt, *cache_,
+ "UPDATE cookies SET last_access_utc=? "
+ "WHERE creation_utc=?");
+ if (!update_access_smt.is_valid()) {
+ NOTREACHED();
+ return;
+ }
+
SQLITE_UNIQUE_STATEMENT(del_smt, *cache_,
"DELETE FROM cookies WHERE creation_utc=?");
if (!del_smt.is_valid()) {
@@ -181,11 +200,23 @@ void SQLitePersistentCookieStore::Backend::Commit() {
add_smt->bind_int64(5, po->cc().ExpiryDate().ToInternalValue());
add_smt->bind_int(6, po->cc().IsSecure());
add_smt->bind_int(7, po->cc().IsHttpOnly());
+ add_smt->bind_int64(8, po->cc().LastAccessDate().ToInternalValue());
if (add_smt->step() != SQLITE_DONE) {
NOTREACHED() << "Could not add a cookie to the DB.";
}
break;
+ case PendingOperation::COOKIE_UPDATEACCESS:
+ update_access_smt->reset();
+ update_access_smt->bind_int64(0,
+ po->cc().LastAccessDate().ToInternalValue());
+ update_access_smt->bind_int64(1,
+ po->cc().CreationDate().ToInternalValue());
+ if (update_access_smt->step() != SQLITE_DONE) {
+ NOTREACHED() << "Could not update cookie last access time in the DB.";
+ }
+ break;
+
case PendingOperation::COOKIE_DELETE:
del_smt->reset();
del_smt->bind_int64(0, po->cc().CreationDate().ToInternalValue());
@@ -241,8 +272,8 @@ SQLitePersistentCookieStore::~SQLitePersistentCookieStore() {
}
// Version number of the database.
-static const int kCurrentVersionNumber = 2;
-static const int kCompatibleVersionNumber = 2;
+static const int kCurrentVersionNumber = 3;
+static const int kCompatibleVersionNumber = 3;
namespace {
@@ -258,7 +289,8 @@ bool InitTable(sqlite3* db) {
// We only store persistent, so we know it expires
"expires_utc INTEGER NOT NULL,"
"secure INTEGER NOT NULL,"
- "httponly INTEGER NOT NULL)",
+ "httponly INTEGER NOT NULL,"
+ "last_access_utc INTEGER NOT NULL)",
NULL, NULL, NULL) != SQLITE_OK)
return false;
}
@@ -292,7 +324,7 @@ bool SQLitePersistentCookieStore::Load(
SQLStatement smt;
if (smt.prepare(db,
"SELECT creation_utc, host_key, name, value, path, expires_utc, secure, "
- "httponly FROM cookies") != SQLITE_OK) {
+ "httponly, last_access_utc FROM cookies") != SQLITE_OK) {
NOTREACHED() << "select statement prep failed";
sqlite3_close(db);
return false;
@@ -319,6 +351,7 @@ bool SQLitePersistentCookieStore::Load(
smt.column_int(6) != 0, // secure
smt.column_int(7) != 0, // httponly
Time::FromInternalValue(smt.column_int64(0)), // creation_utc
+ Time::FromInternalValue(smt.column_int64(8)), // last_access_utc
true, // has_expires
Time::FromInternalValue(smt.column_int64(5)))); // expires_utc
// Memory allocation failed.
@@ -350,6 +383,24 @@ bool SQLitePersistentCookieStore::EnsureDatabaseVersion(sqlite3* db) {
}
int cur_version = meta_table_.GetVersionNumber();
+ if (cur_version == 2) {
+ SQLTransaction transaction(db);
+ transaction.Begin();
+ if ((sqlite3_exec(db,
+ "ALTER TABLE cookies ADD COLUMN last_access_utc "
+ "INTEGER DEFAULT 0", NULL, NULL, NULL) != SQLITE_OK) ||
+ (sqlite3_exec(db,
+ "UPDATE cookies SET last_access_utc = creation_utc",
+ NULL, NULL, NULL) != SQLITE_OK)) {
+ LOG(WARNING) << "Unable to update cookie database to version 3.";
+ return false;
+ }
+ ++cur_version;
+ meta_table_.SetVersionNumber(cur_version);
+ meta_table_.SetCompatibleVersionNumber(
+ std::min(cur_version, kCompatibleVersionNumber));
+ transaction.Commit();
+ }
// Put future migration cases here.
@@ -368,6 +419,12 @@ void SQLitePersistentCookieStore::AddCookie(
backend_->AddCookie(key, cc);
}
+void SQLitePersistentCookieStore::UpdateCookieAccessTime(
+ const net::CookieMonster::CanonicalCookie& cc) {
+ if (backend_.get())
+ backend_->UpdateCookieAccessTime(cc);
+}
+
void SQLitePersistentCookieStore::DeleteCookie(
const net::CookieMonster::CanonicalCookie& cc) {
if (backend_.get())
diff --git a/chrome/common/net/cookie_monster_sqlite.h b/chrome/common/net/cookie_monster_sqlite.h
index 7607915..14da41c 100644
--- a/chrome/common/net/cookie_monster_sqlite.h
+++ b/chrome/common/net/cookie_monster_sqlite.h
@@ -28,6 +28,8 @@ class SQLitePersistentCookieStore
virtual void AddCookie(const std::string&,
const net::CookieMonster::CanonicalCookie&);
+ virtual void UpdateCookieAccessTime(
+ const net::CookieMonster::CanonicalCookie&);
virtual void DeleteCookie(const net::CookieMonster::CanonicalCookie&);
private:
diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc
index 7a56831..987bfb8 100644
--- a/net/base/cookie_monster.cc
+++ b/net/base/cookie_monster.cc
@@ -68,6 +68,10 @@ using base::TimeDelta;
namespace net {
+// Default minimum delay after updating a cookie's LastAccessDate before we
+// will update it again.
+static const int kDefaultAccessUpdateThresholdSeconds = 60;
+
// static
bool CookieMonster::enable_file_scheme_ = false;
@@ -78,12 +82,16 @@ void CookieMonster::EnableFileScheme() {
CookieMonster::CookieMonster()
: initialized_(false),
- store_(NULL) {
+ store_(NULL),
+ last_access_threshold_(
+ TimeDelta::FromSeconds(kDefaultAccessUpdateThresholdSeconds)) {
}
CookieMonster::CookieMonster(PersistentCookieStore* store)
: initialized_(false),
- store_(store) {
+ store_(store),
+ last_access_threshold_(
+ TimeDelta::FromSeconds(kDefaultAccessUpdateThresholdSeconds)) {
}
CookieMonster::~CookieMonster() {
@@ -398,8 +406,8 @@ bool CookieMonster::SetCookieWithCreationTime(const GURL& url,
cc.reset(new CanonicalCookie(pc.Name(), pc.Value(), cookie_path,
pc.IsSecure(), pc.IsHttpOnly(),
- creation_time, !cookie_expires.is_null(),
- cookie_expires));
+ creation_time, creation_time,
+ !cookie_expires.is_null(), cookie_expires));
if (!cc.get()) {
COOKIE_DLOG(WARNING) << "Failed to allocate CanonicalCookie";
@@ -440,6 +448,20 @@ void CookieMonster::InternalInsertCookie(const std::string& key,
cookies_.insert(CookieMap::value_type(key, cc));
}
+void CookieMonster::InternalUpdateCookieAccessTime(CanonicalCookie* cc) {
+ // Based off the Mozilla code. When a cookie has been accessed recently,
+ // don't bother updating its access time again. This reduces the number of
+ // updates we do during pageload, which in turn reduces the chance our storage
+ // backend will hit its batch thresholds and be forced to update.
+ const Time current = Time::Now();
+ if ((current - cc->LastAccessDate()) < last_access_threshold_)
+ return;
+
+ cc->SetLastAccessDate(current);
+ if (cc->IsPersistent() && store_)
+ store_->UpdateCookieAccessTime(*cc);
+}
+
void CookieMonster::InternalDeleteCookie(CookieMap::iterator it,
bool sync_to_store) {
CanonicalCookie* cc = it->second;
@@ -507,10 +529,15 @@ int CookieMonster::GarbageCollect(const Time& current,
return num_deleted;
}
-// TODO we should be sorting by last access time, however, right now
-// we're not saving an access time, so we're sorting by creation time.
-static bool OldestCookieSorter(const CookieMonster::CookieMap::iterator& it1,
- const CookieMonster::CookieMap::iterator& it2) {
+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();
}
@@ -522,7 +549,7 @@ int CookieMonster::GarbageCollectRange(const Time& current,
std::vector<CookieMap::iterator> cookie_its;
int num_deleted = GarbageCollectExpired(current, itpair, &cookie_its);
- // If the range still has too many cookies, delete the oldest.
+ // If the range still has too many cookies, delete the least recently used.
if (cookie_its.size() > num_max) {
COOKIE_DLOG(INFO) << "GarbageCollectRange() Deep Garbage Collect.";
// Purge down to (|num_max| - |num_purge|) total cookies.
@@ -530,7 +557,7 @@ int CookieMonster::GarbageCollectRange(const Time& current,
num_purge += cookie_its.size() - num_max;
std::partial_sort(cookie_its.begin(), cookie_its.begin() + num_purge,
- cookie_its.end(), OldestCookieSorter);
+ cookie_its.end(), LRUCookieSorter);
for (size_t i = 0; i < num_purge; ++i)
InternalDeleteCookie(cookie_its[i], true);
@@ -763,7 +790,9 @@ void CookieMonster::FindCookiesForKey(
if (!cc->IsOnPath(url.path()))
continue;
- // Congratulations Charlie, you passed the test!
+ // Add this cookie to the set of matching cookies. Since we're reading the
+ // cookie, update its last access time.
+ InternalUpdateCookieAccessTime(cc);
cookies->push_back(cc);
}
}
diff --git a/net/base/cookie_monster.h b/net/base/cookie_monster.h
index 6db774a..68d8c3b 100644
--- a/net/base/cookie_monster.h
+++ b/net/base/cookie_monster.h
@@ -30,9 +30,6 @@ namespace net {
//
// TODO(deanm) Implement CookieMonster, the cookie database.
// - Verify that our domain enforcement and non-dotted handling is correct
-// - Currently garbage collection is done on oldest CreationUTC, Mozilla
-// purges cookies on last access time, which would require adding and
-// keeping track of access times on a CanonicalCookie
class CookieMonster {
public:
class ParsedCookie;
@@ -68,6 +65,15 @@ class CookieMonster {
// existence.
CookieMonster(PersistentCookieStore* store);
+#ifdef UNIT_TEST
+ CookieMonster::CookieMonster(int last_access_threshold_seconds)
+ : initialized_(false),
+ store_(NULL),
+ last_access_threshold_(
+ base::TimeDelta::FromSeconds(last_access_threshold_seconds)) {
+ }
+#endif
+
~CookieMonster();
// Parse the string with the cookie time (very forgivingly).
@@ -90,7 +96,8 @@ class CookieMonster {
// It will _not_ return httponly cookies, see GetCookiesWithOptions
std::string GetCookies(const GURL& url);
std::string GetCookiesWithOptions(const GURL& url, CookieOptions options);
- // Returns all the cookies, for use in management UI, etc.
+ // Returns all the cookies, for use in management UI, etc. This does not mark
+ // the cookies as having been accessed.
CookieList GetAllCookies();
// Delete all of the cookies.
@@ -149,6 +156,8 @@ class CookieMonster {
CanonicalCookie* cc,
bool sync_to_store);
+ void InternalUpdateCookieAccessTime(CanonicalCookie* cc);
+
void InternalDeleteCookie(CookieMap::iterator it, bool sync_to_store);
// If the number of cookies for host |key|, or globally, are over preset
@@ -161,7 +170,8 @@ class CookieMonster {
// Deletes all expired cookies in |itpair|;
// then, if the number of remaining cookies is greater than |num_max|,
- // collects the oldest cookies until (|num_max| - |num_purge|) cookies remain.
+ // collects the least recently accessed cookies until
+ // (|num_max| - |num_purge|) cookies remain.
//
// Returns the number of cookies deleted.
int GarbageCollectRange(const base::Time& current,
@@ -191,6 +201,10 @@ class CookieMonster {
base::Time CurrentTime();
base::Time last_time_seen_;
+ // Minimum delay after updating a cookie's LastAccessDate before we will
+ // update it again.
+ const base::TimeDelta last_access_threshold_;
+
// Lock for thread-safety
Lock lock_;
@@ -261,12 +275,14 @@ class CookieMonster::CanonicalCookie {
bool secure,
bool httponly,
const base::Time& creation,
+ const base::Time& last_access,
bool has_expires,
const base::Time& expires)
: name_(name),
value_(value),
path_(path),
creation_date_(creation),
+ last_access_date_(last_access),
expiry_date_(expires),
has_expires_(has_expires),
secure_(secure),
@@ -288,6 +304,7 @@ class CookieMonster::CanonicalCookie {
const std::string& Value() const { return value_; }
const std::string& Path() const { return path_; }
const base::Time& CreationDate() const { return creation_date_; }
+ const base::Time& LastAccessDate() const { return last_access_date_; }
bool DoesExpire() const { return has_expires_; }
bool IsPersistent() const { return DoesExpire(); }
const base::Time& ExpiryDate() const { return expiry_date_; }
@@ -306,6 +323,10 @@ class CookieMonster::CanonicalCookie {
return name_ == ecc.Name() && path_ == ecc.Path();
}
+ void SetLastAccessDate(const base::Time& date) {
+ last_access_date_ = date;
+ }
+
bool IsOnPath(const std::string& url_path) const;
std::string DebugString() const;
@@ -314,6 +335,7 @@ class CookieMonster::CanonicalCookie {
std::string value_;
std::string path_;
base::Time creation_date_;
+ base::Time last_access_date_;
base::Time expiry_date_;
bool has_expires_;
bool secure_;
@@ -329,6 +351,7 @@ class CookieMonster::PersistentCookieStore {
virtual bool Load(std::vector<CookieMonster::KeyedCanonicalCookie>*) = 0;
virtual void AddCookie(const std::string&, const CanonicalCookie&) = 0;
+ virtual void UpdateCookieAccessTime(const CanonicalCookie&) = 0;
virtual void DeleteCookie(const CanonicalCookie&) = 0;
protected:
diff --git a/net/base/cookie_monster_unittest.cc b/net/base/cookie_monster_unittest.cc
index e9a5b00..7b6f0ad 100644
--- a/net/base/cookie_monster_unittest.cc
+++ b/net/base/cookie_monster_unittest.cc
@@ -718,6 +718,31 @@ TEST(CookieMonsterTest, TestSecure) {
EXPECT_EQ("D=E; A=B", cm.GetCookies(url_google_secure));
}
+static Time GetFirstCookieAccessDate(net::CookieMonster* cm) {
+ const net::CookieMonster::CookieList all_cookies(cm->GetAllCookies());
+ return all_cookies.front().second.LastAccessDate();
+}
+
+static const int kLastAccessThresholdSeconds = 1;
+
+TEST(CookieMonsterTest, TestLastAccess) {
+ GURL url_google(kUrlGoogle);
+ net::CookieMonster cm(kLastAccessThresholdSeconds);
+
+ EXPECT_TRUE(cm.SetCookie(url_google, "A=B"));
+ const Time last_access_date(GetFirstCookieAccessDate(&cm));
+
+ // Reading the cookie again immediately shouldn't update the access date,
+ // since we're inside the threshold.
+ EXPECT_EQ("A=B", cm.GetCookies(url_google));
+ EXPECT_TRUE(last_access_date == GetFirstCookieAccessDate(&cm));
+
+ // Reading after a short wait should update the access date.
+ Sleep(1500);
+ EXPECT_EQ("A=B", cm.GetCookies(url_google));
+ EXPECT_FALSE(last_access_date == GetFirstCookieAccessDate(&cm));
+}
+
static int CountInString(const std::string& str, char c) {
int count = 0;
for (std::string::const_iterator it = str.begin();
@@ -744,23 +769,31 @@ TEST(CookieMonsterTest, TestHostGarbageCollection) {
}
TEST(CookieMonsterTest, TestTotalGarbageCollection) {
- net::CookieMonster cm;
+ net::CookieMonster cm(kLastAccessThresholdSeconds);
// 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 < 2000; ++i) {
GURL url(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)) {
+ Sleep(1500); // Ensure the timestamps will be different enough to update.
+ EXPECT_EQ("a=b", cm.GetCookies(sticky_cookie));
+ }
}
// Check that cookies that still exist.
for (int i = 0; i < 2000; ++i) {
GURL url(StringPrintf("http://a%04d.izzle", i));
- if (i < 900) {
- // Cookies should have gotten purged.
- EXPECT_TRUE(cm.GetCookies(url).empty());
- } else if (i > 1100) {
+ if ((i == 0) || (i > 1101)) {
// Cookies should still be around.
EXPECT_FALSE(cm.GetCookies(url).empty());
+ } else if (i < 901) {
+ // Cookies should have gotten purged.
+ EXPECT_TRUE(cm.GetCookies(url).empty());
}
}
}
generated by cgit v1.1 at 2025-01-19 16:58:48 +0000