diff options
author | erikwright@chromium.org <erikwright@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-08 18:03:31 +0000 |
---|---|---|
committer | erikwright@chromium.org <erikwright@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-08 18:03:31 +0000 |
commit | 5e9791ee25e21ea59ffbde31d9330b4d3ccb4b85 (patch) | |
tree | 08280972fffb91b4729a4f2d3d61db84d9f0630b /net | |
parent | 73455b1bf7535666dd7d0655b398934f5ea9e953 (diff) | |
download | chromium_src-5e9791ee25e21ea59ffbde31d9330b4d3ccb4b85.zip chromium_src-5e9791ee25e21ea59ffbde31d9330b4d3ccb4b85.tar.gz chromium_src-5e9791ee25e21ea59ffbde31d9330b4d3ccb4b85.tar.bz2 |
Finalize a CL originally by departed intern ycxiao@ that detaches the loading of cookies from the IO thread.
They are now loaded on the DB thread. Cookie operations received in the meantime are queued and executed, on the IO thread, in the order they were received, when loading completes.
A few straggler clients are updated to use the asynchronous CookieStore/CookieMonster API as part of this CL, as the synchronous API is removed.
BUG=68657
TEST=net_unittests / DeferredCookieTaskTest.* and CookieMonsterTest.*
Review URL: http://codereview.chromium.org/7833042
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@100188 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
-rw-r--r-- | net/base/cookie_monster.cc | 706 | ||||
-rw-r--r-- | net/base/cookie_monster.h | 163 | ||||
-rw-r--r-- | net/base/cookie_monster_perftest.cc | 201 | ||||
-rw-r--r-- | net/base/cookie_monster_store_test.cc | 16 | ||||
-rw-r--r-- | net/base/cookie_monster_store_test.h | 26 | ||||
-rw-r--r-- | net/base/cookie_monster_unittest.cc | 952 | ||||
-rw-r--r-- | net/base/cookie_store.cc | 26 | ||||
-rw-r--r-- | net/base/cookie_store.h | 52 | ||||
-rw-r--r-- | net/base/cookie_store_test_helpers.cc | 10 | ||||
-rw-r--r-- | net/base/cookie_store_test_helpers.h | 4 | ||||
-rw-r--r-- | net/url_request/url_request_http_job.cc | 10 | ||||
-rw-r--r-- | net/url_request/url_request_http_job.h | 4 | ||||
-rw-r--r-- | net/url_request/url_request_unittest.cc | 17 |
13 files changed, 1608 insertions, 579 deletions
diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc index 5012bc4..6afe739 100644 --- a/net/base/cookie_monster.cc +++ b/net/base/cookie_monster.cc @@ -48,10 +48,12 @@ #include <set> #include "base/basictypes.h" +#include "base/bind.h" #include "base/format_macros.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/message_loop.h" +#include "base/message_loop_proxy.h" #include "base/metrics/histogram.h" #include "base/string_tokenizer.h" #include "base/string_util.h" @@ -420,6 +422,7 @@ bool CookieMonster::enable_file_scheme_ = false; CookieMonster::CookieMonster(PersistentCookieStore* store, Delegate* delegate) : initialized_(false), + loaded_(false), expiry_and_key_scheme_(expiry_and_key_default_), store_(store), last_access_threshold_( @@ -435,6 +438,7 @@ CookieMonster::CookieMonster(PersistentCookieStore* store, Delegate* delegate, int last_access_threshold_milliseconds) : initialized_(false), + loaded_(false), expiry_and_key_scheme_(expiry_and_key_default_), store_(store), last_access_threshold_(base::TimeDelta::FromMilliseconds( @@ -568,6 +572,547 @@ bool CookieMonster::DomainIsHostOnly(const std::string& domain_string) { return (domain_string.empty() || domain_string[0] != '.'); } +// Task classes for queueing the coming request. + +class CookieMonster::CookieMonsterTask + : public base::RefCountedThreadSafe<CookieMonsterTask> { + public: + // Runs the task and invokes the client callback on the thread that + // originally constructed the task. + virtual void Run() = 0; + + protected: + explicit CookieMonsterTask(CookieMonster* cookie_monster); + virtual ~CookieMonsterTask(); + + // Invokes the callback immediately, if the current thread is the one + // that originated the task, or queues the callback for execution on the + // appropriate thread. Maintains a reference to this CookieMonsterTask + // instance until the callback completes. + void InvokeCallback(base::Closure callback); + + CookieMonster* cookie_monster() { + return cookie_monster_; + } + + friend class base::RefCountedThreadSafe<CookieMonsterTask>; + + private: + CookieMonster* cookie_monster_; + scoped_refptr<base::MessageLoopProxy> thread_; + + DISALLOW_COPY_AND_ASSIGN(CookieMonsterTask); +}; + +CookieMonster::CookieMonsterTask::CookieMonsterTask( + CookieMonster* cookie_monster) + : cookie_monster_(cookie_monster), + thread_(base::MessageLoopProxy::current()) { } + +CookieMonster::CookieMonsterTask::~CookieMonsterTask() { } + +// Unfortunately, one cannot re-bind a Callback with parameters into a closure. +// Therefore, the closure passed to InvokeCallback is a clumsy binding of +// Callback::Run on a wrapped Callback instance. Since Callback is not +// reference counted, we bind to an instance that is a member of the +// CookieMonsterTask subclass. Then, we cannot simply post the callback to a +// message loop because the underlying instance may be destroyed (along with the +// CookieMonsterTask instance) in the interim. Therefore, we post a callback +// bound to the CookieMonsterTask, which *is* reference counted (thus preventing +// destruction of the original callback), and which invokes the closure (which +// invokes the original callback with the returned data). +void CookieMonster::CookieMonsterTask::InvokeCallback(base::Closure callback) { + if (thread_->BelongsToCurrentThread()) { + callback.Run(); + } else { + thread_->PostTask(FROM_HERE, base::Bind( + &CookieMonster::CookieMonsterTask::InvokeCallback, this, callback)); + } +} + +// Task class for SetCookieWithDetails call. +class CookieMonster::SetCookieWithDetailsTask + : public CookieMonster::CookieMonsterTask { + public: + SetCookieWithDetailsTask( + CookieMonster* cookie_monster, + const GURL& url, const std::string& name, const std::string& value, + const std::string& domain, const std::string& path, + const base::Time& expiration_time, bool secure, bool http_only, + const CookieMonster::SetCookiesCallback& callback) + : CookieMonsterTask(cookie_monster), + url_(url), + name_(name), + value_(value), + domain_(domain), + path_(path), + expiration_time_(expiration_time), + secure_(secure), + http_only_(http_only), + callback_(callback) { } + + virtual void Run() OVERRIDE; + + private: + GURL url_; + std::string name_; + std::string value_; + std::string domain_; + std::string path_; + base::Time expiration_time_; + bool secure_; + bool http_only_; + CookieMonster::SetCookiesCallback callback_; + + DISALLOW_COPY_AND_ASSIGN(SetCookieWithDetailsTask); +}; + +void CookieMonster::SetCookieWithDetailsTask::Run() { + bool success = this->cookie_monster()-> + SetCookieWithDetails(url_, name_, value_, domain_, path_, + expiration_time_, secure_, http_only_); + if (!callback_.is_null()) { + this->InvokeCallback(base::Bind(&CookieMonster::SetCookiesCallback::Run, + base::Unretained(&callback_), success)); + } +} + +// Task class for GetAllCookies call. +class CookieMonster::GetAllCookiesTask + : public CookieMonster::CookieMonsterTask { + public: + GetAllCookiesTask(CookieMonster* cookie_monster, + const CookieMonster::GetCookieListCallback& callback) + : CookieMonsterTask(cookie_monster), + callback_(callback) { } + + virtual void Run() OVERRIDE; + + private: + CookieMonster::GetCookieListCallback callback_; + + DISALLOW_COPY_AND_ASSIGN(GetAllCookiesTask); +}; + +void CookieMonster::GetAllCookiesTask::Run() { + if (!callback_.is_null()) { + CookieList cookies = this->cookie_monster()->GetAllCookies(); + this->InvokeCallback(base::Bind(&CookieMonster::GetCookieListCallback::Run, + base::Unretained(&callback_), cookies)); + } +} + +// Task class for GetAllCookiesForURLWithOptions call. +class CookieMonster::GetAllCookiesForURLWithOptionsTask + : public CookieMonster::CookieMonsterTask { + public: + GetAllCookiesForURLWithOptionsTask( + CookieMonster* cookie_monster, + const GURL& url, + const CookieOptions& options, + const CookieMonster::GetCookieListCallback& callback) + : CookieMonsterTask(cookie_monster), + url_(url), + options_(options), + callback_(callback) { } + + virtual void Run() OVERRIDE; + + private: + GURL url_; + CookieOptions options_; + CookieMonster::GetCookieListCallback callback_; + + DISALLOW_COPY_AND_ASSIGN(GetAllCookiesForURLWithOptionsTask); +}; + +void CookieMonster::GetAllCookiesForURLWithOptionsTask::Run() { + if (!callback_.is_null()) { + CookieList cookies = this->cookie_monster()-> + GetAllCookiesForURLWithOptions(url_, options_); + this->InvokeCallback(base::Bind(&CookieMonster::GetCookieListCallback::Run, + base::Unretained(&callback_), cookies)); + } +} + +// Task class for DeleteAll call. +class CookieMonster::DeleteAllTask : public CookieMonster::CookieMonsterTask { + public: + DeleteAllTask(CookieMonster* cookie_monster, + const CookieMonster::DeleteCallback& callback) + : CookieMonsterTask(cookie_monster), + callback_(callback) { } + + virtual void Run() OVERRIDE; + + private: + CookieMonster::DeleteCallback callback_; + + DISALLOW_COPY_AND_ASSIGN(DeleteAllTask); +}; + +void CookieMonster::DeleteAllTask::Run() { + int num_deleted = this->cookie_monster()->DeleteAll(true); + if (!callback_.is_null()) { + this->InvokeCallback(base::Bind(&CookieMonster::DeleteCallback::Run, + base::Unretained(&callback_), num_deleted)); + } +} + +// Task class for DeleteAllCreatedBetween call. +class CookieMonster::DeleteAllCreatedBetweenTask + : public CookieMonster::CookieMonsterTask { + public: + DeleteAllCreatedBetweenTask( + CookieMonster* cookie_monster, + const Time& delete_begin, + const Time& delete_end, + const CookieMonster::DeleteCallback& callback) + : CookieMonsterTask(cookie_monster), + delete_begin_(delete_begin), + delete_end_(delete_end), + callback_(callback) { } + + virtual void Run() OVERRIDE; + + private: + Time delete_begin_; + Time delete_end_; + CookieMonster::DeleteCallback callback_; + + DISALLOW_COPY_AND_ASSIGN(DeleteAllCreatedBetweenTask); +}; + +void CookieMonster::DeleteAllCreatedBetweenTask::Run() { + int num_deleted = this->cookie_monster()-> + DeleteAllCreatedBetween(delete_begin_, delete_end_); + if (!callback_.is_null()) { + this->InvokeCallback(base::Bind(&CookieMonster::DeleteCallback::Run, + base::Unretained(&callback_), num_deleted)); + } +} + +// Task class for DeleteAllForHost call. +class CookieMonster::DeleteAllForHostTask + : public CookieMonster::CookieMonsterTask { + public: + DeleteAllForHostTask(CookieMonster* cookie_monster, + const GURL& url, + const CookieMonster::DeleteCallback& callback) + : CookieMonsterTask(cookie_monster), + url_(url), + callback_(callback) { } + + virtual void Run() OVERRIDE; + + private: + GURL url_; + CookieMonster::DeleteCallback callback_; + + DISALLOW_COPY_AND_ASSIGN(DeleteAllForHostTask); +}; + +void CookieMonster::DeleteAllForHostTask::Run() { + int num_deleted = this->cookie_monster()->DeleteAllForHost(url_); + if (!callback_.is_null()) { + this->InvokeCallback(base::Bind(&CookieMonster::DeleteCallback::Run, + base::Unretained(&callback_), num_deleted)); + } +} + +// Task class for DeleteCanonicalCookie call. +class CookieMonster::DeleteCanonicalCookieTask + : public CookieMonster::CookieMonsterTask { + public: + DeleteCanonicalCookieTask( + CookieMonster* cookie_monster, + const CookieMonster::CanonicalCookie& cookie, + const CookieMonster::DeleteCookieCallback& callback) + : CookieMonsterTask(cookie_monster), + cookie_(cookie), + callback_(callback) { } + + virtual void Run() OVERRIDE; + + private: + CookieMonster::CanonicalCookie cookie_; + CookieMonster::DeleteCookieCallback callback_; + + DISALLOW_COPY_AND_ASSIGN(DeleteCanonicalCookieTask); +}; + +void CookieMonster::DeleteCanonicalCookieTask::Run() { + bool result = this->cookie_monster()->DeleteCanonicalCookie(cookie_); + if (!callback_.is_null()) { + this->InvokeCallback(base::Bind(&CookieMonster::DeleteCookieCallback::Run, + base::Unretained(&callback_), result)); + } +} + +// Task class for SetCookieWithOptions call. +class CookieMonster::SetCookieWithOptionsTask + : public CookieMonster::CookieMonsterTask { + public: + SetCookieWithOptionsTask(CookieMonster* cookie_monster, + const GURL& url, + const std::string& cookie_line, + const CookieOptions& options, + const CookieMonster::SetCookiesCallback& callback) + : CookieMonsterTask(cookie_monster), + url_(url), + cookie_line_(cookie_line), + options_(options), + callback_(callback) { } + + virtual void Run() OVERRIDE; + + private: + GURL url_; + std::string cookie_line_; + CookieOptions options_; + CookieMonster::SetCookiesCallback callback_; + + DISALLOW_COPY_AND_ASSIGN(SetCookieWithOptionsTask); +}; + +void CookieMonster::SetCookieWithOptionsTask::Run() { + bool result = this->cookie_monster()-> + SetCookieWithOptions(url_, cookie_line_, options_); + if (!callback_.is_null()) { + this->InvokeCallback(base::Bind(&CookieMonster::SetCookiesCallback::Run, + base::Unretained(&callback_), result)); + } +} + +// Task class for GetCookiesWithOptions call. +class CookieMonster::GetCookiesWithOptionsTask + : public CookieMonster::CookieMonsterTask { + public: + GetCookiesWithOptionsTask(CookieMonster* cookie_monster, + GURL url, + const CookieOptions& options, + const CookieMonster::GetCookiesCallback& callback) + : CookieMonsterTask(cookie_monster), + url_(url), + options_(options), + callback_(callback) { } + + virtual void Run() OVERRIDE; + + private: + GURL url_; + CookieOptions options_; + CookieMonster::GetCookiesCallback callback_; + + DISALLOW_COPY_AND_ASSIGN(GetCookiesWithOptionsTask); +}; + +void CookieMonster::GetCookiesWithOptionsTask::Run() { + std::string cookie = this->cookie_monster()-> + GetCookiesWithOptions(url_, options_); + if (!callback_.is_null()) { + this->InvokeCallback(base::Bind(&CookieMonster::GetCookiesCallback::Run, + base::Unretained(&callback_), cookie)); + } +} + +// Task class for GetCookiesWithInfo call. +class CookieMonster::GetCookiesWithInfoTask + : public CookieMonster::CookieMonsterTask { + public: + GetCookiesWithInfoTask(CookieMonster* cookie_monster, + const GURL& url, + const CookieOptions& options, + const CookieMonster::GetCookieInfoCallback& callback) + : CookieMonsterTask(cookie_monster), + url_(url), + options_(options), + callback_(callback) { } + + virtual void Run() OVERRIDE; + + private: + GURL url_; + CookieOptions options_; + CookieMonster::GetCookieInfoCallback callback_; + + DISALLOW_COPY_AND_ASSIGN(GetCookiesWithInfoTask); +}; + +void CookieMonster::GetCookiesWithInfoTask::Run() { + if (!callback_.is_null()) { + std::string cookie_line; + std::vector<CookieMonster::CookieInfo> cookie_infos; + this->cookie_monster()-> + GetCookiesWithInfo(url_, options_, &cookie_line, &cookie_infos); + this->InvokeCallback(base::Bind(&CookieMonster::GetCookieInfoCallback::Run, + base::Unretained(&callback_), + cookie_line, cookie_infos)); + } +} + +// Task class for DeleteCookie call. +class CookieMonster::DeleteCookieTask + : public CookieMonster::CookieMonsterTask { + public: + DeleteCookieTask(CookieMonster* cookie_monster, + GURL url, + const std::string& cookie_name, + const base::Closure& callback) + : CookieMonsterTask(cookie_monster), + url_(url), + cookie_name_(cookie_name), + callback_(callback) { } + + virtual void Run() OVERRIDE; + + private: + GURL url_; + std::string cookie_name_; + base::Closure callback_; + + DISALLOW_COPY_AND_ASSIGN(DeleteCookieTask); +}; + +void CookieMonster::DeleteCookieTask::Run() { + this->cookie_monster()->DeleteCookie(url_, cookie_name_); + if (!callback_.is_null()) { + this->InvokeCallback(callback_); + } +} + +// Asynchronous CookieMonster API + +void CookieMonster::SetCookieWithDetailsAsync( + const GURL& url, const std::string& name, const std::string& value, + const std::string& domain, const std::string& path, + const base::Time& expiration_time, bool secure, bool http_only, + const SetCookiesCallback& callback) { + scoped_refptr<SetCookieWithDetailsTask> task = + new SetCookieWithDetailsTask(this, url, name, value, domain, path, + expiration_time, secure, http_only, + callback); + + DoCookieTask(task); +} + +void CookieMonster::GetAllCookiesAsync(const GetCookieListCallback& callback) { + scoped_refptr<GetAllCookiesTask> task = + new GetAllCookiesTask(this, callback); + + DoCookieTask(task); +} + + +void CookieMonster::GetAllCookiesForURLWithOptionsAsync( + const GURL& url, + const CookieOptions& options, + const GetCookieListCallback& callback) { + scoped_refptr<GetAllCookiesForURLWithOptionsTask> task = + new GetAllCookiesForURLWithOptionsTask(this, url, options, callback); + + DoCookieTask(task); +} + +void CookieMonster::GetAllCookiesForURLAsync( + const GURL& url, const GetCookieListCallback& callback) { + CookieOptions options; + options.set_include_httponly(); + scoped_refptr<GetAllCookiesForURLWithOptionsTask> task = + new GetAllCookiesForURLWithOptionsTask(this, url, options, callback); + + DoCookieTask(task); +} + +void CookieMonster::DeleteAllAsync(const DeleteCallback& callback) { + scoped_refptr<DeleteAllTask> task = + new DeleteAllTask(this, callback); + + DoCookieTask(task); +} + +void CookieMonster::DeleteAllCreatedBetweenAsync( + const Time& delete_begin, const Time& delete_end, + const DeleteCallback& callback) { + scoped_refptr<DeleteAllCreatedBetweenTask> task = + new DeleteAllCreatedBetweenTask(this, delete_begin, delete_end, + callback); + + DoCookieTask(task); +} + +void CookieMonster::DeleteAllForHostAsync( + const GURL& url, const DeleteCallback& callback) { + scoped_refptr<DeleteAllForHostTask> task = + new DeleteAllForHostTask(this, url, callback); + + DoCookieTask(task); +} + +void CookieMonster::DeleteCanonicalCookieAsync( + const CanonicalCookie& cookie, + const DeleteCookieCallback& callback) { + scoped_refptr<DeleteCanonicalCookieTask> task = + new DeleteCanonicalCookieTask(this, cookie, callback); + + DoCookieTask(task); +} + +void CookieMonster::SetCookieWithOptionsAsync( + const GURL& url, + const std::string& cookie_line, + const CookieOptions& options, + const SetCookiesCallback& callback) { + scoped_refptr<SetCookieWithOptionsTask> task = + new SetCookieWithOptionsTask(this, url, cookie_line, options, callback); + + DoCookieTask(task); +} + +void CookieMonster::GetCookiesWithOptionsAsync( + const GURL& url, + const CookieOptions& options, + const GetCookiesCallback& callback) { + scoped_refptr<GetCookiesWithOptionsTask> task = + new GetCookiesWithOptionsTask(this, url, options, callback); + + DoCookieTask(task); +} + +void CookieMonster::GetCookiesWithInfoAsync( + const GURL& url, + const CookieOptions& options, + const GetCookieInfoCallback& callback) { + scoped_refptr<GetCookiesWithInfoTask> task = + new GetCookiesWithInfoTask(this, url, options, callback); + + DoCookieTask(task); +} + +void CookieMonster::DeleteCookieAsync(const GURL& url, + const std::string& cookie_name, + const base::Closure& callback) { + scoped_refptr<DeleteCookieTask> task = + new DeleteCookieTask(this, url, cookie_name, callback); + + DoCookieTask(task); +} + +void CookieMonster::DoCookieTask( + const scoped_refptr<CookieMonsterTask>& task_item) { + InitIfNecessary(); + + { + base::AutoLock autolock(lock_); + if (!loaded_) { + queue_.push(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, @@ -577,8 +1122,6 @@ bool CookieMonster::SetCookieWithDetails( if (!HasCookieableScheme(url)) return false; - InitIfNecessary(); - Time creation_time = CurrentTime(); last_time_seen_ = creation_time; @@ -601,17 +1144,6 @@ bool CookieMonster::SetCookieWithDetails( return SetCanonicalCookie(&cc, creation_time, options); } -void CookieMonster::SetCookieWithDetailsAsync( - const GURL& url, const std::string& name, const std::string& value, - const std::string& domain, const std::string& path, - const base::Time& expiration_time, bool secure, bool http_only, - const SetCookiesCallback& callback) { - bool success_ = SetCookieWithDetails(url, name, value, domain, path, - expiration_time, secure, http_only); - if (!callback.is_null()) - callback.Run(success_); -} - bool CookieMonster::InitializeFrom(const CookieList& list) { base::AutoLock autolock(lock_); InitIfNecessary(); @@ -631,7 +1163,6 @@ bool CookieMonster::InitializeFrom(const CookieList& list) { CookieList CookieMonster::GetAllCookies() { base::AutoLock autolock(lock_); - InitIfNecessary(); // This function is being called to scrape the cookie list for management UI // or similar. We shouldn't show expired cookies in this list since it will @@ -662,16 +1193,10 @@ CookieList CookieMonster::GetAllCookies() { return cookie_list; } -void CookieMonster::GetAllCookiesAsync(const GetCookieListCallback& callback) { - if (!callback.is_null()) - callback.Run(GetAllCookies()); -} - CookieList CookieMonster::GetAllCookiesForURLWithOptions( const GURL& url, const CookieOptions& options) { base::AutoLock autolock(lock_); - InitIfNecessary(); std::vector<CanonicalCookie*> cookie_ptrs; FindCookiesForHostAndDomain(url, options, false, &cookie_ptrs); @@ -685,14 +1210,6 @@ CookieList CookieMonster::GetAllCookiesForURLWithOptions( return cookies; } -void CookieMonster::GetAllCookiesForURLWithOptionsAsync( - const GURL& url, - const CookieOptions& options, - const GetCookieListCallback& callback) { - if (!callback.is_null()) - callback.Run(GetAllCookiesForURLWithOptions(url, options)); -} - CookieList CookieMonster::GetAllCookiesForURL(const GURL& url) { CookieOptions options; options.set_include_httponly(); @@ -700,16 +1217,8 @@ CookieList CookieMonster::GetAllCookiesForURL(const GURL& url) { return GetAllCookiesForURLWithOptions(url, options); } -void CookieMonster::GetAllCookiesForURLAsync( - const GURL& url, const GetCookieListCallback& callback) { - if (!callback.is_null()) - callback.Run(GetAllCookiesForURL(url)); -} - int CookieMonster::DeleteAll(bool sync_to_store) { base::AutoLock autolock(lock_); - if (sync_to_store) - InitIfNecessary(); int num_deleted = 0; for (CookieMap::iterator it = cookies_.begin(); it != cookies_.end();) { @@ -725,10 +1234,8 @@ int CookieMonster::DeleteAll(bool sync_to_store) { } int CookieMonster::DeleteAllCreatedBetween(const Time& delete_begin, - const Time& delete_end, - bool sync_to_store) { + const Time& delete_end) { base::AutoLock autolock(lock_); - InitIfNecessary(); int num_deleted = 0; for (CookieMap::iterator it = cookies_.begin(); it != cookies_.end();) { @@ -738,7 +1245,9 @@ int CookieMonster::DeleteAllCreatedBetween(const Time& delete_begin, if (cc->CreationDate() >= delete_begin && (delete_end.is_null() || cc->CreationDate() < delete_end)) { - InternalDeleteCookie(curit, sync_to_store, DELETE_COOKIE_EXPLICIT); + InternalDeleteCookie(curit, + true, /*sync_to_store*/ + DELETE_COOKIE_EXPLICIT); ++num_deleted; } } @@ -746,19 +1255,8 @@ int CookieMonster::DeleteAllCreatedBetween(const Time& delete_begin, return num_deleted; } -void CookieMonster::DeleteAllCreatedBetweenAsync( - const Time& delete_begin, const Time& delete_end, - bool sync_to_store, - const DeleteCallback& callback) { - int num_deleted = DeleteAllCreatedBetween( - delete_begin, delete_end, sync_to_store); - if (!callback.is_null()) - callback.Run(num_deleted); -} - int CookieMonster::DeleteAllForHost(const GURL& url) { base::AutoLock autolock(lock_); - InitIfNecessary(); if (!HasCookieableScheme(url)) return 0; @@ -787,16 +1285,8 @@ int CookieMonster::DeleteAllForHost(const GURL& url) { return num_deleted; } -void CookieMonster::DeleteAllForHostAsync( - const GURL& url, const DeleteCallback& callback) { - int num_deleted = DeleteAllForHost(url); - if (!callback.is_null()) - callback.Run(num_deleted); -} - bool CookieMonster::DeleteCanonicalCookie(const CanonicalCookie& cookie) { base::AutoLock autolock(lock_); - InitIfNecessary(); for (CookieMapItPair its = cookies_.equal_range(GetKey(cookie.Domain())); its.first != its.second; ++its.first) { @@ -809,14 +1299,6 @@ bool CookieMonster::DeleteCanonicalCookie(const CanonicalCookie& cookie) { return false; } -void CookieMonster::DeleteCanonicalCookieAsync( - const CanonicalCookie& cookie, - const DeleteCookieCallback& callback) { - bool result = DeleteCanonicalCookie(cookie); - if (!callback.is_null()) - callback.Run(result); -} - void CookieMonster::SetCookieableSchemes( const char* schemes[], size_t num_schemes) { base::AutoLock autolock(lock_); @@ -865,25 +1347,12 @@ bool CookieMonster::SetCookieWithOptions(const GURL& url, return false; } - InitIfNecessary(); - return SetCookieWithCreationTimeAndOptions(url, cookie_line, Time(), options); } -void CookieMonster::SetCookieWithOptionsAsync( - const GURL& url, - const std::string& cookie_line, - const CookieOptions& options, - const SetCookiesCallback& callback) { - bool result = SetCookieWithOptions(url, cookie_line, options); - if (!callback.is_null()) - callback.Run(result); -} - std::string CookieMonster::GetCookiesWithOptions(const GURL& url, const CookieOptions& options) { base::AutoLock autolock(lock_); - InitIfNecessary(); if (!HasCookieableScheme(url)) return std::string(); @@ -903,14 +1372,6 @@ std::string CookieMonster::GetCookiesWithOptions(const GURL& url, return cookie_line; } -void CookieMonster::GetCookiesWithOptionsAsync( - const GURL& url, const CookieOptions& options, - const GetCookiesCallback& callback) { - std::string cookie = GetCookiesWithOptions(url, options); - if (!callback.is_null()) - callback.Run(cookie); -} - void CookieMonster::GetCookiesWithInfo(const GURL& url, const CookieOptions& options, std::string* cookie_line, @@ -919,7 +1380,6 @@ void CookieMonster::GetCookiesWithInfo(const GURL& url, DCHECK(cookie_infos->empty()); base::AutoLock autolock(lock_); - InitIfNecessary(); if (!HasCookieableScheme(url)) return; @@ -938,22 +1398,9 @@ void CookieMonster::GetCookiesWithInfo(const GURL& url, histogram_time_mac_->AddTime(TimeTicks::Now() - mac_start_time); } -void CookieMonster::GetCookiesWithInfoAsync( - const GURL& url, - const CookieOptions& options, - const GetCookieInfoCallback& callback) { - std::string cookie_line; - std::vector<CookieInfo> cookie_infos; - GetCookiesWithInfo(url, options, &cookie_line, &cookie_infos); - - if (!callback.is_null()) - callback.Run(&cookie_line, &cookie_infos); -} - void CookieMonster::DeleteCookie(const GURL& url, const std::string& cookie_name) { base::AutoLock autolock(lock_); - InitIfNecessary(); if (!HasCookieableScheme(url)) return; @@ -983,14 +1430,6 @@ void CookieMonster::DeleteCookie(const GURL& url, } } -void CookieMonster::DeleteCookieAsync(const GURL& url, - const std::string& cookie_name, - const base::Closure& callback) { - DeleteCookie(url, cookie_name); - if (!callback.is_null()) - callback.Run(); -} - CookieMonster* CookieMonster::GetCookieMonster() { return this; } @@ -1002,6 +1441,7 @@ CookieMonster::~CookieMonster() { bool CookieMonster::SetCookieWithCreationTime(const GURL& url, const std::string& cookie_line, const base::Time& creation_time) { + DCHECK(!store_) << "This method is only to be used by unit-tests."; base::AutoLock autolock(lock_); if (!HasCookieableScheme(url)) { @@ -1016,16 +1456,26 @@ bool CookieMonster::SetCookieWithCreationTime(const GURL& url, void CookieMonster::InitStore() { DCHECK(store_) << "Store must exist to initialize"; - TimeTicks beginning_time(TimeTicks::Now()); + // We bind in the current time so that we can report the wall-clock time for + // loading cookies. + store_->Load(base::Bind(&CookieMonster::OnLoaded, this, TimeTicks::Now())); +} + +void CookieMonster::OnLoaded(TimeTicks beginning_time, + const std::vector<CanonicalCookie*>& cookies) { + StoreLoadedCookies(cookies); + histogram_time_load_->AddTime(TimeTicks::Now() - beginning_time); + + // Invoke the task queue of cookie request. + InvokeQueue(); +} +void CookieMonster::StoreLoadedCookies( + const std::vector<CanonicalCookie*>& cookies) { // Initialize the store and sync in any saved persistent cookies. We don't // care if it's expired, insert it so it can be garbage collected, removed, // and sync'd. - std::vector<CanonicalCookie*> 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(kMaxCookies); - store_->Load(&cookies); + 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 @@ -1064,8 +1514,22 @@ void CookieMonster::InitStore() { // // In particular, the backing store might have given us duplicate cookies. EnsureCookiesMapIsValid(); +} - histogram_time_load_->AddTime(TimeTicks::Now() - beginning_time); +void CookieMonster::InvokeQueue() { + while (true) { + scoped_refptr<CookieMonsterTask> request_task; + { + base::AutoLock autolock(lock_); + if (queue_.empty()) { + loaded_ = true; + break; + } + request_task = queue_.front(); + queue_.pop(); + } + request_task->Run(); + } } void CookieMonster::EnsureCookiesMapIsValid() { @@ -1166,7 +1630,7 @@ int CookieMonster::TrimDuplicateCookiesForKey( for (CookieSet::iterator dupes_it = dupes.begin(); dupes_it != dupes.end(); ++dupes_it) { - InternalDeleteCookie(*dupes_it, true /*sync_to_store*/, + InternalDeleteCookie(*dupes_it, true, DELETE_COOKIE_DUPLICATE_IN_BACKING_STORE); } } @@ -1820,7 +2284,7 @@ CookieMonster::ParsedCookie::~ParsedCookie() { } // Returns true if |c| occurs in |chars| -// TODO maybe make this take an iterator, could check for end also? +// TODO(erikwright): maybe make this take an iterator, could check for end also? static inline bool CharIsA(const char c, const char* chars) { return strchr(chars, c) != NULL; } @@ -1976,8 +2440,8 @@ void CookieMonster::ParsedCookie::ParseTokenValuePairs( std::string::const_iterator start = cookie_line.begin(); std::string::const_iterator it = start; - // TODO Make sure we're stripping \r\n in the network code. Then we - // can log any unexpected terminators. + // TODO(erikwright): Make sure we're stripping \r\n in the network code. + // Then we can log any unexpected terminators. std::string::const_iterator end = FindFirstTerminator(cookie_line); for (int pair_num = 0; pair_num < kMaxPairs && it != end; ++pair_num) { diff --git a/net/base/cookie_monster.h b/net/base/cookie_monster.h index cd24707..e272466 100644 --- a/net/base/cookie_monster.h +++ b/net/base/cookie_monster.h @@ -9,6 +9,7 @@ #pragma once #include <map> +#include <queue> #include <string> #include <utility> #include <vector> @@ -27,6 +28,7 @@ class GURL; namespace base { class Histogram; +class TimeTicks; } namespace net { @@ -138,19 +140,19 @@ class NET_EXPORT CookieMonster : public CookieStore { // i.e. it doesn't begin with a leading '.' character. static bool DomainIsHostOnly(const std::string& domain_string); + // Helper function that adds all cookies from |list| into this instance. + bool InitializeFrom(const CookieList& list); + + typedef base::Callback<void(const CookieList& cookies)> GetCookieListCallback; + typedef base::Callback<void(int num_deleted)> DeleteCallback; + typedef base::Callback<void(bool)> DeleteCookieCallback; + // Sets a cookie given explicit user-provided cookie attributes. The cookie // name, value, domain, etc. are each provided as separate strings. This // function expects each attribute to be well-formed. It will check for // disallowed characters (e.g. the ';' character is disallowed within the // cookie value attribute) and will return false without setting the cookie // if such characters are found. - bool SetCookieWithDetails(const GURL& url, - const std::string& name, - const std::string& value, - const std::string& domain, - const std::string& path, - const base::Time& expiration_time, - bool secure, bool http_only); void SetCookieWithDetailsAsync(const GURL& url, const std::string& name, const std::string& value, @@ -161,16 +163,10 @@ class NET_EXPORT CookieMonster : public CookieStore { const SetCookiesCallback& callback); - // Helper function that adds all cookies from |cookie_monster| into this - // instance. - bool InitializeFrom(const CookieList& list); - // Returns all the cookies, for use in management UI, etc. This does not mark // the cookies as having been accessed. // The returned cookies are ordered by longest path, then by earliest // creation date. - CookieList GetAllCookies(); - typedef base::Callback<void(const CookieList& cookies)> GetCookieListCallback; void GetAllCookiesAsync(const GetCookieListCallback& callback); // Returns all the cookies, for use in management UI, etc. Filters results @@ -178,8 +174,6 @@ class NET_EXPORT CookieMonster : public CookieStore { // mark the cookies as having been accessed. // The returned cookies are ordered by longest path, then earliest // creation date. - CookieList GetAllCookiesForURLWithOptions(const GURL& url, - const CookieOptions& options); void GetAllCookiesForURLWithOptionsAsync( const GURL& url, const CookieOptions& options, @@ -187,38 +181,27 @@ class NET_EXPORT CookieMonster : public CookieStore { // Invokes GetAllCookiesForURLWithOptions with options set to include HTTP // only cookies. - CookieList GetAllCookiesForURL(const GURL& url); void GetAllCookiesForURLAsync(const GURL& url, const GetCookieListCallback& callback); // Deletes all of the cookies. - int DeleteAll(bool sync_to_store); + void DeleteAllAsync(const DeleteCallback& callback); + // Deletes all of the cookies that have a creation_date greater than or equal // to |delete_begin| and less than |delete_end| // Returns the number of cookies that have been deleted. - int DeleteAllCreatedBetween(const base::Time& delete_begin, - const base::Time& delete_end, - bool sync_to_store); - typedef base::Callback<void(int num_deleted)> DeleteCallback; void DeleteAllCreatedBetweenAsync(const base::Time& delete_begin, const base::Time& delete_end, - bool sync_to_store, const DeleteCallback& callback); // Deletes all cookies that match the host of the given URL // regardless of path. This includes all http_only and secure cookies, // but does not include any domain cookies that may apply to this host. // Returns the number of cookies deleted. - int DeleteAllForHost(const GURL& url); - void DeleteAllForHostAsync(const GURL& url, const DeleteCallback& callback); // Deletes one specific cookie. - bool DeleteCanonicalCookie(const CanonicalCookie& cookie); - - typedef SetCookiesCallback DeleteCookieCallback; - void DeleteCanonicalCookieAsync(const CanonicalCookie& cookie, const DeleteCookieCallback& callback); @@ -260,42 +243,31 @@ class NET_EXPORT CookieMonster : public CookieStore { // Sets the cookies specified by |cookie_list| returned from |url| // with options |options| in effect. - virtual bool SetCookieWithOptions(const GURL& url, - const std::string& cookie_line, - const CookieOptions& options); - - virtual void SetCookieWithOptionsAsync(const GURL& url, - const std::string& cookie_line, - const CookieOptions& options, - const SetCookiesCallback& callback); + virtual void SetCookieWithOptionsAsync( + const GURL& url, + const std::string& cookie_line, + const CookieOptions& options, + const SetCookiesCallback& callback) OVERRIDE; // Gets all cookies that apply to |url| given |options|. // The returned cookies are ordered by longest path, then earliest // creation date. - virtual std::string GetCookiesWithOptions(const GURL& url, - const CookieOptions& options); - virtual void GetCookiesWithOptionsAsync( const GURL& url, const CookieOptions& options, - const GetCookiesCallback& callback); - - virtual void GetCookiesWithInfo(const GURL& url, - const CookieOptions& options, - std::string* cookie_line, - std::vector<CookieInfo>* cookie_infos); + const GetCookiesCallback& callback) OVERRIDE; - virtual void GetCookiesWithInfoAsync(const GURL& url, - const CookieOptions& options, - const GetCookieInfoCallback& callback); + virtual void GetCookiesWithInfoAsync( + const GURL& url, + const CookieOptions& options, + const GetCookieInfoCallback& callback) OVERRIDE; // Deletes all cookies with that might apply to |url| that has |cookie_name|. - virtual void DeleteCookie(const GURL& url, const std::string& cookie_name); virtual void DeleteCookieAsync( const GURL& url, const std::string& cookie_name, - const base::Closure& callback); + const base::Closure& callback) OVERRIDE; - virtual CookieMonster* GetCookieMonster(); + virtual CookieMonster* GetCookieMonster() OVERRIDE; // Debugging method to perform various validation checks on the map. // Currently just checking that there are no null CanonicalCookie pointers @@ -309,6 +281,20 @@ class NET_EXPORT CookieMonster : public CookieStore { static const int kDefaultCookieableSchemesCount; private: + // For queueing the cookie monster calls. + class CookieMonsterTask; + class DeleteAllCreatedBetweenTask; + class DeleteAllForHostTask; + class DeleteAllTask; + class DeleteCookieTask; + class DeleteCanonicalCookieTask; + class GetAllCookiesForURLWithOptionsTask; + class GetAllCookiesTask; + class GetCookiesWithOptionsTask; + class GetCookiesWithInfoTask; + class SetCookieWithDetailsTask; + class SetCookieWithOptionsTask; + // Testing support. // For SetCookieWithCreationTime. FRIEND_TEST_ALL_PREFIXES(CookieMonsterTest, @@ -397,6 +383,47 @@ class NET_EXPORT CookieMonster : public CookieStore { virtual ~CookieMonster(); + // The following are synchronous calls to which the asynchronous methods + // delegate either immediately (if the store is loaded) or through a deferred + // task (if the store is not yet loaded). + bool SetCookieWithDetails(const GURL& url, + const std::string& name, + const std::string& value, + const std::string& domain, + const std::string& path, + const base::Time& expiration_time, + bool secure, bool http_only); + + CookieList GetAllCookies(); + + CookieList GetAllCookiesForURLWithOptions(const GURL& url, + const CookieOptions& options); + + CookieList GetAllCookiesForURL(const GURL& url); + + int DeleteAll(bool sync_to_store); + + int DeleteAllCreatedBetween(const base::Time& delete_begin, + const base::Time& delete_end); + + int DeleteAllForHost(const GURL& url); + + bool DeleteCanonicalCookie(const CanonicalCookie& cookie); + + bool SetCookieWithOptions(const GURL& url, + const std::string& cookie_line, + const CookieOptions& options); + + std::string GetCookiesWithOptions(const GURL& url, + const CookieOptions& options); + + void GetCookiesWithInfo(const GURL& url, + const CookieOptions& options, + std::string* cookie_line, + std::vector<CookieInfo>* cookie_infos); + + void DeleteCookie(const GURL& url, const std::string& cookie_name); + bool SetCookieWithCreationTime(const GURL& url, const std::string& cookie_line, const base::Time& creation_time); @@ -407,8 +434,11 @@ class NET_EXPORT CookieMonster : public CookieStore { // Note: this method should always be called with lock_ held. void InitIfNecessary() { if (!initialized_) { - if (store_) + if (store_) { InitStore(); + } else { + loaded_ = true; + } initialized_ = true; } } @@ -417,6 +447,18 @@ class NET_EXPORT CookieMonster : public CookieStore { // Should only be called by InitIfNecessary(). void InitStore(); + // Stores cookies loaded from the backing store and invokes any deferred + // calls. |beginning_time| should be the moment PersistentCookieStore::Load + // was invoked and is used for reporting histogram_time_load_. + void OnLoaded(base::TimeTicks beginning_time, + const std::vector<CanonicalCookie*>& cookies); + + // Stores the loaded cookies. + void StoreLoadedCookies(const std::vector<CanonicalCookie*>& cookies); + + // Invokes deferred calls. + void InvokeQueue(); + // Checks that |cookies_| matches our invariants, and tries to repair any // inconsistencies. (In other words, it does not have duplicate cookies). void EnsureCookiesMapIsValid(); @@ -526,6 +568,10 @@ class NET_EXPORT CookieMonster : public CookieStore { // ugly and increment when we've seen the same time twice. base::Time CurrentTime(); + // Run the cookie request task if cookie loaded, otherwise added the task + // to task queue. + void DoCookieTask(const scoped_refptr<CookieMonsterTask>& task_item); + // Histogram variables; see CookieMonster::InitializeHistograms() in // cookie_monster.cc for details. base::Histogram* histogram_expiration_duration_minutes_; @@ -547,6 +593,14 @@ class NET_EXPORT CookieMonster : public CookieStore { // lazily in InitStoreIfNecessary(). bool initialized_; + // Indicates whether loading from the backend store is completed and + // calls may be immediately processed. + bool loaded_; + + // Queues calls to CookieMonster until loading from the backend store is + // completed. + std::queue<scoped_refptr<CookieMonsterTask> > queue_; + // Indicates whether this cookie monster uses the new effective domain // key scheme or not. ExpiryAndKeyScheme expiry_and_key_scheme_; @@ -861,9 +915,12 @@ class CookieMonster::PersistentCookieStore public: virtual ~PersistentCookieStore() {} + typedef base::Callback<void(const std::vector< + CookieMonster::CanonicalCookie*>&)> LoadedCallback; + // Initializes the store and retrieves the existing cookies. This will be // called only once at startup. - virtual bool Load(std::vector<CookieMonster::CanonicalCookie*>* cookies) = 0; + virtual bool Load(const LoadedCallback& loaded_callback) = 0; virtual void AddCookie(const CanonicalCookie& cc) = 0; virtual void UpdateCookieAccessTime(const CanonicalCookie& cc) = 0; diff --git a/net/base/cookie_monster_perftest.cc b/net/base/cookie_monster_perftest.cc index cd2c35e..98a2b5b 100644 --- a/net/base/cookie_monster_perftest.cc +++ b/net/base/cookie_monster_perftest.cc @@ -4,8 +4,8 @@ #include <algorithm> -#include "net/base/cookie_monster.h" - +#include "base/bind.h" +#include "base/message_loop.h" #include "base/perftimer.h" #include "base/string_util.h" #include "base/stringprintf.h" @@ -15,8 +15,13 @@ #include "testing/gtest/include/gtest/gtest.h" namespace { - class ParsedCookieTest : public testing::Test { }; - class CookieMonsterTest : public testing::Test { }; +class CookieMonsterTest : public testing::Test { + public: + CookieMonsterTest() : message_loop_(new MessageLoopForIO()) {} + + private: + scoped_ptr<MessageLoop> message_loop_; +}; } static const int kNumCookies = 20000; @@ -47,102 +52,172 @@ TEST(ParsedCookieTest, TestParseBigCookies) { static const GURL kUrlGoogle("http://www.google.izzle"); -TEST(CookieMonsterTest, TestAddCookiesOnSingleHost) { +class BaseCallback { + public: + BaseCallback() : has_run_(false) {} + + protected: + void WaitForCallback() { + // Note that the performance tests currently all operate on a loaded cookie + // store (or, more precisely, one that has no backing persistent store). + // Therefore, callbacks will actually always complete synchronously. If the + // tests get more advanced we need to add other means of signaling + // completion. + EXPECT_TRUE(has_run_); + has_run_ = false; + } + + void Run() { + has_run_ = true; + } + + bool has_run_; +}; + + +class SetCookieCallback : public BaseCallback { + public: + void SetCookie( + CookieMonster* cm, const GURL& gurl, const std::string& cookie) { + cm->SetCookieWithOptionsAsync(gurl, cookie, options_, base::Bind( + &SetCookieCallback::Run, base::Unretained(this))); + WaitForCallback(); + } + private: + void Run(bool success) { + EXPECT_TRUE(success); + BaseCallback::Run(); + } + net::CookieOptions options_; +}; + +class GetCookiesCallback : public BaseCallback { + public: + const std::string& GetCookies(CookieMonster* cm, const GURL& gurl) { + cm->GetCookiesWithOptionsAsync(gurl, options_, base::Bind( + &GetCookiesCallback::Run, base::Unretained(this))); + WaitForCallback(); + return cookies_; + } + + private: + void Run(const std::string& cookies) { + cookies_ = cookies; + BaseCallback::Run(); + } + std::string cookies_; + net::CookieOptions options_; +}; + +class GetCookiesWithInfoCallback : public BaseCallback { + public: + const std::string& GetCookiesWithInfo(CookieMonster* cm, const GURL& gurl) { + cm->GetCookiesWithInfoAsync(gurl, options_, base::Bind( + &GetCookiesWithInfoCallback::Run, + base::Unretained(this))); + WaitForCallback(); + return cookies_; + } + + private: + void Run( + const std::string& cookie_line, + const std::vector<CookieStore::CookieInfo>& cookie_infos) { + cookies_ = cookie_line; + BaseCallback::Run(); + } + + std::string cookies_; + net::CookieOptions options_; +}; + + +TEST_F(CookieMonsterTest, TestAddCookiesOnSingleHost) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); std::vector<std::string> cookies; for (int i = 0; i < kNumCookies; i++) { cookies.push_back(base::StringPrintf("a%03d=b", i)); } + SetCookieCallback setCookieCallback; + // Add a bunch of cookies on a single host PerfTimeLogger timer("Cookie_monster_add_single_host"); + for (std::vector<std::string>::const_iterator it = cookies.begin(); it != cookies.end(); ++it) { - EXPECT_TRUE(cm->SetCookie(kUrlGoogle, *it)); + setCookieCallback.SetCookie(cm, kUrlGoogle, *it); } timer.Done(); + GetCookiesCallback getCookiesCallback; + PerfTimeLogger timer2("Cookie_monster_query_single_host"); for (std::vector<std::string>::const_iterator it = cookies.begin(); it != cookies.end(); ++it) { - cm->GetCookies(kUrlGoogle); + getCookiesCallback.GetCookies(cm, kUrlGoogle); } timer2.Done(); PerfTimeLogger timer3("Cookie_monster_deleteall_single_host"); - cm->DeleteAll(false); + cm->DeleteAllAsync(CookieMonster::DeleteCallback()); + MessageLoop::current()->RunAllPending(); timer3.Done(); } -TEST(CookieMonsterTest, TestAddCookieOnManyHosts) { +TEST_F(CookieMonsterTest, TestAddCookieOnManyHosts) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); std::string cookie(kCookieLine); std::vector<GURL> gurls; // just wanna have ffffuunnn for (int i = 0; i < kNumCookies; ++i) { - gurls.push_back(GURL(base::StringPrintf("http://a%04d.izzle", i))); + gurls.push_back(GURL(base::StringPrintf("https://a%04d.izzle", i))); } + SetCookieCallback setCookieCallback; + // Add a cookie on a bunch of host PerfTimeLogger timer("Cookie_monster_add_many_hosts"); for (std::vector<GURL>::const_iterator it = gurls.begin(); it != gurls.end(); ++it) { - EXPECT_TRUE(cm->SetCookie(*it, cookie)); + setCookieCallback.SetCookie(cm, *it, cookie); } timer.Done(); + GetCookiesCallback getCookiesCallback; + PerfTimeLogger timer2("Cookie_monster_query_many_hosts"); for (std::vector<GURL>::const_iterator it = gurls.begin(); it != gurls.end(); ++it) { - cm->GetCookies(*it); + getCookiesCallback.GetCookies(cm, *it); } timer2.Done(); PerfTimeLogger timer3("Cookie_monster_deleteall_many_hosts"); - cm->DeleteAll(false); + cm->DeleteAllAsync(CookieMonster::DeleteCallback()); + MessageLoop::current()->RunAllPending(); timer3.Done(); } -TEST(CookieMonsterTest, TestGetCookiesWithOptions) { +TEST_F(CookieMonsterTest, TestGetCookiesWithInfo) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); - std::string cookie(kCookieLine); + std::vector<GURL> gurls; for (int i = 0; i < kNumCookies; ++i) - gurls.push_back(GURL(base::StringPrintf("http://a%04d.izzle", i))); + gurls.push_back(GURL(base::StringPrintf("https://a%04d.izzle", i))); - for (std::vector<GURL>::const_iterator it = gurls.begin(); - it != gurls.end(); ++it) { - EXPECT_TRUE(cm->SetCookie(*it, cookie)); - } + SetCookieCallback setCookieCallback; - PerfTimeLogger timer("Cookie_monster_get_cookie_info"); for (std::vector<GURL>::const_iterator it = gurls.begin(); it != gurls.end(); ++it) { - CookieOptions options; - std::string cookie_line; - cookie_line = cm->GetCookiesWithOptions(*it, options); + setCookieCallback.SetCookie(cm, *it, kCookieLine); } - timer.Done(); -} -TEST(CookieMonsterTest, TestGetCookiesWithInfo) { - scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); - std::string cookie(kCookieLine); - std::vector<GURL> gurls; - for (int i = 0; i < kNumCookies; ++i) - gurls.push_back(GURL(base::StringPrintf("http://a%04d.izzle", i))); - - for (std::vector<GURL>::const_iterator it = gurls.begin(); - it != gurls.end(); ++it) { - EXPECT_TRUE(cm->SetCookie(*it, cookie)); - } + GetCookiesWithInfoCallback getCookiesCallback; PerfTimeLogger timer("Cookie_monster_get_cookie_info"); for (std::vector<GURL>::const_iterator it = gurls.begin(); it != gurls.end(); ++it) { - CookieOptions options; - std::string cookie_line; - std::vector<CookieStore::CookieInfo> cookie_infos; - cm->GetCookiesWithInfo(*it, options, &cookie_line, &cookie_infos); + getCookiesCallback.GetCookiesWithInfo(cm, *it); } timer.Done(); } @@ -151,8 +226,10 @@ static int CountInString(const std::string& str, char c) { return std::count(str.begin(), str.end(), c); } -TEST(CookieMonsterTest, TestDomainTree) { +TEST_F(CookieMonsterTest, TestDomainTree) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); + GetCookiesCallback getCookiesCallback; + SetCookieCallback setCookieCallback; const char* domain_cookie_format_tree = "a=b; domain=%s"; const std::string domain_base("top.com"); @@ -188,25 +265,25 @@ TEST(CookieMonsterTest, TestDomainTree) { GURL gurl("https://" + *it + "/"); const std::string cookie = base::StringPrintf(domain_cookie_format_tree, it->c_str()); - EXPECT_TRUE(cm->SetCookie(gurl, cookie)); + setCookieCallback.SetCookie(cm, gurl, cookie); } EXPECT_EQ(31u, cm->GetAllCookies().size()); GURL probe_gurl("https://b.a.b.a.top.com/"); - std::string cookie_line; - cookie_line = cm->GetCookies(probe_gurl); - EXPECT_EQ(5, CountInString(cookie_line, '=')) << "Cookie line: " - << cookie_line; + std::string cookie_line = getCookiesCallback.GetCookies(cm, probe_gurl); + EXPECT_EQ(5, CountInString(cookie_line, '=')) << "Cookie line: " << + cookie_line; PerfTimeLogger timer("Cookie_monster_query_domain_tree"); for (int i = 0; i < kNumCookies; i++) { - cm->GetCookies(probe_gurl); + getCookiesCallback.GetCookies(cm, probe_gurl); } timer.Done(); - } -TEST(CookieMonsterTest, TestDomainLine) { +TEST_F(CookieMonsterTest, TestDomainLine) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); + SetCookieCallback setCookieCallback; + GetCookiesCallback getCookiesCallback; std::vector<std::string> domain_list; GURL probe_gurl("https://b.a.b.a.top.com/"); std::string cookie_line; @@ -230,23 +307,23 @@ TEST(CookieMonsterTest, TestDomainLine) { GURL gurl("https://" + *it + "/"); const std::string cookie = base::StringPrintf(domain_cookie_format_line, i, it->c_str()); - EXPECT_TRUE(cm->SetCookie(gurl, cookie)); + setCookieCallback.SetCookie(cm, gurl, cookie); } } - EXPECT_EQ(32u, cm->GetAllCookies().size()); - cookie_line = cm->GetCookies(probe_gurl); + cookie_line = getCookiesCallback.GetCookies(cm, probe_gurl); EXPECT_EQ(32, CountInString(cookie_line, '=')); PerfTimeLogger timer2("Cookie_monster_query_domain_line"); for (int i = 0; i < kNumCookies; i++) { - cm->GetCookies(probe_gurl); + getCookiesCallback.GetCookies(cm, probe_gurl); } timer2.Done(); } -TEST(CookieMonsterTest, TestImport) { +TEST_F(CookieMonsterTest, TestImport) { scoped_refptr<MockPersistentCookieStore> store(new MockPersistentCookieStore); std::vector<CookieMonster::CanonicalCookie*> initial_cookies; + GetCookiesCallback getCookiesCallback; // We want to setup a fairly large backing store, with 300 domains of 50 // cookies each. Creation times must be unique. @@ -272,14 +349,14 @@ TEST(CookieMonsterTest, TestImport) { GURL gurl("www.google.com"); CookieOptions options; PerfTimeLogger timer("Cookie_monster_import_from_store"); - cm->GetCookiesWithOptions(gurl, options); + getCookiesCallback.GetCookies(cm, gurl); timer.Done(); // Just confirm keys were set as expected. EXPECT_EQ("domain_1.com", cm->GetKey("www.Domain_1.com")); } -TEST(CookieMonsterTest, TestGetKey) { +TEST_F(CookieMonsterTest, TestGetKey) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); PerfTimeLogger timer("Cookie_monster_get_key"); for (int i = 0; i < kNumCookies; i++) @@ -294,7 +371,9 @@ TEST(CookieMonsterTest, TestGetKey) { // 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) { +TEST_F(CookieMonsterTest, TestGCTimes) { + SetCookieCallback setCookieCallback; + const struct TestCase { const char* name; int num_cookies; @@ -338,13 +417,13 @@ TEST(CookieMonsterTest, TestGCTimes) { 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)); + setCookieCallback.SetCookie(cm, 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)); + setCookieCallback.SetCookie(cm, gurl, cookie_line); timer.Done(); } } -} // namespace +} // namespace diff --git a/net/base/cookie_monster_store_test.cc b/net/base/cookie_monster_store_test.cc index b3a6c13..717a70c 100644 --- a/net/base/cookie_monster_store_test.cc +++ b/net/base/cookie_monster_store_test.cc @@ -25,11 +25,13 @@ void MockPersistentCookieStore::SetLoadExpectation( load_result_ = result; } -bool MockPersistentCookieStore::Load( - std::vector<CookieMonster::CanonicalCookie*>* out_cookies) { +bool MockPersistentCookieStore::Load(const LoadedCallback& loaded_callback) { bool ok = load_return_value_; - if (ok) - *out_cookies = load_result_; + std::vector<CookieMonster::CanonicalCookie*> out_cookies; + if (ok) { + out_cookies = load_result_; + } + loaded_callback.Run(out_cookies); return ok; } @@ -108,11 +110,13 @@ MockSimplePersistentCookieStore::MockSimplePersistentCookieStore() {} MockSimplePersistentCookieStore::~MockSimplePersistentCookieStore() {} bool MockSimplePersistentCookieStore::Load( - std::vector<CookieMonster::CanonicalCookie*>* out_cookies) { + const LoadedCallback& loaded_callback) { + std::vector<CookieMonster::CanonicalCookie*> out_cookies; for (CanonicalCookieMap::const_iterator it = cookies_.begin(); it != cookies_.end(); it++) - out_cookies->push_back( + out_cookies.push_back( new CookieMonster::CanonicalCookie(it->second)); + loaded_callback.Run(out_cookies); return true; } diff --git a/net/base/cookie_monster_store_test.h b/net/base/cookie_monster_store_test.h index 1ee757d..bff777c 100644 --- a/net/base/cookie_monster_store_test.h +++ b/net/base/cookie_monster_store_test.h @@ -7,6 +7,14 @@ // that need to test out CookieMonster interactions with the backing store. // It should only be included by test code. +#ifndef NET_BASE_COOKIE_MONSTER_STORE_TEST_H_ +#define NET_BASE_COOKIE_MONSTER_STORE_TEST_H_ +#pragma once + +#include <map> +#include <utility> +#include <string> +#include <vector> #include "net/base/cookie_monster.h" namespace base { @@ -51,21 +59,20 @@ class MockPersistentCookieStore return commands_; } - virtual bool Load( - std::vector<CookieMonster::CanonicalCookie*>* out_cookies); + virtual bool Load(const LoadedCallback& loaded_callback) OVERRIDE; - virtual void AddCookie(const CookieMonster::CanonicalCookie& cookie); + virtual void AddCookie(const CookieMonster::CanonicalCookie& cookie) OVERRIDE; virtual void UpdateCookieAccessTime( - const CookieMonster::CanonicalCookie& cookie); + const CookieMonster::CanonicalCookie& cookie) OVERRIDE; virtual void DeleteCookie( - const CookieMonster::CanonicalCookie& cookie); + const CookieMonster::CanonicalCookie& cookie) OVERRIDE; - virtual void Flush(Task* completion_task); + virtual void Flush(Task* completion_task) OVERRIDE; // No files are created so nothing to clear either - virtual void SetClearLocalStateOnExit(bool clear_local_state); + virtual void SetClearLocalStateOnExit(bool clear_local_state) OVERRIDE; private: CommandList commands_; @@ -117,8 +124,7 @@ class MockSimplePersistentCookieStore MockSimplePersistentCookieStore(); virtual ~MockSimplePersistentCookieStore(); - virtual bool Load( - std::vector<CookieMonster::CanonicalCookie*>* out_cookies); + virtual bool Load(const LoadedCallback& loaded_callback); virtual void AddCookie( const CookieMonster::CanonicalCookie& cookie); @@ -154,3 +160,5 @@ CookieMonster* CreateMonsterFromStoreForGC( int days_old); } // namespace net + +#endif // NET_BASE_COOKIE_MONSTER_STORE_TEST_H_ diff --git a/net/base/cookie_monster_unittest.cc b/net/base/cookie_monster_unittest.cc index 3b41c7b..67f3396 100644 --- a/net/base/cookie_monster_unittest.cc +++ b/net/base/cookie_monster_unittest.cc @@ -20,7 +20,8 @@ #include "base/time.h" #include "googleurl/src/gurl.h" #include "net/base/cookie_monster.h" -#include "net/base/cookie_monster_store_test.h" // For CookieStore Mock +#include "net/base/cookie_monster_store_test.h" // For CookieStore mock +#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" namespace net { @@ -31,6 +32,20 @@ using base::Thread; namespace { +// TODO(erikwright): Replace the pre-existing MockPersistentCookieStore (and +// brethren) with this one, and remove the 'New' prefix. +class NewMockPersistentCookieStore + : public CookieMonster::PersistentCookieStore { + public: + MOCK_METHOD1(Load, bool(const LoadedCallback& loaded_callback)); + MOCK_METHOD1(AddCookie, void(const CookieMonster::CanonicalCookie& cc)); + MOCK_METHOD1(UpdateCookieAccessTime, + void(const CookieMonster::CanonicalCookie& cc)); + MOCK_METHOD1(DeleteCookie, void(const CookieMonster::CanonicalCookie& cc)); + MOCK_METHOD1(SetClearLocalStateOnExit, void(bool clear_local_state)); + MOCK_METHOD1(Flush, void(Task* completion_task)); +}; + const int kTimeout = 1000; const char* kTopLevelDomainPlus1 = "http://www.harvard.edu"; @@ -145,10 +160,10 @@ class GetCookiesWithInfoCallback : public CookieCallback { : CookieCallback(run_in_thread) {} void Run( - std::string* cookie_line, - std::vector<CookieStore::CookieInfo>* cookie_info) { - cookie_line_ = *cookie_line; - cookie_info_ = *cookie_info; + const std::string& cookie_line, + const std::vector<CookieStore::CookieInfo>& cookie_info) { + cookie_line_ = cookie_line; + cookie_info_ = cookie_info; CallbackEpilogue(); } @@ -417,7 +432,7 @@ TEST(ParsedCookieTest, TooManyPairs) { EXPECT_FALSE(pc2.IsSecure()); } -// TODO some better test cases for invalid cookies. +// TODO(erikwright): some better test cases for invalid cookies. TEST(ParsedCookieTest, InvalidWhitespace) { CookieMonster::ParsedCookie pc(" "); EXPECT_FALSE(pc.IsValid()); @@ -499,9 +514,9 @@ class CookieMonsterTest : public testing::Test { std::string GetCookies(CookieMonster* cm, const GURL& url) { DCHECK(cm); GetCookieStringCallback callback; - cm->GetCookiesAsync( - url, base::Bind(&GetCookieStringCallback::Run, - base::Unretained(&callback))); + cm->GetCookiesWithOptionsAsync( + url, CookieOptions(), base::Bind(&GetCookieStringCallback::Run, + base::Unretained(&callback))); RunFor(kTimeout); EXPECT_TRUE(callback.did_run()); return callback.cookie(); @@ -589,6 +604,13 @@ class CookieMonsterTest : public testing::Test { return callback.result(); } + bool SetCookie(CookieMonster* cm, + const GURL& url, + const std::string& cookie_line) { + CookieOptions options; + return SetCookieWithOptions(cm, url, cookie_line, options); + } + bool SetCookieWithDetails(CookieMonster* cm, const GURL& url, const std::string& name, @@ -619,14 +641,23 @@ class CookieMonsterTest : public testing::Test { EXPECT_TRUE(callback.did_run()); } + int DeleteAll(CookieMonster*cm) { + DCHECK(cm); + DeleteCallback callback; + cm->DeleteAllAsync( + base::Bind(&DeleteCallback::Run, base::Unretained(&callback))); + RunFor(kTimeout); + EXPECT_TRUE(callback.did_run()); + return callback.num_deleted(); + } + int DeleteAllCreatedBetween(CookieMonster*cm, const base::Time& delete_begin, - const base::Time& delete_end, - bool sync_to_store) { + const base::Time& delete_end) { DCHECK(cm); DeleteCallback callback; cm->DeleteAllCreatedBetweenAsync( - delete_begin, delete_end, sync_to_store, + delete_begin, delete_end, base::Bind(&DeleteCallback::Run, base::Unretained(&callback))); RunFor(kTimeout); EXPECT_TRUE(callback.did_run()); @@ -675,7 +706,7 @@ class CookieMonsterTest : public testing::Test { GURL url_top_level_domain_plus_3(kTopLevelDomainPlus3); GURL url_other(kOtherDomain); - cm->DeleteAll(true); + DeleteAll(cm); // Static population for probe: // * Three levels of domain cookie (.b.a, .c.b.a, .d.c.b.a) @@ -778,7 +809,7 @@ class CookieMonsterTest : public testing::Test { 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)); + EXPECT_TRUE(SetCookie(cm, url_google_, cookie)); std::string cookies = this->GetCookies(cm, url_google_); // Make sure we find it in the cookies. EXPECT_NE(cookies.find(cookie), std::string::npos); @@ -800,9 +831,9 @@ class CookieMonsterTest : public testing::Test { 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)); + EXPECT_TRUE(SetCookie(cm, url_google_, cookie_general)); std::string cookie_specific = base::StringPrintf("c%03d=b", i); - EXPECT_TRUE(cm->SetCookie(url_google_specific, cookie_specific)); + EXPECT_TRUE(SetCookie(cm, url_google_specific, cookie_specific)); std::string cookies_general = this->GetCookies(cm, url_google_); EXPECT_NE(cookies_general.find(cookie_general), std::string::npos); std::string cookies_specific = @@ -840,6 +871,16 @@ class CookieMonsterTest : public testing::Test { } } + // Function for creating a CM with a number of cookies in it, + // no store (and hence no ability to affect access time). + CookieMonster* CreateMonsterForGC(int num_cookies) { + CookieMonster* cm(new CookieMonster(NULL, NULL)); + for (int i = 0; i < num_cookies; i++) { + SetCookie(cm, GURL(StringPrintf("http://h%05d.izzle", i)), "a=1"); + } + return cm; + } + protected: GURL url_google_; GURL url_google_secure_; @@ -849,15 +890,471 @@ class CookieMonsterTest : public testing::Test { ScopedRunnableMethodFactory<MessageLoop> message_loop_factory_; }; +// TODO(erikwright): Replace the other callbacks and synchronous helper methods +// in this test suite with these Mocks. +template<typename T, typename C> class MockCookieCallback { + public: + C AsCallback() { + return base::Bind(&T::Invoke, base::Unretained(static_cast<T*>(this))); + } +}; + +class MockGetCookiesCallback + : public MockCookieCallback<MockGetCookiesCallback, + CookieStore::GetCookiesCallback> { + public: + MOCK_METHOD1(Invoke, void(const std::string& cookies)); +}; + +class MockGetCookieInfoCallback + : public MockCookieCallback<MockGetCookieInfoCallback, + CookieStore::GetCookieInfoCallback> { + public: + MOCK_METHOD2(Invoke, + void(const std::string& cookies, + const std::vector<CookieStore::CookieInfo>& cookie_infos)); +}; + +class MockSetCookiesCallback + : public MockCookieCallback<MockSetCookiesCallback, + CookieStore::SetCookiesCallback> { + public: + MOCK_METHOD1(Invoke, void(bool success)); +}; + +class MockClosure + : public MockCookieCallback<MockClosure, base::Closure> { + public: + MOCK_METHOD0(Invoke, void(void)); +}; + +class MockGetCookieListCallback + : public MockCookieCallback<MockGetCookieListCallback, + CookieMonster::GetCookieListCallback> { + public: + MOCK_METHOD1(Invoke, void(const CookieList& cookies)); +}; + +class MockDeleteCallback + : public MockCookieCallback<MockDeleteCallback, + CookieMonster::DeleteCallback> { + public: + MOCK_METHOD1(Invoke, void(int num_deleted)); +}; + +class MockDeleteCookieCallback + : public MockCookieCallback<MockDeleteCookieCallback, + CookieMonster::DeleteCookieCallback> { + public: + MOCK_METHOD1(Invoke, void(bool success)); +}; + +ACTION(QuitCurrentMessageLoop) { + MessageLoop::current()->PostTask(FROM_HERE, new MessageLoop::QuitTask()); +} + +// TODO(erikwright): When the synchronous helpers 'GetCookies' etc. are removed, +// rename these, removing the 'Action' suffix. +ACTION_P4(DeleteCookieAction, cookie_monster, url, name, callback) { + cookie_monster->DeleteCookieAsync(url, name, callback->AsCallback()); +} +ACTION_P3(GetCookiesAction, cookie_monster, url, callback) { + cookie_monster->GetCookiesWithOptionsAsync( + url, CookieOptions(), callback->AsCallback()); +} +ACTION_P3(GetCookiesWithInfoAction, cookie_monster, url, callback) { + cookie_monster->GetCookiesWithInfoAsync( + url, CookieOptions(), callback->AsCallback()); +} +ACTION_P4(SetCookieAction, cookie_monster, url, cookie_line, callback) { + cookie_monster->SetCookieWithOptionsAsync( + url, cookie_line, CookieOptions(), callback->AsCallback()); +} +ACTION_P4(DeleteAllCreatedBetweenAction, + cookie_monster, delete_begin, delete_end, callback) { + cookie_monster->DeleteAllCreatedBetweenAsync( + delete_begin, delete_end, callback->AsCallback()); +} +ACTION_P10(SetCookieWithDetailsAction, + cookie_monster, url, name, value, domain, path, expiration_time, + secure, http_only, callback) { + cookie_monster->SetCookieWithDetailsAsync( + url, name, value, domain, path, expiration_time, secure, http_only, + callback->AsCallback()); +} + +ACTION_P2(GetAllCookiesAction, cookie_monster, callback) { + cookie_monster->GetAllCookiesAsync(callback->AsCallback()); +} + +ACTION_P3(DeleteAllForHostAction, cookie_monster, url, callback) { + cookie_monster->DeleteAllForHostAsync(url, callback->AsCallback()); +} + +ACTION_P3(DeleteCanonicalCookieAction, cookie_monster, cookie, callback) { + cookie_monster->DeleteCanonicalCookieAsync(cookie, callback->AsCallback()); +} + +ACTION_P2(DeleteAllAction, cookie_monster, callback) { + cookie_monster->DeleteAllAsync(callback->AsCallback()); +} + +ACTION_P3(GetAllCookiesForUrlWithOptionsAction, cookie_monster, url, callback) { + cookie_monster->GetAllCookiesForURLWithOptionsAsync( + url, CookieOptions(), callback->AsCallback()); +} + +ACTION_P3(GetAllCookiesForUrlAction, cookie_monster, url, callback) { + cookie_monster->GetAllCookiesForURLAsync(url, callback->AsCallback()); +} + } // namespace +// This test suite verifies the task deferral behaviour of the CookieMonster. +// Specifically, for each asynchronous method, verify that: +// 1. invoking it on an uninitialized cookie store causes the store to begin +// loading its backing data. +// 2. The initial invocation does not complete until the loading completes. +// 3. Invocations after the loading has completed complete immediately. +class DeferredCookieTaskTest : public CookieMonsterTest { + protected: + DeferredCookieTaskTest() { + persistent_store_ = new NewMockPersistentCookieStore(); + cookie_monster_ = new CookieMonster(persistent_store_.get(), NULL); + } + + // Defines a cookie to be returned from PersistentCookieStore::Load + void DeclareLoadedCookie(const std::string& key, + const std::string& cookie_line, + const base::Time& creation_time) { + AddCookieToList(key, cookie_line, creation_time, &loaded_cookies_); + } + + // Runs the message loop, waiting until PersistentCookieStore::Load is called. + // Call CompleteLoadingAndWait to cause the load to complete. + void WaitForLoadCall() { + RunFor(kTimeout); + + // Verify that PeristentStore::Load was called. + testing::Mock::VerifyAndClear(persistent_store_.get()); + } + + // Invokes the PersistentCookieStore::Load completion callback and waits + // until the message loop is quit. + void CompleteLoadingAndWait() { + loaded_callback_.Run(loaded_cookies_); + RunFor(kTimeout); + } + + // Performs the provided action, expecting it to cause a call to + // PersistentCookieStore::Load. Call WaitForLoadCall to verify the load call + // is received. + void BeginWith(testing::Action<void(void)> action) { + EXPECT_CALL(*this, Begin()).WillOnce(action); + ExpectLoadCall(); + Begin(); + } + + // Declares an expectation that PersistentCookieStore::Load will be called, + // saving the provided callback and sending a quit to the message loop. + void ExpectLoadCall() { + EXPECT_CALL(*persistent_store_, Load(testing::_)).WillOnce(testing::DoAll( + testing::SaveArg<0>(&loaded_callback_), + QuitCurrentMessageLoop(), + testing::Return(true))); + } + + // Invokes the initial action. + MOCK_METHOD0(Begin, void(void)); + + // Returns the CookieMonster instance under test. + CookieMonster& cookie_monster() { return *cookie_monster_; } + + private: + // Declares that mock expectations in this test suite are strictly ordered. + testing::InSequence in_sequence_; + // Holds cookies to be returned from PersistentCookieStore::Load. + std::vector<CookieMonster::CanonicalCookie*> loaded_cookies_; + // Stores the callback passed from the CookieMonster to the + // PersistentCookieStore + CookieMonster::PersistentCookieStore::LoadedCallback loaded_callback_; + // Stores the CookieMonster under test. + scoped_refptr<CookieMonster> cookie_monster_; + // Stores the mock PersistentCookieStore. + scoped_refptr<NewMockPersistentCookieStore> persistent_store_; +}; + +TEST_F(DeferredCookieTaskTest, DeferredGetCookies) { + DeclareLoadedCookie("www.google.izzle", + "X=1; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT", + Time::Now() + TimeDelta::FromDays(3)); + + MockGetCookiesCallback get_cookies_callback; + + BeginWith(GetCookiesAction( + &cookie_monster(), url_google_, &get_cookies_callback)); + + WaitForLoadCall(); + + EXPECT_CALL(get_cookies_callback, Invoke("X=1")).WillOnce( + GetCookiesAction(&cookie_monster(), url_google_, &get_cookies_callback)); + EXPECT_CALL(get_cookies_callback, Invoke("X=1")).WillOnce( + QuitCurrentMessageLoop()); + + CompleteLoadingAndWait(); +} + +TEST_F(DeferredCookieTaskTest, DeferredGetCookiesWithInfo) { + DeclareLoadedCookie("www.google.izzle", + "X=1; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT", + Time::Now() + TimeDelta::FromDays(3)); + + MockGetCookieInfoCallback get_cookie_info_callback; + + BeginWith(GetCookiesWithInfoAction( + &cookie_monster(), url_google_, &get_cookie_info_callback)); + + WaitForLoadCall(); + + EXPECT_CALL(get_cookie_info_callback, Invoke("X=1", testing::_)).WillOnce( + GetCookiesWithInfoAction( + &cookie_monster(), url_google_, &get_cookie_info_callback)); + EXPECT_CALL(get_cookie_info_callback, Invoke("X=1", testing::_)).WillOnce( + QuitCurrentMessageLoop()); + + CompleteLoadingAndWait(); +} + +TEST_F(DeferredCookieTaskTest, DeferredSetCookie) { + MockSetCookiesCallback set_cookies_callback; + + BeginWith(SetCookieAction( + &cookie_monster(), url_google_, "A=B", &set_cookies_callback)); + + WaitForLoadCall(); + + EXPECT_CALL(set_cookies_callback, Invoke(true)).WillOnce( + SetCookieAction( + &cookie_monster(), url_google_, "X=Y", &set_cookies_callback)); + EXPECT_CALL(set_cookies_callback, Invoke(true)).WillOnce( + QuitCurrentMessageLoop()); + + CompleteLoadingAndWait(); +} + +TEST_F(DeferredCookieTaskTest, DeferredDeleteCookie) { + MockClosure delete_cookie_callback; + + BeginWith(DeleteCookieAction( + &cookie_monster(), url_google_, "A", &delete_cookie_callback)); + + WaitForLoadCall(); + + EXPECT_CALL(delete_cookie_callback, Invoke()).WillOnce( + DeleteCookieAction( + &cookie_monster(), url_google_, "X", &delete_cookie_callback)); + EXPECT_CALL(delete_cookie_callback, Invoke()).WillOnce( + QuitCurrentMessageLoop()); + + CompleteLoadingAndWait(); +} + +TEST_F(DeferredCookieTaskTest, DeferredSetCookieWithDetails) { + MockSetCookiesCallback set_cookies_callback; + + BeginWith(SetCookieWithDetailsAction( + &cookie_monster(), url_google_foo_, "A", "B", std::string(), "/foo", + base::Time(), false, false, &set_cookies_callback)); + + WaitForLoadCall(); + + EXPECT_CALL(set_cookies_callback, Invoke(true)).WillOnce( + SetCookieWithDetailsAction( + &cookie_monster(), url_google_foo_, "A", "B", std::string(), "/foo", + base::Time(), false, false, &set_cookies_callback)); + EXPECT_CALL(set_cookies_callback, Invoke(true)).WillOnce( + QuitCurrentMessageLoop()); + + CompleteLoadingAndWait(); +} + +TEST_F(DeferredCookieTaskTest, DeferredGetAllCookies) { + DeclareLoadedCookie("www.google.izzle", + "X=1; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT", + Time::Now() + TimeDelta::FromDays(3)); + + MockGetCookieListCallback get_cookie_list_callback; + + BeginWith(GetAllCookiesAction( + &cookie_monster(), &get_cookie_list_callback)); + + WaitForLoadCall(); + + EXPECT_CALL(get_cookie_list_callback, Invoke(testing::_)).WillOnce( + GetAllCookiesAction(&cookie_monster(), &get_cookie_list_callback)); + EXPECT_CALL(get_cookie_list_callback, Invoke(testing::_)).WillOnce( + QuitCurrentMessageLoop()); + + CompleteLoadingAndWait(); +} + +TEST_F(DeferredCookieTaskTest, DeferredGetAllForUrlCookies) { + DeclareLoadedCookie("www.google.izzle", + "X=1; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT", + Time::Now() + TimeDelta::FromDays(3)); + + MockGetCookieListCallback get_cookie_list_callback; + + BeginWith(GetAllCookiesForUrlAction( + &cookie_monster(), url_google_, &get_cookie_list_callback)); + + WaitForLoadCall(); + + EXPECT_CALL(get_cookie_list_callback, Invoke(testing::_)).WillOnce( + GetAllCookiesForUrlAction( + &cookie_monster(), url_google_, &get_cookie_list_callback)); + EXPECT_CALL(get_cookie_list_callback, Invoke(testing::_)).WillOnce( + QuitCurrentMessageLoop()); + + CompleteLoadingAndWait(); +} + + +TEST_F(DeferredCookieTaskTest, DeferredGetAllForUrlWithOptionsCookies) { + DeclareLoadedCookie("www.google.izzle", + "X=1; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT", + Time::Now() + TimeDelta::FromDays(3)); + + MockGetCookieListCallback get_cookie_list_callback; + + BeginWith(GetAllCookiesForUrlWithOptionsAction( + &cookie_monster(), url_google_, &get_cookie_list_callback)); + + WaitForLoadCall(); + + EXPECT_CALL(get_cookie_list_callback, Invoke(testing::_)).WillOnce( + GetAllCookiesForUrlWithOptionsAction( + &cookie_monster(), url_google_, &get_cookie_list_callback)); + EXPECT_CALL(get_cookie_list_callback, Invoke(testing::_)).WillOnce( + QuitCurrentMessageLoop()); + + CompleteLoadingAndWait(); +} + +TEST_F(DeferredCookieTaskTest, DeferredDeleteAllCookies) { + MockDeleteCallback delete_callback; + + BeginWith(DeleteAllAction( + &cookie_monster(), &delete_callback)); + + WaitForLoadCall(); + + EXPECT_CALL(delete_callback, Invoke(false)).WillOnce( + DeleteAllAction(&cookie_monster(), &delete_callback)); + EXPECT_CALL(delete_callback, Invoke(false)).WillOnce( + QuitCurrentMessageLoop()); + + CompleteLoadingAndWait(); +} + +TEST_F(DeferredCookieTaskTest, DeferredDeleteAllCreatedBetweenCookies) { + MockDeleteCallback delete_callback; + + BeginWith(DeleteAllCreatedBetweenAction( + &cookie_monster(), base::Time(), base::Time::Now(), &delete_callback)); + + WaitForLoadCall(); + + EXPECT_CALL(delete_callback, Invoke(false)).WillOnce( + DeleteAllCreatedBetweenAction( + &cookie_monster(), base::Time(), base::Time::Now(), + &delete_callback)); + EXPECT_CALL(delete_callback, Invoke(false)).WillOnce( + QuitCurrentMessageLoop()); + + CompleteLoadingAndWait(); +} + +TEST_F(DeferredCookieTaskTest, DeferredDeleteAllForHostCookies) { + MockDeleteCallback delete_callback; + + BeginWith(DeleteAllForHostAction( + &cookie_monster(), url_google_, &delete_callback)); + + WaitForLoadCall(); + + EXPECT_CALL(delete_callback, Invoke(false)).WillOnce( + DeleteAllForHostAction( + &cookie_monster(), url_google_, &delete_callback)); + EXPECT_CALL(delete_callback, Invoke(false)).WillOnce( + QuitCurrentMessageLoop()); + + CompleteLoadingAndWait(); +} + +TEST_F(DeferredCookieTaskTest, DeferredDeleteCanonicalCookie) { + std::vector<CookieMonster::CanonicalCookie*> cookies; + AddCookieToList("www.google.com", "X=1; path=/", base::Time::Now(), &cookies); + + MockDeleteCookieCallback delete_cookie_callback; + + BeginWith(DeleteCanonicalCookieAction( + &cookie_monster(), *cookies[0], &delete_cookie_callback)); + + WaitForLoadCall(); + + EXPECT_CALL(delete_cookie_callback, Invoke(false)).WillOnce( + DeleteCanonicalCookieAction( + &cookie_monster(), *cookies[0], &delete_cookie_callback)); + EXPECT_CALL(delete_cookie_callback, Invoke(false)).WillOnce( + QuitCurrentMessageLoop()); + + CompleteLoadingAndWait(); +} + +// Verify that a series of queued tasks are executed in order upon loading of +// the backing store and that new tasks received while the queued tasks are +// being dispatched go to the end of the queue. +TEST_F(DeferredCookieTaskTest, DeferredTaskOrder) { + DeclareLoadedCookie("www.google.izzle", + "X=1; path=/; expires=Mon, 18-Apr-22 22:50:14 GMT", + Time::Now() + TimeDelta::FromDays(3)); + + MockGetCookiesCallback get_cookies_callback; + MockSetCookiesCallback set_cookies_callback; + MockClosure delete_cookie_callback; + MockGetCookieInfoCallback get_cookie_info_callback; + + EXPECT_CALL(*this, Begin()).WillOnce(testing::DoAll( + GetCookiesAction( + &cookie_monster(), url_google_, &get_cookies_callback), + SetCookieAction( + &cookie_monster(), url_google_, "A=B", &set_cookies_callback), + DeleteCookieAction( + &cookie_monster(), url_google_, "A", &delete_cookie_callback))); + ExpectLoadCall(); + Begin(); + + WaitForLoadCall(); + EXPECT_CALL(get_cookies_callback, Invoke("X=1")).WillOnce( + GetCookiesWithInfoAction( + &cookie_monster(), url_google_, &get_cookie_info_callback)); + + EXPECT_CALL(set_cookies_callback, Invoke(true)); + EXPECT_CALL(delete_cookie_callback, Invoke()); + EXPECT_CALL(get_cookie_info_callback, Invoke("X=1", testing::_)).WillOnce( + QuitCurrentMessageLoop()); + + CompleteLoadingAndWait(); +} + TEST_F(CookieMonsterTest, DomainTest) { scoped_refptr<MockPersistentCookieStore> store( new MockPersistentCookieStore); scoped_refptr<CookieMonster> cm(new CookieMonster(store, NULL)); - EXPECT_TRUE(cm->SetCookie(url_google_, "A=B")); + EXPECT_TRUE(SetCookie(cm, url_google_, "A=B")); EXPECT_EQ("A=B", GetCookies(cm, url_google_)); - EXPECT_TRUE(cm->SetCookie(url_google_, "C=D; domain=.google.izzle")); + EXPECT_TRUE(SetCookie(cm, url_google_, "C=D; domain=.google.izzle")); EXPECT_EQ("A=B; C=D", GetCookies(cm, url_google_)); // Verify that A=B was set as a host cookie rather than a domain @@ -865,18 +1362,18 @@ TEST_F(CookieMonsterTest, DomainTest) { EXPECT_EQ("C=D", GetCookies(cm, GURL("http://foo.www.google.izzle"))); // Test and make sure we find domain cookies on the same domain. - EXPECT_TRUE(cm->SetCookie(url_google_, "E=F; domain=.www.google.izzle")); + EXPECT_TRUE(SetCookie(cm, url_google_, "E=F; domain=.www.google.izzle")); EXPECT_EQ("A=B; C=D; E=F", GetCookies(cm, url_google_)); // Test setting a domain= that doesn't start w/ a dot, should // treat it as a domain cookie, as if there was a pre-pended dot. - EXPECT_TRUE(cm->SetCookie(url_google_, "G=H; domain=www.google.izzle")); + EXPECT_TRUE(SetCookie(cm, url_google_, "G=H; domain=www.google.izzle")); EXPECT_EQ("A=B; C=D; E=F; G=H", GetCookies(cm, url_google_)); // Test domain enforcement, should fail on a sub-domain or something too deep. - EXPECT_FALSE(cm->SetCookie(url_google_, "I=J; domain=.izzle")); + EXPECT_FALSE(SetCookie(cm, url_google_, "I=J; domain=.izzle")); EXPECT_EQ("", GetCookies(cm, GURL("http://a.izzle"))); - EXPECT_FALSE(cm->SetCookie(url_google_, "K=L; domain=.bla.www.google.izzle")); + EXPECT_FALSE(SetCookie(cm, url_google_, "K=L; domain=.bla.www.google.izzle")); EXPECT_EQ("C=D; E=F; G=H", GetCookies(cm, GURL("http://bla.www.google.izzle"))); EXPECT_EQ("A=B; C=D; E=F; G=H", GetCookies(cm, url_google_)); @@ -891,8 +1388,8 @@ TEST_F(CookieMonsterTest, DomainWithTrailingDotTest) { scoped_refptr<MockPersistentCookieStore> store( new MockPersistentCookieStore); scoped_refptr<CookieMonster> cm(new CookieMonster(store, NULL)); - EXPECT_FALSE(cm->SetCookie(url_google_, "a=1; domain=.www.google.com.")); - EXPECT_FALSE(cm->SetCookie(url_google_, "b=2; domain=.www.google.com..")); + EXPECT_FALSE(SetCookie(cm, url_google_, "a=1; domain=.www.google.com.")); + EXPECT_FALSE(SetCookie(cm, url_google_, "b=2; domain=.www.google.com..")); EXPECT_EQ("", GetCookies(cm, url_google_)); // Nothing was persisted to the backing store. @@ -910,10 +1407,10 @@ TEST_F(CookieMonsterTest, ValidSubdomainTest) { GURL url_cd("http://c.d.com"); GURL url_d("http://d.com"); - EXPECT_TRUE(cm->SetCookie(url_abcd, "a=1; domain=.a.b.c.d.com")); - EXPECT_TRUE(cm->SetCookie(url_abcd, "b=2; domain=.b.c.d.com")); - EXPECT_TRUE(cm->SetCookie(url_abcd, "c=3; domain=.c.d.com")); - EXPECT_TRUE(cm->SetCookie(url_abcd, "d=4; domain=.d.com")); + EXPECT_TRUE(SetCookie(cm, url_abcd, "a=1; domain=.a.b.c.d.com")); + EXPECT_TRUE(SetCookie(cm, url_abcd, "b=2; domain=.b.c.d.com")); + EXPECT_TRUE(SetCookie(cm, url_abcd, "c=3; domain=.c.d.com")); + EXPECT_TRUE(SetCookie(cm, url_abcd, "d=4; domain=.d.com")); EXPECT_EQ("a=1; b=2; c=3; d=4", GetCookies(cm, url_abcd)); EXPECT_EQ("b=2; c=3; d=4", GetCookies(cm, url_bcd)); @@ -921,8 +1418,8 @@ TEST_F(CookieMonsterTest, ValidSubdomainTest) { EXPECT_EQ("d=4", GetCookies(cm, url_d)); // Check that the same cookie can exist on different sub-domains. - EXPECT_TRUE(cm->SetCookie(url_bcd, "X=bcd; domain=.b.c.d.com")); - EXPECT_TRUE(cm->SetCookie(url_bcd, "X=cd; domain=.c.d.com")); + EXPECT_TRUE(SetCookie(cm, url_bcd, "X=bcd; domain=.b.c.d.com")); + EXPECT_TRUE(SetCookie(cm, url_bcd, "X=cd; domain=.c.d.com")); EXPECT_EQ("b=2; c=3; d=4; X=bcd; X=cd", GetCookies(cm, url_bcd)); EXPECT_EQ("c=3; d=4; X=cd", GetCookies(cm, url_cd)); @@ -943,31 +1440,31 @@ TEST_F(CookieMonsterTest, InvalidDomainTest) { GURL url_foobar("http://foo.bar.com"); // More specific sub-domain than allowed. - EXPECT_FALSE(cm->SetCookie(url_foobar, "a=1; domain=.yo.foo.bar.com")); + EXPECT_FALSE(SetCookie(cm, url_foobar, "a=1; domain=.yo.foo.bar.com")); - EXPECT_FALSE(cm->SetCookie(url_foobar, "b=2; domain=.foo.com")); - EXPECT_FALSE(cm->SetCookie(url_foobar, "c=3; domain=.bar.foo.com")); + EXPECT_FALSE(SetCookie(cm, url_foobar, "b=2; domain=.foo.com")); + EXPECT_FALSE(SetCookie(cm, url_foobar, "c=3; domain=.bar.foo.com")); // Different TLD, but the rest is a substring. - EXPECT_FALSE(cm->SetCookie(url_foobar, "d=4; domain=.foo.bar.com.net")); + EXPECT_FALSE(SetCookie(cm, url_foobar, "d=4; domain=.foo.bar.com.net")); // A substring that isn't really a parent domain. - EXPECT_FALSE(cm->SetCookie(url_foobar, "e=5; domain=ar.com")); + EXPECT_FALSE(SetCookie(cm, url_foobar, "e=5; domain=ar.com")); // Completely invalid domains: - EXPECT_FALSE(cm->SetCookie(url_foobar, "f=6; domain=.")); - EXPECT_FALSE(cm->SetCookie(url_foobar, "g=7; domain=/")); - EXPECT_FALSE(cm->SetCookie(url_foobar, "h=8; domain=http://foo.bar.com")); - EXPECT_FALSE(cm->SetCookie(url_foobar, "i=9; domain=..foo.bar.com")); - EXPECT_FALSE(cm->SetCookie(url_foobar, "j=10; domain=..bar.com")); + EXPECT_FALSE(SetCookie(cm, url_foobar, "f=6; domain=.")); + EXPECT_FALSE(SetCookie(cm, url_foobar, "g=7; domain=/")); + EXPECT_FALSE(SetCookie(cm, url_foobar, "h=8; domain=http://foo.bar.com")); + EXPECT_FALSE(SetCookie(cm, url_foobar, "i=9; domain=..foo.bar.com")); + EXPECT_FALSE(SetCookie(cm, url_foobar, "j=10; domain=..bar.com")); // Make sure there isn't something quirky in the domain canonicalization // that supports full URL semantics. - EXPECT_FALSE(cm->SetCookie(url_foobar, "k=11; domain=.foo.bar.com?blah")); - EXPECT_FALSE(cm->SetCookie(url_foobar, "l=12; domain=.foo.bar.com/blah")); - EXPECT_FALSE(cm->SetCookie(url_foobar, "m=13; domain=.foo.bar.com:80")); - EXPECT_FALSE(cm->SetCookie(url_foobar, "n=14; domain=.foo.bar.com:")); - EXPECT_FALSE(cm->SetCookie(url_foobar, "o=15; domain=.foo.bar.com#sup")); + EXPECT_FALSE(SetCookie(cm, url_foobar, "k=11; domain=.foo.bar.com?blah")); + EXPECT_FALSE(SetCookie(cm, url_foobar, "l=12; domain=.foo.bar.com/blah")); + EXPECT_FALSE(SetCookie(cm, url_foobar, "m=13; domain=.foo.bar.com:80")); + EXPECT_FALSE(SetCookie(cm, url_foobar, "n=14; domain=.foo.bar.com:")); + EXPECT_FALSE(SetCookie(cm, url_foobar, "o=15; domain=.foo.bar.com#sup")); EXPECT_EQ("", GetCookies(cm, url_foobar)); @@ -981,7 +1478,7 @@ TEST_F(CookieMonsterTest, InvalidDomainTest) { // hosts below have the same domain + registry. scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); GURL url_foocom("http://foo.com.com"); - EXPECT_FALSE(cm->SetCookie(url_foocom, "a=1; domain=.foo.com.com.com")); + EXPECT_FALSE(SetCookie(cm, url_foocom, "a=1; domain=.foo.com.com.com")); EXPECT_EQ("", GetCookies(cm, url_foocom)); } } @@ -994,7 +1491,7 @@ TEST_F(CookieMonsterTest, DomainWithoutLeadingDotTest) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); GURL url_hosted("http://manage.hosted.filefront.com"); GURL url_filefront("http://www.filefront.com"); - EXPECT_TRUE(cm->SetCookie(url_hosted, "sawAd=1; domain=filefront.com")); + EXPECT_TRUE(SetCookie(cm, url_hosted, "sawAd=1; domain=filefront.com")); EXPECT_EQ("sawAd=1", GetCookies(cm, url_hosted)); EXPECT_EQ("sawAd=1", GetCookies(cm, url_filefront)); } @@ -1002,7 +1499,7 @@ TEST_F(CookieMonsterTest, DomainWithoutLeadingDotTest) { { // Even when the domains match exactly, don't consider it host cookie. scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); GURL url("http://www.google.com"); - EXPECT_TRUE(cm->SetCookie(url, "a=1; domain=www.google.com")); + EXPECT_TRUE(SetCookie(cm, url, "a=1; domain=www.google.com")); EXPECT_EQ("a=1", GetCookies(cm, url)); EXPECT_EQ("a=1", GetCookies(cm, GURL("http://sub.www.google.com"))); EXPECT_EQ("", GetCookies(cm, GURL("http://something-else.com"))); @@ -1014,8 +1511,8 @@ TEST_F(CookieMonsterTest, DomainWithoutLeadingDotTest) { TEST_F(CookieMonsterTest, CaseInsensitiveDomainTest) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); GURL url("http://www.google.com"); - EXPECT_TRUE(cm->SetCookie(url, "a=1; domain=.GOOGLE.COM")); - EXPECT_TRUE(cm->SetCookie(url, "b=2; domain=.wWw.gOOgLE.coM")); + EXPECT_TRUE(SetCookie(cm, url, "a=1; domain=.GOOGLE.COM")); + EXPECT_TRUE(SetCookie(cm, url, "b=2; domain=.wWw.gOOgLE.coM")); EXPECT_EQ("a=1; b=2", GetCookies(cm, url)); } @@ -1023,20 +1520,20 @@ TEST_F(CookieMonsterTest, TestIpAddress) { GURL url_ip("http://1.2.3.4/weee"); { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); - EXPECT_TRUE(cm->SetCookie(url_ip, kValidCookieLine)); + EXPECT_TRUE(SetCookie(cm, url_ip, kValidCookieLine)); EXPECT_EQ("A=B", GetCookies(cm, url_ip)); } { // IP addresses should not be able to set domain cookies. scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); - EXPECT_FALSE(cm->SetCookie(url_ip, "b=2; domain=.1.2.3.4")); - EXPECT_FALSE(cm->SetCookie(url_ip, "c=3; domain=.3.4")); + EXPECT_FALSE(SetCookie(cm, url_ip, "b=2; domain=.1.2.3.4")); + EXPECT_FALSE(SetCookie(cm, url_ip, "c=3; domain=.3.4")); EXPECT_EQ("", GetCookies(cm, url_ip)); // It should be allowed to set a cookie if domain= matches the IP address // exactly. This matches IE/Firefox, even though it seems a bit wrong. - EXPECT_FALSE(cm->SetCookie(url_ip, "b=2; domain=1.2.3.3")); + EXPECT_FALSE(SetCookie(cm, url_ip, "b=2; domain=1.2.3.3")); EXPECT_EQ("", GetCookies(cm, url_ip)); - EXPECT_TRUE(cm->SetCookie(url_ip, "b=2; domain=1.2.3.4")); + EXPECT_TRUE(SetCookie(cm, url_ip, "b=2; domain=1.2.3.4")); EXPECT_EQ("b=2", GetCookies(cm, url_ip)); } } @@ -1047,9 +1544,9 @@ TEST_F(CookieMonsterTest, TestNonDottedAndTLD) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); GURL url("http://com/"); // Allow setting on "com", (but only as a host cookie). - EXPECT_TRUE(cm->SetCookie(url, "a=1")); - EXPECT_FALSE(cm->SetCookie(url, "b=2; domain=.com")); - EXPECT_FALSE(cm->SetCookie(url, "c=3; domain=com")); + EXPECT_TRUE(SetCookie(cm, url, "a=1")); + EXPECT_FALSE(SetCookie(cm, url, "b=2; domain=.com")); + EXPECT_FALSE(SetCookie(cm, url, "c=3; domain=com")); EXPECT_EQ("a=1", GetCookies(cm, url)); // Make sure it doesn't show up for a normal .com, it should be a host // not a domain cookie. @@ -1060,7 +1557,7 @@ TEST_F(CookieMonsterTest, TestNonDottedAndTLD) { { // http://com. should be treated the same as http://com. scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); GURL url("http://com./index.html"); - EXPECT_TRUE(cm->SetCookie(url, "a=1")); + EXPECT_TRUE(SetCookie(cm, url, "a=1")); EXPECT_EQ("a=1", GetCookies(cm, url)); EXPECT_EQ("", GetCookies(cm, GURL("http://hopefully-no-cookies.com./"))); } @@ -1068,24 +1565,24 @@ TEST_F(CookieMonsterTest, TestNonDottedAndTLD) { { // Should not be able to set host cookie from a subdomain. scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); GURL url("http://a.b"); - EXPECT_FALSE(cm->SetCookie(url, "a=1; domain=.b")); - EXPECT_FALSE(cm->SetCookie(url, "b=2; domain=b")); + EXPECT_FALSE(SetCookie(cm, url, "a=1; domain=.b")); + EXPECT_FALSE(SetCookie(cm, url, "b=2; domain=b")); EXPECT_EQ("", GetCookies(cm, url)); } { // Same test as above, but explicitly on a known TLD (com). scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); GURL url("http://google.com"); - EXPECT_FALSE(cm->SetCookie(url, "a=1; domain=.com")); - EXPECT_FALSE(cm->SetCookie(url, "b=2; domain=com")); + EXPECT_FALSE(SetCookie(cm, url, "a=1; domain=.com")); + EXPECT_FALSE(SetCookie(cm, url, "b=2; domain=com")); EXPECT_EQ("", GetCookies(cm, url)); } { // Make sure can't set cookie on TLD which is dotted. scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); GURL url("http://google.co.uk"); - EXPECT_FALSE(cm->SetCookie(url, "a=1; domain=.co.uk")); - EXPECT_FALSE(cm->SetCookie(url, "b=2; domain=.uk")); + EXPECT_FALSE(SetCookie(cm, url, "a=1; domain=.co.uk")); + EXPECT_FALSE(SetCookie(cm, url, "b=2; domain=.uk")); EXPECT_EQ("", GetCookies(cm, url)); EXPECT_EQ("", GetCookies(cm, GURL("http://something-else.co.uk"))); EXPECT_EQ("", GetCookies(cm, GURL("http://something-else.uk"))); @@ -1094,9 +1591,9 @@ TEST_F(CookieMonsterTest, TestNonDottedAndTLD) { { // Intranet URLs should only be able to set host cookies. scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); GURL url("http://b"); - EXPECT_TRUE(cm->SetCookie(url, "a=1")); - EXPECT_FALSE(cm->SetCookie(url, "b=2; domain=.b")); - EXPECT_FALSE(cm->SetCookie(url, "c=3; domain=b")); + EXPECT_TRUE(SetCookie(cm, url, "a=1")); + EXPECT_FALSE(SetCookie(cm, url, "b=2; domain=.b")); + EXPECT_FALSE(SetCookie(cm, url, "c=3; domain=b")); EXPECT_EQ("a=1", GetCookies(cm, url)); } } @@ -1107,15 +1604,15 @@ TEST_F(CookieMonsterTest, TestHostEndsWithDot) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); GURL url("http://www.google.com"); GURL url_with_dot("http://www.google.com."); - EXPECT_TRUE(cm->SetCookie(url, "a=1")); + EXPECT_TRUE(SetCookie(cm, url, "a=1")); EXPECT_EQ("a=1", GetCookies(cm, url)); // Do not share cookie space with the dot version of domain. // Note: this is not what FireFox does, but it _is_ what IE+Safari do. - EXPECT_FALSE(cm->SetCookie(url, "b=2; domain=.www.google.com.")); + EXPECT_FALSE(SetCookie(cm, url, "b=2; domain=.www.google.com.")); EXPECT_EQ("a=1", GetCookies(cm, url)); - EXPECT_TRUE(cm->SetCookie(url_with_dot, "b=2; domain=.google.com.")); + EXPECT_TRUE(SetCookie(cm, url_with_dot, "b=2; domain=.google.com.")); EXPECT_EQ("b=2", GetCookies(cm, url_with_dot)); // Make sure there weren't any side effects. @@ -1125,19 +1622,19 @@ TEST_F(CookieMonsterTest, TestHostEndsWithDot) { TEST_F(CookieMonsterTest, InvalidScheme) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); - EXPECT_FALSE(cm->SetCookie(GURL(kUrlFtp), kValidCookieLine)); + EXPECT_FALSE(SetCookie(cm, GURL(kUrlFtp), kValidCookieLine)); } TEST_F(CookieMonsterTest, InvalidScheme_Read) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); - EXPECT_TRUE(cm->SetCookie(GURL(kUrlGoogle), kValidDomainCookieLine)); + EXPECT_TRUE(SetCookie(cm, GURL(kUrlGoogle), kValidDomainCookieLine)); EXPECT_EQ("", GetCookies(cm, GURL(kUrlFtp))); } TEST_F(CookieMonsterTest, PathTest) { std::string url("http://www.google.izzle"); scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); - EXPECT_TRUE(cm->SetCookie(GURL(url), "A=B; path=/wee")); + EXPECT_TRUE(SetCookie(cm, GURL(url), "A=B; path=/wee")); EXPECT_EQ("A=B", GetCookies(cm, GURL(url + "/wee"))); EXPECT_EQ("A=B", GetCookies(cm, GURL(url + "/wee/"))); EXPECT_EQ("A=B", GetCookies(cm, GURL(url + "/wee/war"))); @@ -1146,7 +1643,7 @@ TEST_F(CookieMonsterTest, PathTest) { EXPECT_EQ("", GetCookies(cm, GURL(url + "/"))); // If we add a 0 length path, it should default to / - EXPECT_TRUE(cm->SetCookie(GURL(url), "A=C; path=")); + EXPECT_TRUE(SetCookie(cm, GURL(url), "A=C; path=")); EXPECT_EQ("A=B; A=C", GetCookies(cm, GURL(url + "/wee"))); EXPECT_EQ("A=C", GetCookies(cm, GURL(url + "/"))); } @@ -1164,14 +1661,14 @@ TEST_F(CookieMonsterTest, HttpOnlyTest) { EXPECT_EQ("A=B", GetCookiesWithOptions(cm, url_google_, options)); // Check httponly overwrite protection. - EXPECT_FALSE(cm->SetCookie(url_google_, "A=C")); + EXPECT_FALSE(SetCookie(cm, url_google_, "A=C")); EXPECT_EQ("", GetCookies(cm, url_google_)); EXPECT_EQ("A=B", GetCookiesWithOptions(cm, url_google_, options)); EXPECT_TRUE(SetCookieWithOptions(cm, url_google_, "A=C", options)); EXPECT_EQ("A=C", GetCookies(cm, url_google_)); // Check httponly create protection. - EXPECT_FALSE(cm->SetCookie(url_google_, "B=A; httponly")); + EXPECT_FALSE(SetCookie(cm, url_google_, "B=A; httponly")); EXPECT_EQ("A=C", GetCookiesWithOptions(cm, url_google_, options)); EXPECT_TRUE(SetCookieWithOptions(cm, url_google_, "B=A; httponly", options)); EXPECT_EQ("A=C; B=A", GetCookiesWithOptions(cm, url_google_, options)); @@ -1324,24 +1821,24 @@ TEST_F(CookieMonsterTest, TestCookieDeletion) { scoped_refptr<CookieMonster> cm(new CookieMonster(store, NULL)); // Create a session cookie. - EXPECT_TRUE(cm->SetCookie(url_google_, kValidCookieLine)); + EXPECT_TRUE(SetCookie(cm, url_google_, kValidCookieLine)); EXPECT_EQ("A=B", GetCookies(cm, url_google_)); // Delete it via Max-Age. - EXPECT_TRUE(cm->SetCookie(url_google_, + EXPECT_TRUE(SetCookie(cm, url_google_, std::string(kValidCookieLine) + "; max-age=0")); EXPECT_EQ("", GetCookies(cm, url_google_)); // Create a session cookie. - EXPECT_TRUE(cm->SetCookie(url_google_, kValidCookieLine)); + EXPECT_TRUE(SetCookie(cm, url_google_, kValidCookieLine)); EXPECT_EQ("A=B", GetCookies(cm, url_google_)); // Delete it via Expires. - EXPECT_TRUE(cm->SetCookie(url_google_, + EXPECT_TRUE(SetCookie(cm, url_google_, std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-1977 22:50:13 GMT")); EXPECT_EQ("", GetCookies(cm, url_google_)); // Create a persistent cookie. - EXPECT_TRUE(cm->SetCookie(url_google_, + EXPECT_TRUE(SetCookie(cm, url_google_, std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-22 22:50:13 GMT")); ASSERT_EQ(1u, store->commands().size()); @@ -1349,38 +1846,38 @@ TEST_F(CookieMonsterTest, TestCookieDeletion) { EXPECT_EQ("A=B", GetCookies(cm, url_google_)); // Delete it via Max-Age. - EXPECT_TRUE(cm->SetCookie(url_google_, + EXPECT_TRUE(SetCookie(cm, url_google_, std::string(kValidCookieLine) + "; max-age=0")); ASSERT_EQ(2u, store->commands().size()); EXPECT_EQ(CookieStoreCommand::REMOVE, store->commands()[1].type); EXPECT_EQ("", GetCookies(cm, url_google_)); // Create a persistent cookie. - EXPECT_TRUE(cm->SetCookie(url_google_, - std::string(kValidCookieLine) + - "; expires=Mon, 18-Apr-22 22:50:13 GMT")); + EXPECT_TRUE(SetCookie(cm, url_google_, + std::string(kValidCookieLine) + + "; expires=Mon, 18-Apr-22 22:50:13 GMT")); ASSERT_EQ(3u, store->commands().size()); EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[2].type); EXPECT_EQ("A=B", GetCookies(cm, url_google_)); // Delete it via Expires. - EXPECT_TRUE(cm->SetCookie(url_google_, - std::string(kValidCookieLine) + - "; expires=Mon, 18-Apr-1977 22:50:13 GMT")); + EXPECT_TRUE(SetCookie(cm, url_google_, + std::string(kValidCookieLine) + + "; expires=Mon, 18-Apr-1977 22:50:13 GMT")); ASSERT_EQ(4u, store->commands().size()); EXPECT_EQ(CookieStoreCommand::REMOVE, store->commands()[3].type); EXPECT_EQ("", GetCookies(cm, url_google_)); // Create a persistent cookie. - EXPECT_TRUE(cm->SetCookie(url_google_, - std::string(kValidCookieLine) + - "; expires=Mon, 18-Apr-22 22:50:13 GMT")); + EXPECT_TRUE(SetCookie(cm, url_google_, + std::string(kValidCookieLine) + + "; expires=Mon, 18-Apr-22 22:50:13 GMT")); ASSERT_EQ(5u, store->commands().size()); EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[4].type); EXPECT_EQ("A=B", GetCookies(cm, url_google_)); // Delete it via Expires, with a unix epoch of 0. - EXPECT_TRUE(cm->SetCookie(url_google_, - std::string(kValidCookieLine) + - "; expires=Thu, 1-Jan-1970 00:00:00 GMT")); + EXPECT_TRUE(SetCookie(cm, url_google_, + std::string(kValidCookieLine) + + "; expires=Thu, 1-Jan-1970 00:00:00 GMT")); ASSERT_EQ(6u, store->commands().size()); EXPECT_EQ(CookieStoreCommand::REMOVE, store->commands()[5].type); EXPECT_EQ("", GetCookies(cm, url_google_)); @@ -1393,25 +1890,25 @@ TEST_F(CookieMonsterTest, TestCookieDeleteAll) { CookieOptions options; options.set_include_httponly(); - EXPECT_TRUE(cm->SetCookie(url_google_, kValidCookieLine)); + EXPECT_TRUE(SetCookie(cm, url_google_, kValidCookieLine)); EXPECT_EQ("A=B", GetCookies(cm, url_google_)); EXPECT_TRUE(SetCookieWithOptions(cm, url_google_, "C=D; httponly", options)); EXPECT_EQ("A=B; C=D", GetCookiesWithOptions(cm, url_google_, options)); - EXPECT_EQ(2, cm->DeleteAll(false)); + EXPECT_EQ(2, DeleteAll(cm)); EXPECT_EQ("", GetCookiesWithOptions(cm, url_google_, options)); EXPECT_EQ(0u, store->commands().size()); // Create a persistent cookie. - EXPECT_TRUE(cm->SetCookie(url_google_, - std::string(kValidCookieLine) + - "; expires=Mon, 18-Apr-22 22:50:13 GMT")); + EXPECT_TRUE(SetCookie(cm, url_google_, + std::string(kValidCookieLine) + + "; expires=Mon, 18-Apr-22 22:50:13 GMT")); ASSERT_EQ(1u, store->commands().size()); EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[0].type); - EXPECT_EQ(1, cm->DeleteAll(true)); // sync_to_store = true. + EXPECT_EQ(1, DeleteAll(cm)); // sync_to_store = true. ASSERT_EQ(2u, store->commands().size()); EXPECT_EQ(CookieStoreCommand::REMOVE, store->commands()[1].type); @@ -1424,59 +1921,56 @@ TEST_F(CookieMonsterTest, TestCookieDeleteAllCreatedBetweenTimestamps) { // Nothing has been added so nothing should be deleted. EXPECT_EQ(0, DeleteAllCreatedBetween(cm, now - TimeDelta::FromDays(99), - Time(), false)); + Time())); // Create 3 cookies with creation date of today, yesterday and the day before. EXPECT_TRUE(cm->SetCookieWithCreationTime(url_google_, "T-0=Now", now)); EXPECT_TRUE(cm->SetCookieWithCreationTime(url_google_, "T-1=Yesterday", - now - TimeDelta::FromDays(1))); + now - TimeDelta::FromDays(1))); EXPECT_TRUE(cm->SetCookieWithCreationTime(url_google_, "T-2=DayBefore", - now - TimeDelta::FromDays(2))); + now - TimeDelta::FromDays(2))); EXPECT_TRUE(cm->SetCookieWithCreationTime(url_google_, "T-3=ThreeDays", - now - TimeDelta::FromDays(3))); + now - TimeDelta::FromDays(3))); EXPECT_TRUE(cm->SetCookieWithCreationTime(url_google_, "T-7=LastWeek", - now - TimeDelta::FromDays(7))); + now - TimeDelta::FromDays(7))); // Try to delete threedays and the daybefore. EXPECT_EQ(2, DeleteAllCreatedBetween(cm, now - TimeDelta::FromDays(3), - now - TimeDelta::FromDays(1), - false)); + now - TimeDelta::FromDays(1))); // Try to delete yesterday, also make sure that delete_end is not // inclusive. EXPECT_EQ(1, DeleteAllCreatedBetween(cm, now - TimeDelta::FromDays(2), - now, - false)); + now)); // Make sure the delete_begin is inclusive. EXPECT_EQ(1, DeleteAllCreatedBetween(cm, now - TimeDelta::FromDays(7), - now, - false)); + now)); // Delete the last (now) item. - EXPECT_EQ(1, DeleteAllCreatedBetween(cm, Time(), Time(), false)); + EXPECT_EQ(1, DeleteAllCreatedBetween(cm, Time(), Time())); // Really make sure everything is gone. - EXPECT_EQ(0, cm->DeleteAll(false)); + EXPECT_EQ(0, DeleteAll(cm)); } TEST_F(CookieMonsterTest, TestSecure) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); - EXPECT_TRUE(cm->SetCookie(url_google_, "A=B")); + EXPECT_TRUE(SetCookie(cm, url_google_, "A=B")); EXPECT_EQ("A=B", GetCookies(cm, url_google_)); EXPECT_EQ("A=B", GetCookies(cm, url_google_secure_)); - EXPECT_TRUE(cm->SetCookie(url_google_secure_, "A=B; secure")); + EXPECT_TRUE(SetCookie(cm, url_google_secure_, "A=B; secure")); // The secure should overwrite the non-secure. EXPECT_EQ("", GetCookies(cm, url_google_)); EXPECT_EQ("A=B", GetCookies(cm, url_google_secure_)); - EXPECT_TRUE(cm->SetCookie(url_google_secure_, "D=E; secure")); + EXPECT_TRUE(SetCookie(cm, url_google_secure_, "D=E; secure")); EXPECT_EQ("", GetCookies(cm, url_google_)); EXPECT_EQ("A=B; D=E", GetCookies(cm, url_google_secure_)); - EXPECT_TRUE(cm->SetCookie(url_google_secure_, "A=B")); + EXPECT_TRUE(SetCookie(cm, url_google_secure_, "A=B")); // The non-secure should overwrite the secure. EXPECT_EQ("A=B", GetCookies(cm, url_google_)); EXPECT_EQ("D=E; A=B", GetCookies(cm, url_google_secure_)); @@ -1488,7 +1982,7 @@ TEST_F(CookieMonsterTest, TestLastAccess) { scoped_refptr<CookieMonster> cm( new CookieMonster(NULL, NULL, kLastAccessThresholdMilliseconds)); - EXPECT_TRUE(cm->SetCookie(url_google_, "A=B")); + EXPECT_TRUE(SetCookie(cm, url_google_, "A=B")); const Time last_access_date(GetFirstCookieAccessDate(cm)); // Reading the cookie again immediately shouldn't update the access date, @@ -1517,13 +2011,13 @@ TEST_F(CookieMonsterTest, NetUtilCookieTest) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); - EXPECT_TRUE(cm->SetCookie(test_url, "foo=bar")); + EXPECT_TRUE(SetCookie(cm, test_url, "foo=bar")); std::string value = GetCookies(cm, test_url); EXPECT_EQ("foo=bar", value); // test that we can retrieve all cookies: - EXPECT_TRUE(cm->SetCookie(test_url, "x=1")); - EXPECT_TRUE(cm->SetCookie(test_url, "y=2")); + EXPECT_TRUE(SetCookie(cm, test_url, "x=1")); + EXPECT_TRUE(SetCookie(cm, test_url, "y=2")); std::string result = GetCookies(cm, test_url); EXPECT_FALSE(result.empty()); @@ -1534,9 +2028,9 @@ TEST_F(CookieMonsterTest, NetUtilCookieTest) { TEST_F(CookieMonsterTest, TestDeleteSingleCookie) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); - EXPECT_TRUE(cm->SetCookie(url_google_, "A=B")); - EXPECT_TRUE(cm->SetCookie(url_google_, "C=D")); - EXPECT_TRUE(cm->SetCookie(url_google_, "E=F")); + EXPECT_TRUE(SetCookie(cm, url_google_, "A=B")); + EXPECT_TRUE(SetCookie(cm, url_google_, "C=D")); + EXPECT_TRUE(SetCookie(cm, url_google_, "E=F")); EXPECT_EQ("A=B; C=D; E=F", GetCookies(cm, url_google_)); EXPECT_TRUE(FindAndDeleteCookie(cm, url_google_.host(), "C")); @@ -1557,10 +2051,10 @@ TEST_F(CookieMonsterTest, SetCookieableSchemes) { GURL foo_url("foo://host/path"); GURL http_url("http://host/path"); - EXPECT_TRUE(cm->SetCookie(http_url, "x=1")); - EXPECT_FALSE(cm->SetCookie(foo_url, "x=1")); - EXPECT_TRUE(cm_foo->SetCookie(foo_url, "x=1")); - EXPECT_FALSE(cm_foo->SetCookie(http_url, "x=1")); + EXPECT_TRUE(SetCookie(cm, http_url, "x=1")); + EXPECT_FALSE(SetCookie(cm, foo_url, "x=1")); + EXPECT_TRUE(SetCookie(cm_foo, foo_url, "x=1")); + EXPECT_FALSE(SetCookie(cm_foo, http_url, "x=1")); } TEST_F(CookieMonsterTest, GetAllCookiesForURL) { @@ -1573,11 +2067,11 @@ TEST_F(CookieMonsterTest, GetAllCookiesForURL) { EXPECT_TRUE(SetCookieWithOptions(cm, url_google_, "A=B; httponly", options)); EXPECT_TRUE(SetCookieWithOptions(cm, url_google_, - "C=D; domain=.google.izzle", - options)); + "C=D; domain=.google.izzle", + options)); EXPECT_TRUE(SetCookieWithOptions(cm, url_google_secure_, - "E=F; domain=.google.izzle; secure", - options)); + "E=F; domain=.google.izzle; secure", + options)); const Time last_access_date(GetFirstCookieAccessDate(cm)); @@ -1635,14 +2129,11 @@ TEST_F(CookieMonsterTest, GetAllCookiesForURLPathMatching) { CookieOptions options; EXPECT_TRUE(SetCookieWithOptions(cm, url_google_foo_, - "A=B; path=/foo;", - options)); + "A=B; path=/foo;", options)); EXPECT_TRUE(SetCookieWithOptions(cm, url_google_bar_, - "C=D; path=/bar;", - options)); + "C=D; path=/bar;", options)); EXPECT_TRUE(SetCookieWithOptions(cm, url_google_, - "E=F;", - options)); + "E=F;", options)); CookieList cookies = GetAllCookiesForURL(cm, url_google_foo_); CookieList::iterator it = cookies.begin(); @@ -1674,12 +2165,12 @@ TEST_F(CookieMonsterTest, GetAllCookiesForURLPathMatching) { TEST_F(CookieMonsterTest, DeleteCookieByName) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); - EXPECT_TRUE(cm->SetCookie(url_google_, "A=A1; path=/")); - EXPECT_TRUE(cm->SetCookie(url_google_, "A=A2; path=/foo")); - EXPECT_TRUE(cm->SetCookie(url_google_, "A=A3; path=/bar")); - EXPECT_TRUE(cm->SetCookie(url_google_, "B=B1; path=/")); - EXPECT_TRUE(cm->SetCookie(url_google_, "B=B2; path=/foo")); - EXPECT_TRUE(cm->SetCookie(url_google_, "B=B3; path=/bar")); + EXPECT_TRUE(SetCookie(cm, url_google_, "A=A1; path=/")); + EXPECT_TRUE(SetCookie(cm, url_google_, "A=A2; path=/foo")); + EXPECT_TRUE(SetCookie(cm, url_google_, "A=A3; path=/bar")); + EXPECT_TRUE(SetCookie(cm, url_google_, "B=B1; path=/")); + EXPECT_TRUE(SetCookie(cm, url_google_, "B=B2; path=/foo")); + EXPECT_TRUE(SetCookie(cm, url_google_, "B=B3; path=/bar")); DeleteCookie(cm, GURL(std::string(kUrlGoogle) + "/foo/bar"), "A"); @@ -1742,15 +2233,15 @@ TEST_F(CookieMonsterTest, OverwritePersistentCookie) { // Insert a cookie "a" for path "/path1" EXPECT_TRUE( - cm->SetCookie(url_google, "a=val1; path=/path1; " - "expires=Mon, 18-Apr-22 22:50:13 GMT")); + SetCookie(cm, url_google, "a=val1; path=/path1; " + "expires=Mon, 18-Apr-22 22:50:13 GMT")); ASSERT_EQ(1u, store->commands().size()); EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[0].type); // Insert a cookie "b" for path "/path1" EXPECT_TRUE( - cm->SetCookie(url_google, "b=val1; path=/path1; " - "expires=Mon, 18-Apr-22 22:50:14 GMT")); + SetCookie(cm, url_google, "b=val1; path=/path1; " + "expires=Mon, 18-Apr-22 22:50:14 GMT")); ASSERT_EQ(2u, store->commands().size()); EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[1].type); @@ -1760,35 +2251,35 @@ TEST_F(CookieMonsterTest, OverwritePersistentCookie) { allow_httponly.set_include_httponly(); EXPECT_TRUE( SetCookieWithOptions(cm, url_google, - "b=val2; path=/path1; httponly; " - "expires=Mon, 18-Apr-22 22:50:14 GMT", - allow_httponly)); + "b=val2; path=/path1; httponly; " + "expires=Mon, 18-Apr-22 22:50:14 GMT", + allow_httponly)); ASSERT_EQ(4u, store->commands().size()); EXPECT_EQ(CookieStoreCommand::REMOVE, store->commands()[2].type); EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[3].type); // Insert a cookie "a" for path "/path1". This should overwrite. - EXPECT_TRUE(cm->SetCookie(url_google, - "a=val33; path=/path1; " - "expires=Mon, 18-Apr-22 22:50:14 GMT")); + EXPECT_TRUE(SetCookie(cm, url_google, + "a=val33; path=/path1; " + "expires=Mon, 18-Apr-22 22:50:14 GMT")); ASSERT_EQ(6u, store->commands().size()); EXPECT_EQ(CookieStoreCommand::REMOVE, store->commands()[4].type); EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[5].type); // Insert a cookie "a" for path "/path2". This should NOT overwrite // cookie "a", since the path is different. - EXPECT_TRUE(cm->SetCookie(url_google, - "a=val9; path=/path2; " - "expires=Mon, 18-Apr-22 22:50:14 GMT")); + EXPECT_TRUE(SetCookie(cm, url_google, + "a=val9; path=/path2; " + "expires=Mon, 18-Apr-22 22:50:14 GMT")); ASSERT_EQ(7u, store->commands().size()); EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[6].type); // Insert a cookie "a" for path "/path1", but this time for "chromium.org". // Although the name and path match, the hostnames do not, so shouldn't // overwrite. - EXPECT_TRUE(cm->SetCookie(url_chromium, - "a=val99; path=/path1; " - "expires=Mon, 18-Apr-22 22:50:14 GMT")); + EXPECT_TRUE(SetCookie(cm, url_chromium, + "a=val99; path=/path1; " + "expires=Mon, 18-Apr-22 22:50:14 GMT")); ASSERT_EQ(8u, store->commands().size()); EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[7].type); @@ -1929,9 +2420,9 @@ TEST_F(CookieMonsterTest, Delegate) { new MockCookieMonsterDelegate); scoped_refptr<CookieMonster> cm(new CookieMonster(store, delegate)); - EXPECT_TRUE(cm->SetCookie(url_google_, "A=B")); - EXPECT_TRUE(cm->SetCookie(url_google_, "C=D")); - EXPECT_TRUE(cm->SetCookie(url_google_, "E=F")); + EXPECT_TRUE(SetCookie(cm, url_google_, "A=B")); + EXPECT_TRUE(SetCookie(cm, url_google_, "C=D")); + EXPECT_TRUE(SetCookie(cm, url_google_, "E=F")); EXPECT_EQ("A=B; C=D; E=F", GetCookies(cm, url_google_)); ASSERT_EQ(3u, delegate->changes().size()); EXPECT_FALSE(delegate->changes()[0].second); @@ -1963,7 +2454,7 @@ TEST_F(CookieMonsterTest, Delegate) { // Insert a cookie "a" for path "/path1" EXPECT_TRUE( - cm->SetCookie(url_google_, "a=val1; path=/path1; " + SetCookie(cm, url_google_, "a=val1; path=/path1; " "expires=Mon, 18-Apr-22 22:50:13 GMT")); ASSERT_EQ(1u, store->commands().size()); EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[0].type); @@ -1980,9 +2471,9 @@ TEST_F(CookieMonsterTest, Delegate) { allow_httponly.set_include_httponly(); EXPECT_TRUE( SetCookieWithOptions(cm, url_google_, - "a=val2; path=/path1; httponly; " - "expires=Mon, 18-Apr-22 22:50:14 GMT", - allow_httponly)); + "a=val2; path=/path1; httponly; " + "expires=Mon, 18-Apr-22 22:50:14 GMT", + allow_httponly)); ASSERT_EQ(3u, store->commands().size()); EXPECT_EQ(CookieStoreCommand::REMOVE, store->commands()[1].type); EXPECT_EQ(CookieStoreCommand::ADD, store->commands()[2].type); @@ -2143,40 +2634,22 @@ TEST_F(CookieMonsterTest, UniqueCreationTime) { // are not included as they aren't going to be public for very much // longer. - // SetCookie, SetCookies, SetCookiesWithOptions, - // SetCookieWithOptions, SetCookieWithDetails + // SetCookie, SetCookieWithOptions, SetCookieWithDetails - cm->SetCookie(url_google_, "SetCookie1=A"); - cm->SetCookie(url_google_, "SetCookie2=A"); - cm->SetCookie(url_google_, "SetCookie3=A"); - - { - std::vector<std::string> cookie_lines; - cookie_lines.push_back("setCookies1=A"); - cookie_lines.push_back("setCookies2=A"); - cookie_lines.push_back("setCookies4=A"); - cm->SetCookies(url_google_, cookie_lines); - } - - { - std::vector<std::string> cookie_lines; - cookie_lines.push_back("setCookiesWithOptions1=A"); - cookie_lines.push_back("setCookiesWithOptions2=A"); - cookie_lines.push_back("setCookiesWithOptions3=A"); - - cm->SetCookiesWithOptions(url_google_, cookie_lines, options); - } + SetCookie(cm, url_google_, "SetCookie1=A"); + SetCookie(cm, url_google_, "SetCookie2=A"); + SetCookie(cm, url_google_, "SetCookie3=A"); SetCookieWithOptions(cm, url_google_, "setCookieWithOptions1=A", options); SetCookieWithOptions(cm, url_google_, "setCookieWithOptions2=A", options); SetCookieWithOptions(cm, url_google_, "setCookieWithOptions3=A", options); SetCookieWithDetails(cm, url_google_, "setCookieWithDetails1", "A", - ".google.com", "/", Time(), false, false); + ".google.com", "/", Time(), false, false); SetCookieWithDetails(cm, url_google_, "setCookieWithDetails2", "A", - ".google.com", "/", Time(), false, false); + ".google.com", "/", Time(), false, false); SetCookieWithDetails(cm, url_google_, "setCookieWithDetails3", "A", - ".google.com", "/", Time(), false, false); + ".google.com", "/", Time(), false, false); // Now we check CookieList cookie_list(GetAllCookies(cm)); @@ -2263,9 +2736,9 @@ TEST_F(CookieMonsterTest, BackingStoreCommunication) { scoped_refptr<CookieMonster> cmout(new CookieMonster(store, NULL)); for (const CookiesInputInfo* p = input_info; 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)); + EXPECT_TRUE(SetCookieWithDetails(cmout, GURL(p->gurl), p->name, p->value, + p->domain, p->path, p->expires, + p->secure, p->http_only)); } DeleteCookie(cmout, GURL(std::string(input_info[INPUT_DELETE].gurl) + input_info[INPUT_DELETE].path), @@ -2304,26 +2777,26 @@ TEST_F(CookieMonsterTest, CookieOrdering) { // Put a random set of cookies into a monster and make sure // they're returned in the right order. scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); - EXPECT_TRUE(cm->SetCookie(GURL("http://d.c.b.a.google.com/aa/x.html"), - "c=1")); - EXPECT_TRUE(cm->SetCookie(GURL("http://b.a.google.com/aa/bb/cc/x.html"), - "d=1; domain=b.a.google.com")); - EXPECT_TRUE(cm->SetCookie(GURL("http://b.a.google.com/aa/bb/cc/x.html"), - "a=4; domain=b.a.google.com")); - EXPECT_TRUE(cm->SetCookie(GURL("http://c.b.a.google.com/aa/bb/cc/x.html"), - "e=1; domain=c.b.a.google.com")); - EXPECT_TRUE(cm->SetCookie(GURL("http://d.c.b.a.google.com/aa/bb/x.html"), - "b=1")); - EXPECT_TRUE(cm->SetCookie(GURL("http://news.bbc.co.uk/midpath/x.html"), - "g=10")); + EXPECT_TRUE(SetCookie(cm, GURL("http://d.c.b.a.google.com/aa/x.html"), + "c=1")); + EXPECT_TRUE(SetCookie(cm, GURL("http://b.a.google.com/aa/bb/cc/x.html"), + "d=1; domain=b.a.google.com")); + EXPECT_TRUE(SetCookie(cm, GURL("http://b.a.google.com/aa/bb/cc/x.html"), + "a=4; domain=b.a.google.com")); + EXPECT_TRUE(SetCookie(cm, GURL("http://c.b.a.google.com/aa/bb/cc/x.html"), + "e=1; domain=c.b.a.google.com")); + EXPECT_TRUE(SetCookie(cm, GURL("http://d.c.b.a.google.com/aa/bb/x.html"), + "b=1")); + EXPECT_TRUE(SetCookie(cm, GURL("http://news.bbc.co.uk/midpath/x.html"), + "g=10")); EXPECT_EQ("d=1; a=4; e=1; b=1; c=1", - GetCookiesWithOptions(cm, - GURL("http://d.c.b.a.google.com/aa/bb/cc/dd"), + GetCookiesWithOptions( + cm, GURL("http://d.c.b.a.google.com/aa/bb/cc/dd"), CookieOptions())); { unsigned int i = 0; - CookieList cookies(cm->GetAllCookiesForURL( - GURL("http://d.c.b.a.google.com/aa/bb/cc/dd"))); + CookieList cookies(GetAllCookiesForURL( + cm, GURL("http://d.c.b.a.google.com/aa/bb/cc/dd"))); ASSERT_EQ(5u, cookies.size()); EXPECT_EQ("d", cookies[i++].Name()); EXPECT_EQ("a", cookies[i++].Name()); @@ -2345,16 +2818,6 @@ TEST_F(CookieMonsterTest, CookieOrdering) { } } - -// Function for creating a CM with a number of cookies in it, -// no store (and hence no ability to affect access time). -static CookieMonster* CreateMonsterForGC(int num_cookies) { - CookieMonster* cm(new 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 @@ -2368,7 +2831,7 @@ TEST_F(CookieMonsterTest, GarbageCollectionTriggers) { scoped_refptr<CookieMonster> cm( CreateMonsterForGC(CookieMonster::kMaxCookies * 2)); EXPECT_EQ(CookieMonster::kMaxCookies * 2, GetAllCookies(cm).size()); - cm->SetCookie(GURL("http://newdomain.com"), "b=2"); + SetCookie(cm, GURL("http://newdomain.com"), "b=2"); EXPECT_EQ(CookieMonster::kMaxCookies * 2 + 1, GetAllCookies(cm).size()); } @@ -2433,7 +2896,7 @@ TEST_F(CookieMonsterTest, GarbageCollectionTriggers) { static_cast<int>(GetAllCookies(cm).size())) << "For test case " << ci; // Will trigger GC - cm->SetCookie(GURL("http://newdomain.com"), "b=2"); + SetCookie(cm, GURL("http://newdomain.com"), "b=2"); EXPECT_EQ(test_case->expected_cookies_after_set[recent_scheme], static_cast<int>((GetAllCookies(cm).size()))) << "For test case (" << ci << ", " << recent_scheme << ")"; @@ -2449,7 +2912,8 @@ TEST_F(CookieMonsterTest, ForceSessionOnly) { // Set a persistent cookie, but force it to be a session cookie. options.set_force_session(); - ASSERT_TRUE(SetCookieWithOptions(cm, url_google_, + ASSERT_TRUE(SetCookieWithOptions( + cm, url_google_, std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-22 22:50:13 GMT", options)); @@ -2459,7 +2923,8 @@ TEST_F(CookieMonsterTest, ForceSessionOnly) { ASSERT_FALSE(cookie_list[0].IsPersistent()); // Use a past expiry date to delete the cookie, but force it to session only. - ASSERT_TRUE(SetCookieWithOptions(cm, url_google_, + ASSERT_TRUE(SetCookieWithOptions( + cm, url_google_, std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-1977 22:50:13 GMT", options)); @@ -2475,7 +2940,8 @@ TEST_F(CookieMonsterTest, KeepExpiredCookies) { CookieOptions options; // Set a persistent cookie. - ASSERT_TRUE(SetCookieWithOptions(cm, url_google_, + ASSERT_TRUE(SetCookieWithOptions( + cm, url_google_, std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-22 22:50:13 GMT", options)); @@ -2484,7 +2950,8 @@ TEST_F(CookieMonsterTest, KeepExpiredCookies) { ASSERT_EQ(1U, cookie_list.size()); // Use a past expiry date to delete the cookie. - ASSERT_TRUE(SetCookieWithOptions(cm, url_google_, + ASSERT_TRUE(SetCookieWithOptions( + cm, url_google_, std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-1977 22:50:13 GMT", options)); @@ -2502,7 +2969,9 @@ class FlushablePersistentStore : public CookieMonster::PersistentCookieStore { public: FlushablePersistentStore() : flush_count_(0) {} - bool Load(std::vector<CookieMonster::CanonicalCookie*>*) { + bool Load(const LoadedCallback& loaded_callback) { + std::vector<CookieMonster::CanonicalCookie*> out_cookies; + loaded_callback.Run(out_cookies); return false; } @@ -2652,7 +3121,7 @@ TEST_F(CookieMonsterTest, HistogramCheck) { histogram_set_2.TotalCount()); // kValidCookieLine creates a session cookie. - ASSERT_TRUE(cm->SetCookie(url_google_, kValidCookieLine)); + ASSERT_TRUE(SetCookie(cm, url_google_, kValidCookieLine)); expired_histogram->SnapshotSample(&histogram_set_1); EXPECT_EQ(histogram_set_2.TotalCount(), histogram_set_1.TotalCount()); @@ -2670,8 +3139,8 @@ class MultiThreadedCookieMonsterTest : public CookieMonsterTest { void GetCookiesTask(CookieMonster* cm, const GURL& url, GetCookieStringCallback* callback) { - cm->GetCookiesAsync( - url, + cm->GetCookiesWithOptionsAsync( + url, CookieOptions(), base::Bind(&GetCookieStringCallback::Run, base::Unretained(callback))); } @@ -2755,10 +3224,9 @@ class MultiThreadedCookieMonsterTest : public CookieMonsterTest { void DeleteAllCreatedBetweenTask(CookieMonster* cm, const base::Time& delete_begin, const base::Time& delete_end, - bool sync_to_store, DeleteCallback* callback) { cm->DeleteAllCreatedBetweenAsync( - delete_begin, delete_end, sync_to_store, + delete_begin, delete_end, base::Bind(&DeleteCallback::Run, base::Unretained(callback))); } @@ -2798,7 +3266,7 @@ class MultiThreadedCookieMonsterTest : public CookieMonsterTest { TEST_F(MultiThreadedCookieMonsterTest, ThreadCheckGetCookies) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); - EXPECT_TRUE(cm->SetCookie(url_google_, "A=B")); + EXPECT_TRUE(SetCookie(cm, url_google_, "A=B")); EXPECT_EQ("A=B", GetCookies(cm, url_google_)); GetCookieStringCallback callback(&other_thread_); base::Closure task = base::Bind( @@ -2813,7 +3281,7 @@ TEST_F(MultiThreadedCookieMonsterTest, ThreadCheckGetCookies) { TEST_F(MultiThreadedCookieMonsterTest, ThreadCheckGetCookiesWithOptions) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); CookieOptions options; - EXPECT_TRUE(cm->SetCookie(url_google_, "A=B")); + EXPECT_TRUE(SetCookie(cm, url_google_, "A=B")); EXPECT_EQ("A=B", GetCookiesWithOptions(cm, url_google_, options)); GetCookieStringCallback callback(&other_thread_); base::Closure task = base::Bind( @@ -2830,7 +3298,7 @@ TEST_F(MultiThreadedCookieMonsterTest, ThreadCheckGetCookiesWithInfo) { CookieOptions options; std::string cookie_line; std::vector<CookieStore::CookieInfo> cookie_infos; - EXPECT_TRUE(cm->SetCookie(url_google_, "A=B")); + EXPECT_TRUE(SetCookie(cm, url_google_, "A=B")); GetCookiesWithInfo(cm, url_google_, options, &cookie_line, &cookie_infos); EXPECT_EQ("A=B", cookie_line); EXPECT_EQ(1U, cookie_infos.size()); @@ -2853,7 +3321,7 @@ TEST_F(MultiThreadedCookieMonsterTest, ThreadCheckGetCookiesWithInfo) { TEST_F(MultiThreadedCookieMonsterTest, ThreadCheckGetAllCookies) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); - EXPECT_TRUE(cm->SetCookie(url_google_, "A=B")); + EXPECT_TRUE(SetCookie(cm, url_google_, "A=B")); CookieList cookies = GetAllCookies(cm); CookieList::const_iterator it = cookies.begin(); ASSERT_TRUE(it != cookies.end()); @@ -2876,7 +3344,7 @@ TEST_F(MultiThreadedCookieMonsterTest, ThreadCheckGetAllCookies) { TEST_F(MultiThreadedCookieMonsterTest, ThreadCheckGetAllCookiesForURL) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); - EXPECT_TRUE(cm->SetCookie(url_google_, "A=B")); + EXPECT_TRUE(SetCookie(cm, url_google_, "A=B")); CookieList cookies = GetAllCookiesForURL(cm, url_google_); CookieList::const_iterator it = cookies.begin(); ASSERT_TRUE(it != cookies.end()); @@ -2899,7 +3367,7 @@ TEST_F(MultiThreadedCookieMonsterTest, ThreadCheckGetAllCookiesForURL) { TEST_F(MultiThreadedCookieMonsterTest, ThreadCheckGetAllCookiesForURLWithOpt) { scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL)); - EXPECT_TRUE(cm->SetCookie(url_google_, "A=B")); + EXPECT_TRUE(SetCookie(cm, url_google_, "A=B")); CookieOptions options; CookieList cookies = GetAllCookiesForURLWithOptions(cm, url_google_, options); @@ -2973,14 +3441,14 @@ TEST_F(MultiThreadedCookieMonsterTest, ThreadCheckDeleteAllCreatedBetween) { Time now = Time::Now(); EXPECT_TRUE(SetCookieWithOptions(cm, url_google_, "A=B", options)); EXPECT_EQ(1, DeleteAllCreatedBetween(cm, now - TimeDelta::FromDays(99), - Time(), false)); + Time())); EXPECT_TRUE(SetCookieWithOptions(cm, url_google_, "A=B", options)); DeleteCallback callback(&other_thread_); base::Closure task = base::Bind( &net::MultiThreadedCookieMonsterTest::DeleteAllCreatedBetweenTask, base::Unretained(this), cm, now - TimeDelta::FromDays(99), - Time(), false, &callback); + Time(), &callback); RunOnOtherThread(task); EXPECT_TRUE(callback.did_run()); EXPECT_EQ(1, callback.num_deleted()); diff --git a/net/base/cookie_store.cc b/net/base/cookie_store.cc index 0b75580..a806d6c 100644 --- a/net/base/cookie_store.cc +++ b/net/base/cookie_store.cc @@ -8,32 +8,6 @@ namespace net { -bool CookieStore::SetCookie(const GURL& url, const std::string& cookie_line) { - return SetCookieWithOptions(url, cookie_line, CookieOptions()); -} - -std::string CookieStore::GetCookies(const GURL& url) { - return GetCookiesWithOptions(url, CookieOptions()); -} - -void CookieStore::GetCookiesAsync( - const GURL& url, const GetCookiesCallback& callback) { - GetCookiesWithOptionsAsync(url, CookieOptions(), callback); -} - -void CookieStore::SetCookiesWithOptions( - const GURL& url, - const std::vector<std::string>& cookie_lines, - const CookieOptions& options) { - for (size_t i = 0; i < cookie_lines.size(); ++i) - SetCookieWithOptions(url, cookie_lines[i], options); -} - -void CookieStore::SetCookies(const GURL& url, - const std::vector<std::string>& cookie_lines) { - SetCookiesWithOptions(url, cookie_lines, CookieOptions()); -} - CookieStore::CookieStore() {} CookieStore::~CookieStore() {} diff --git a/net/base/cookie_store.h b/net/base/cookie_store.h index d0ede14..e6b0738 100644 --- a/net/base/cookie_store.h +++ b/net/base/cookie_store.h @@ -47,17 +47,20 @@ class NET_EXPORT CookieStore : public base::RefCountedThreadSafe<CookieStore> { std::string mac_algorithm; }; + // Callback definitions. + typedef base::Callback <void( + const std::string& cookie_line, + const std::vector<CookieInfo>& cookie_infos)> GetCookieInfoCallback; + typedef base::Callback<void(const std::string& cookie)> + GetCookiesCallback; + typedef base::Callback<void(bool)> SetCookiesCallback; + + // Sets a single cookie. Expects a cookie line, like "a=1; domain=b.com". // // Fails either if the cookie is invalid or if this is a non-HTTPONLY cookie // and it would overwrite an existing HTTPONLY cookie. // Returns true if the cookie is successfully set. - virtual bool SetCookieWithOptions(const GURL& url, - const std::string& cookie_line, - const CookieOptions& options) = 0; - - typedef base::Callback<void(bool)> SetCookiesCallback; - virtual void SetCookieWithOptionsAsync( const GURL& url, const std::string& cookie_line, @@ -70,12 +73,6 @@ class NET_EXPORT CookieStore : public base::RefCountedThreadSafe<CookieStore> { // // Simple interface, gets a cookie string "a=b; c=d" for the given URL. // Use options to access httponly cookies. - virtual std::string GetCookiesWithOptions(const GURL& url, - const CookieOptions& options) = 0; - - typedef base::Callback<void(const std::string& cookie)> - GetCookiesCallback; - virtual void GetCookiesWithOptionsAsync( const GURL& url, const CookieOptions& options, const GetCookiesCallback& callback) = 0; @@ -84,24 +81,12 @@ class NET_EXPORT CookieStore : public base::RefCountedThreadSafe<CookieStore> { // GetCookiesWithOptions except that it additionaly provides detailed // information about the cookie contained in the cookie line. See |struct // CookieInfo| above for details. - virtual void GetCookiesWithInfo(const GURL& url, - const CookieOptions& options, - std::string* cookie_line, - std::vector<CookieInfo>* cookie_info) = 0; - - // Using for complete the asyn interface - typedef base::Callback <void( - std::string* cookie_line, - std::vector<CookieInfo>* cookie_infos)> GetCookieInfoCallback; - virtual void GetCookiesWithInfoAsync( const GURL& url, const CookieOptions& options, const GetCookieInfoCallback& callback) = 0; // Deletes the passed in cookie for the specified URL. - virtual void DeleteCookie(const GURL& url, - const std::string& cookie_name) = 0; virtual void DeleteCookieAsync(const GURL& url, const std::string& cookie_name, const base::Closure& callback) = 0; @@ -109,25 +94,6 @@ class NET_EXPORT CookieStore : public base::RefCountedThreadSafe<CookieStore> { // Returns the underlying CookieMonster. virtual CookieMonster* GetCookieMonster() = 0; - - // -------------------------------------------------------------------------- - // Helpers to make the above interface simpler for some cases. - - // Sets a cookie for the given URL using default options. - bool SetCookie(const GURL& url, const std::string& cookie_line); - - // Gets cookies for the given URL using default options. - std::string GetCookies(const GURL& url); - void GetCookiesAsync(const GURL& url, - const GetCookiesCallback& callback); - - // Sets a vector of response cookie values for the same URL. - void SetCookiesWithOptions(const GURL& url, - const std::vector<std::string>& cookie_lines, - const CookieOptions& options); - void SetCookies(const GURL& url, - const std::vector<std::string>& cookie_lines); - protected: friend class base::RefCountedThreadSafe<CookieStore>; CookieStore(); diff --git a/net/base/cookie_store_test_helpers.cc b/net/base/cookie_store_test_helpers.cc index c74133e..0d50de3 100644 --- a/net/base/cookie_store_test_helpers.cc +++ b/net/base/cookie_store_test_helpers.cc @@ -20,10 +20,10 @@ DelayedCookieMonster::~DelayedCookieMonster() { } void DelayedCookieMonster::GetCookiesInternalCallback( - std::string* cookie_line, - std::vector<CookieStore::CookieInfo>* cookie_info) { - cookie_line_ = *cookie_line; - cookie_info_ = *cookie_info; + const std::string& cookie_line, + const std::vector<CookieStore::CookieInfo>& cookie_info) { + cookie_line_ = cookie_line; + cookie_info_ = cookie_info; did_run_ = true; } @@ -91,7 +91,7 @@ void DelayedCookieMonster::GetCookiesWithOptionsAsync( void DelayedCookieMonster::InvokeGetCookiesCallback( const CookieMonster::GetCookieInfoCallback& callback) { if (!callback.is_null()) - callback.Run(&cookie_line_, &cookie_info_); + callback.Run(cookie_line_, cookie_info_); } void DelayedCookieMonster::InvokeSetCookiesCallback( diff --git a/net/base/cookie_store_test_helpers.h b/net/base/cookie_store_test_helpers.h index 568bbe5..e6107c6 100644 --- a/net/base/cookie_store_test_helpers.h +++ b/net/base/cookie_store_test_helpers.h @@ -65,8 +65,8 @@ class DelayedCookieMonster : public CookieStore { // Be called immediately from CookieMonster. void GetCookiesInternalCallback( - std::string* cookie_line, - std::vector<CookieStore::CookieInfo>* cookie_info); + const std::string& cookie_line, + const std::vector<CookieStore::CookieInfo>& cookie_info); void SetCookiesInternalCallback(bool result); diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc index 89a0ef9..8b13d5b1 100644 --- a/net/url_request/url_request_http_job.cc +++ b/net/url_request/url_request_http_job.cc @@ -543,14 +543,14 @@ void URLRequestHttpJob::CheckCookiePolicyAndLoad( } void URLRequestHttpJob::OnCookiesLoaded( - std::string* cookie_line, - std::vector<net::CookieStore::CookieInfo>* cookie_infos) { - if (!cookie_line->empty()) { + const std::string& cookie_line, + const std::vector<net::CookieStore::CookieInfo>& cookie_infos) { + if (!cookie_line.empty()) { request_info_.extra_headers.SetHeader( - HttpRequestHeaders::kCookie, *cookie_line); + HttpRequestHeaders::kCookie, cookie_line); } if (URLRequest::AreMacCookiesEnabled()) - AddAuthorizationHeader(*cookie_infos, &request_info_); + AddAuthorizationHeader(cookie_infos, &request_info_); DoStartTransaction(); } diff --git a/net/url_request/url_request_http_job.h b/net/url_request/url_request_http_job.h index 1957dfd..8293c13 100644 --- a/net/url_request/url_request_http_job.h +++ b/net/url_request/url_request_http_job.h @@ -169,8 +169,8 @@ class URLRequestHttpJob : public URLRequestJob { // Callback functions for Cookie Monster void CheckCookiePolicyAndLoad(const CookieList& cookie_list); void OnCookiesLoaded( - std::string* cookie_line, - std::vector<CookieStore::CookieInfo>* cookie_infos); + const std::string& cookie_line, + const std::vector<CookieStore::CookieInfo>& cookie_infos); void DoStartTransaction(); void OnCookieSaved(bool cookie_status); void CookieHandled(); diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc index a8f4a47..44f6560 100644 --- a/net/url_request/url_request_unittest.cc +++ b/net/url_request/url_request_unittest.cc @@ -13,6 +13,7 @@ #include <string> #include "base/basictypes.h" +#include "base/bind.h" #include "base/compiler_specific.h" #include "base/file_util.h" #include "base/format_macros.h" @@ -2017,6 +2018,13 @@ TEST_F(URLRequestTest, DoNotSaveCookies_ViaPolicy_Async) { } } +void CheckCookiePolicyCallback(bool* was_run, const CookieList& cookies) { + EXPECT_EQ(1U, cookies.size()); + EXPECT_FALSE(cookies[0].IsPersistent()); + *was_run = true; + MessageLoop::current()->PostTask(FROM_HERE, new MessageLoop::QuitTask()); +} + TEST_F(URLRequestTest, CookiePolicy_ForceSession) { TestServer test_server(TestServer::TYPE_HTTP, FilePath()); ASSERT_TRUE(test_server.Start()); @@ -2037,10 +2045,11 @@ TEST_F(URLRequestTest, CookiePolicy_ForceSession) { } // Now, check the cookie store. - CookieList cookies = - default_context_->cookie_store()->GetCookieMonster()->GetAllCookies(); - EXPECT_EQ(1U, cookies.size()); - EXPECT_FALSE(cookies[0].IsPersistent()); + bool was_run = false; + default_context_->cookie_store()->GetCookieMonster()->GetAllCookiesAsync( + base::Bind(&CheckCookiePolicyCallback, &was_run)); + MessageLoop::current()->RunAllPending(); + DCHECK(was_run); } // In this test, we do a POST which the server will 302 redirect. |