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

Unified Diff: net/base/cookie_monster.cc

Issue 8289028: Revert 105639 - The change list splits loading of cookies from DB by the domain key(eTLD+1). (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src/
Patch Set: Created 9 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/base/cookie_monster.h ('k') | net/base/cookie_monster_perftest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/base/cookie_monster.cc
===================================================================
--- net/base/cookie_monster.cc (revision 105639)
+++ net/base/cookie_monster.cc (working copy)
@@ -49,7 +49,6 @@
#include "base/basictypes.h"
#include "base/bind.h"
-#include "base/callback.h"
#include "base/format_macros.h"
#include "base/logging.h"
#include "base/memory/scoped_ptr.h"
@@ -68,26 +67,6 @@
using base::TimeDelta;
using base::TimeTicks;
-// In steady state, most cookie requests can be satisfied by the in memory
-// cookie monster store. However, if a request comes in during the initial
-// cookie load, it must be delayed until that load completes. That is done by
-// queueing it on CookieMonster::queue_ and running it when notification of
-// cookie load completion is received via CookieMonster::OnLoaded. This callback
-// is passed to the persistent store from CookieMonster::InitStore(), which is
-// called on the first operation invoked on the CookieMonster.
-//
-// On the browser critical paths (e.g. for loading initial web pages in a
-// session restore) it may take too long to wait for the full load. If a cookie
-// request is for a specific URL, DoCookieTaskForURL is called, which triggers a
-// priority load if the key is not loaded yet by calling PersistentCookieStore
-// :: LoadCookiesForKey. The request is queued in CookieMonster::tasks_queued
-// and executed upon receiving notification of key load completion via
-// CookieMonster::OnKeyLoaded(). If multiple requests for the same eTLD+1 are
-// received before key load completion, only the first request calls
-// PersistentCookieStore::LoadCookiesForKey, all subsequent requests are queued
-// in CookieMonster::tasks_queued and executed upon receiving notification of
-// key load completion triggered by the first request for the same eTLD+1.
-
static const int kMinutesInTenYears = 10 * 365 * 24 * 60;
namespace net {
@@ -1014,7 +993,7 @@
expiration_time, secure, http_only,
callback);
- DoCookieTaskForURL(task, url);
+ DoCookieTask(task);
}
void CookieMonster::GetAllCookiesAsync(const GetCookieListCallback& callback) {
@@ -1032,7 +1011,7 @@
scoped_refptr<GetAllCookiesForURLWithOptionsTask> task =
new GetAllCookiesForURLWithOptionsTask(this, url, options, callback);
- DoCookieTaskForURL(task, url);
+ DoCookieTask(task);
}
void CookieMonster::GetAllCookiesForURLAsync(
@@ -1042,7 +1021,7 @@
scoped_refptr<GetAllCookiesForURLWithOptionsTask> task =
new GetAllCookiesForURLWithOptionsTask(this, url, options, callback);
- DoCookieTaskForURL(task, url);
+ DoCookieTask(task);
}
void CookieMonster::DeleteAllAsync(const DeleteCallback& callback) {
@@ -1067,7 +1046,7 @@
scoped_refptr<DeleteAllForHostTask> task =
new DeleteAllForHostTask(this, url, callback);
- DoCookieTaskForURL(task, url);
+ DoCookieTask(task);
}
void CookieMonster::DeleteCanonicalCookieAsync(
@@ -1087,7 +1066,7 @@
scoped_refptr<SetCookieWithOptionsTask> task =
new SetCookieWithOptionsTask(this, url, cookie_line, options, callback);
- DoCookieTaskForURL(task, url);
+ DoCookieTask(task);
}
void CookieMonster::GetCookiesWithOptionsAsync(
@@ -1097,7 +1076,7 @@
scoped_refptr<GetCookiesWithOptionsTask> task =
new GetCookiesWithOptionsTask(this, url, options, callback);
- DoCookieTaskForURL(task, url);
+ DoCookieTask(task);
}
void CookieMonster::GetCookiesWithInfoAsync(
@@ -1107,7 +1086,7 @@
scoped_refptr<GetCookiesWithInfoTask> task =
new GetCookiesWithInfoTask(this, url, options, callback);
- DoCookieTaskForURL(task, url);
+ DoCookieTask(task);
}
void CookieMonster::DeleteCookieAsync(const GURL& url,
@@ -1116,14 +1095,15 @@
scoped_refptr<DeleteCookieTask> task =
new DeleteCookieTask(this, url, cookie_name, callback);
- DoCookieTaskForURL(task, url);
+ DoCookieTask(task);
}
void CookieMonster::DoCookieTask(
const scoped_refptr<CookieMonsterTask>& task_item) {
+ InitIfNecessary();
+
{
base::AutoLock autolock(lock_);
- InitIfNecessary();
if (!loaded_) {
queue_.push(task_item);
return;
@@ -1133,34 +1113,6 @@
task_item->Run();
}
-void CookieMonster::DoCookieTaskForURL(
- const scoped_refptr<CookieMonsterTask>& task_item,
- const GURL& url) {
- {
- base::AutoLock autolock(lock_);
- InitIfNecessary();
- // If cookies for the requested domain key (eTLD+1) have been loaded from DB
- // then run the task, otherwise load from DB.
- if (!loaded_) {
- // Checks if the domain key has been loaded.
- std::string key(GetEffectiveDomain(url.scheme(), url.host()));
- if (keys_loaded_.find(key) == keys_loaded_.end()) {
- std::map<std::string, std::deque<scoped_refptr<CookieMonsterTask> > >
- ::iterator it = tasks_queued_.find(key);
- if (it == tasks_queued_.end()) {
- store_->LoadCookiesForKey(key,
- base::Bind(&CookieMonster::OnKeyLoaded, this, key));
- it = tasks_queued_.insert(std::make_pair(key,
- std::deque<scoped_refptr<CookieMonsterTask> >())).first;
- }
- it->second.push_back(task_item);
- return;
- }
- }
- }
- task_item->Run();
-}
-
bool CookieMonster::SetCookieWithDetails(
const GURL& url, const std::string& name, const std::string& value,
const std::string& domain, const std::string& path,
@@ -1518,30 +1470,6 @@
InvokeQueue();
}
-void CookieMonster::OnKeyLoaded(const std::string& key,
- const std::vector<CanonicalCookie*>& cookies) {
- // This function does its own separate locking.
- StoreLoadedCookies(cookies);
-
- std::deque<scoped_refptr<CookieMonsterTask> > tasks_queued;
- {
- base::AutoLock autolock(lock_);
- keys_loaded_.insert(key);
- std::map<std::string, std::deque<scoped_refptr<CookieMonsterTask> > >
- ::iterator it = tasks_queued_.find(key);
- if (it == tasks_queued_.end())
- return;
- it->second.swap(tasks_queued);
- tasks_queued_.erase(it);
- }
-
- while (!tasks_queued.empty()) {
- scoped_refptr<CookieMonsterTask> task = tasks_queued.front();
- task->Run();
- tasks_queued.pop_front();
- }
-}
-
void CookieMonster::StoreLoadedCookies(
const std::vector<CanonicalCookie*>& cookies) {
// Initialize the store and sync in any saved persistent cookies. We don't
@@ -1549,16 +1477,24 @@
// and sync'd.
base::AutoLock autolock(lock_);
+ // Avoid ever letting cookies with duplicate creation times into the store;
+ // 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) {
+ 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;
+ 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: "
@@ -1571,14 +1507,12 @@
delete (*it);
}
}
+ earliest_access_time_= earliest_access_time;
// After importing cookies from the PersistentCookieStore, verify that
// none of our other constraints are violated.
+ //
// In particular, the backing store might have given us duplicate cookies.
-
- // This method could be called multiple times due to priority loading, thus
- // cookies loaded in previous runs will be validated again, but this is OK
- // since they are expected to be much fewer than total DB.
EnsureCookiesMapIsValid();
}
@@ -1589,8 +1523,6 @@
base::AutoLock autolock(lock_);
if (queue_.empty()) {
loaded_ = true;
- creation_times_.clear();
- keys_loaded_.clear();
break;
}
request_task = queue_.front();
« no previous file with comments | « net/base/cookie_monster.h ('k') | net/base/cookie_monster_perftest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698