diff --git a/net/cookies/cookie_monster.cc b/net/cookies/cookie_monster.cc index db0bd0028a3690..82029e77732ad5 100644 --- a/net/cookies/cookie_monster.cc +++ b/net/cookies/cookie_monster.cc @@ -522,11 +522,11 @@ void CookieMonster::GetAllCookiesTask::Run() { } } -// Task class for GetAllCookiesForURLWithOptions call. -class CookieMonster::GetAllCookiesForURLWithOptionsTask +// Task class for GetCookieListForURLWithOptionsAsync call. +class CookieMonster::GetCookieListForURLWithOptionsTask : public CookieMonsterTask { public: - GetAllCookiesForURLWithOptionsTask(CookieMonster* cookie_monster, + GetCookieListForURLWithOptionsTask(CookieMonster* cookie_monster, const GURL& url, const CookieOptions& options, const GetCookieListCallback& callback) @@ -539,20 +539,20 @@ class CookieMonster::GetAllCookiesForURLWithOptionsTask void Run() override; protected: - ~GetAllCookiesForURLWithOptionsTask() override {} + ~GetCookieListForURLWithOptionsTask() override {} private: GURL url_; CookieOptions options_; GetCookieListCallback callback_; - DISALLOW_COPY_AND_ASSIGN(GetAllCookiesForURLWithOptionsTask); + DISALLOW_COPY_AND_ASSIGN(GetCookieListForURLWithOptionsTask); }; -void CookieMonster::GetAllCookiesForURLWithOptionsTask::Run() { +void CookieMonster::GetCookieListForURLWithOptionsTask::Run() { if (!callback_.is_null()) { CookieList cookies = - this->cookie_monster()->GetAllCookiesForURLWithOptions(url_, options_); + this->cookie_monster()->GetCookieListForURLWithOptions(url_, options_); this->InvokeCallback(base::Bind(&GetCookieListCallback::Run, base::Unretained(&callback_), cookies)); } @@ -905,12 +905,12 @@ void CookieMonster::SetCookieWithDetailsAsync( DoCookieTaskForURL(task, url); } -void CookieMonster::GetAllCookiesForURLWithOptionsAsync( +void CookieMonster::GetCookieListForURLWithOptionsAsync( const GURL& url, const CookieOptions& options, const GetCookieListCallback& callback) { - scoped_refptr task = - new GetAllCookiesForURLWithOptionsTask(this, url, options, callback); + scoped_refptr task = + new GetCookieListForURLWithOptionsTask(this, url, options, callback); DoCookieTaskForURL(task, url); } @@ -962,8 +962,9 @@ void CookieMonster::GetAllCookiesForURLAsync( CookieOptions options; options.set_include_httponly(); options.set_include_same_site(); - scoped_refptr task = - new GetAllCookiesForURLWithOptionsTask(this, url, options, callback); + options.set_do_not_update_access_time(); + scoped_refptr task = + new GetCookieListForURLWithOptionsTask(this, url, options, callback); DoCookieTaskForURL(task, url); } @@ -1159,13 +1160,13 @@ CookieList CookieMonster::GetAllCookies() { return cookie_list; } -CookieList CookieMonster::GetAllCookiesForURLWithOptions( +CookieList CookieMonster::GetCookieListForURLWithOptions( const GURL& url, const CookieOptions& options) { base::AutoLock autolock(lock_); std::vector cookie_ptrs; - FindCookiesForHostAndDomain(url, options, false, &cookie_ptrs); + FindCookiesForHostAndDomain(url, options, &cookie_ptrs); std::sort(cookie_ptrs.begin(), cookie_ptrs.end(), CookieSorter); CookieList cookies; @@ -1254,7 +1255,7 @@ std::string CookieMonster::GetCookiesWithOptions(const GURL& url, return std::string(); std::vector cookies; - FindCookiesForHostAndDomain(url, options, true, &cookies); + FindCookiesForHostAndDomain(url, options, &cookies); std::sort(cookies.begin(), cookies.end(), CookieSorter); std::string cookie_line = BuildCookieLine(cookies); @@ -1276,7 +1277,7 @@ void CookieMonster::DeleteCookie(const GURL& url, options.set_include_same_site(); // Get the cookies for this host and its domain(s). std::vector cookies; - FindCookiesForHostAndDomain(url, options, true, &cookies); + FindCookiesForHostAndDomain(url, options, &cookies); std::set matching_cookies; for (std::vector::const_iterator it = cookies.begin(); @@ -1607,7 +1608,6 @@ void CookieMonster::TrimDuplicateCookiesForKey(const std::string& key, void CookieMonster::FindCookiesForHostAndDomain( const GURL& url, const CookieOptions& options, - bool update_access_time, std::vector* cookies) { lock_.AssertAcquired(); @@ -1620,15 +1620,13 @@ void CookieMonster::FindCookiesForHostAndDomain( // Can just dispatch to FindCookiesForKey const std::string key(GetKey(url.host())); - FindCookiesForKey(key, url, options, current_time, update_access_time, - cookies); + FindCookiesForKey(key, url, options, current_time, cookies); } void CookieMonster::FindCookiesForKey(const std::string& key, const GURL& url, const CookieOptions& options, const Time& current, - bool update_access_time, std::vector* cookies) { lock_.AssertAcquired(); @@ -1652,7 +1650,7 @@ void CookieMonster::FindCookiesForKey(const std::string& key, // Add this cookie to the set of matching cookies. Update the access // time if we've been requested to do so. - if (update_access_time) { + if (options.update_access_time()) { InternalUpdateCookieAccessTime(cc, current); } cookies->push_back(cc); diff --git a/net/cookies/cookie_monster.h b/net/cookies/cookie_monster.h index f482e5c19a3cf6..87acb9a05e2ce4 100644 --- a/net/cookies/cookie_monster.h +++ b/net/cookies/cookie_monster.h @@ -153,7 +153,7 @@ 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. - void GetAllCookiesForURLWithOptionsAsync( + void GetCookieListForURLWithOptionsAsync( const GURL& url, const CookieOptions& options, const GetCookieListCallback& callback); @@ -238,7 +238,7 @@ class NET_EXPORT CookieMonster : public CookieStore { class DeleteAllCreatedBetweenForHostTask; class DeleteCookieTask; class DeleteCanonicalCookieTask; - class GetAllCookiesForURLWithOptionsTask; + class GetCookieListForURLWithOptionsTask; class GetAllCookiesTask; class GetCookiesWithOptionsTask; class SetAllCookiesTask; @@ -418,7 +418,7 @@ class NET_EXPORT CookieMonster : public CookieStore { CookieList GetAllCookies(); - CookieList GetAllCookiesForURLWithOptions(const GURL& url, + CookieList GetCookieListForURLWithOptions(const GURL& url, const CookieOptions& options); int DeleteAllCreatedBetween(const base::Time& delete_begin, @@ -496,14 +496,12 @@ class NET_EXPORT CookieMonster : public CookieStore { void FindCookiesForHostAndDomain(const GURL& url, const CookieOptions& options, - bool update_access_time, std::vector* cookies); void FindCookiesForKey(const std::string& key, const GURL& url, const CookieOptions& options, const base::Time& current, - bool update_access_time, std::vector* cookies); // Delete any cookies that are equivalent to |ecc| (same path, domain, etc). diff --git a/net/cookies/cookie_monster_unittest.cc b/net/cookies/cookie_monster_unittest.cc index ecba7eaafe64c4..eab8b756403bb2 100644 --- a/net/cookies/cookie_monster_unittest.cc +++ b/net/cookies/cookie_monster_unittest.cc @@ -126,7 +126,7 @@ class CookieMonsterTestBase : public CookieStoreTest { const CookieOptions& options) { DCHECK(cm); GetCookieListCallback callback; - cm->GetAllCookiesForURLWithOptionsAsync( + cm->GetCookieListForURLWithOptionsAsync( url, options, base::Bind(&GetCookieListCallback::Run, base::Unretained(&callback))); callback.WaitUntilDone(); @@ -826,8 +826,8 @@ ACTION_P2(DeleteAllAction, cookie_monster, callback) { cookie_monster->DeleteAllAsync(callback->AsCallback()); } -ACTION_P3(GetAllCookiesForUrlWithOptionsAction, cookie_monster, url, callback) { - cookie_monster->GetAllCookiesForURLWithOptionsAsync(url, CookieOptions(), +ACTION_P3(GetCookieListForUrlWithOptionsAction, cookie_monster, url, callback) { + cookie_monster->GetCookieListForURLWithOptionsAsync(url, CookieOptions(), callback->AsCallback()); } @@ -1137,14 +1137,14 @@ TEST_F(DeferredCookieTaskTest, DeferredGetAllForUrlWithOptionsCookies) { MockGetCookieListCallback get_cookie_list_callback; BeginWithForDomainKey(http_www_google_.domain(), - GetAllCookiesForUrlWithOptionsAction( + GetCookieListForUrlWithOptionsAction( &cookie_monster(), http_www_google_.url(), &get_cookie_list_callback)); WaitForLoadCall(); EXPECT_CALL(get_cookie_list_callback, Invoke(testing::_)) - .WillOnce(GetAllCookiesForUrlWithOptionsAction( + .WillOnce(GetCookieListForUrlWithOptionsAction( &cookie_monster(), http_www_google_.url(), &get_cookie_list_callback)); base::RunLoop loop; @@ -1372,12 +1372,33 @@ TEST_F(CookieMonsterTest, TestLastAccess) { // Reading the cookie again immediately shouldn't update the access date, // since we're inside the threshold. EXPECT_EQ("A=B", GetCookies(cm.get(), http_www_google_.url())); - EXPECT_TRUE(last_access_date == GetFirstCookieAccessDate(cm.get())); + EXPECT_EQ(last_access_date, GetFirstCookieAccessDate(cm.get())); - // Reading after a short wait should update the access date. + // Reading after a short wait will update the access date, if the cookie + // is requested with options that would update the access date. First, test + // that the flag's behavior is respected. base::PlatformThread::Sleep( base::TimeDelta::FromMilliseconds(kAccessDelayMs)); - EXPECT_EQ("A=B", GetCookies(cm.get(), http_www_google_.url())); + CookieOptions options; + options.set_do_not_update_access_time(); + EXPECT_EQ("A=B", + GetCookiesWithOptions(cm.get(), http_www_google_.url(), options)); + EXPECT_EQ(last_access_date, GetFirstCookieAccessDate(cm.get())); + + // Getting all cookies for a URL doesn't update the accessed time either. + CookieList cookies = GetAllCookiesForURL(cm.get(), http_www_google_.url()); + CookieList::iterator it = cookies.begin(); + ASSERT_TRUE(it != cookies.end()); + EXPECT_EQ(http_www_google_.host(), it->Domain()); + EXPECT_EQ("A", it->Name()); + EXPECT_EQ("B", it->Value()); + EXPECT_EQ(last_access_date, GetFirstCookieAccessDate(cm.get())); + EXPECT_TRUE(++it == cookies.end()); + + // If the flag isn't set, the last accessed time should be updated. + options = CookieOptions(); + EXPECT_EQ("A=B", + GetCookiesWithOptions(cm.get(), http_www_google_.url(), options)); EXPECT_FALSE(last_access_date == GetFirstCookieAccessDate(cm.get())); } @@ -1481,7 +1502,7 @@ TEST_F(CookieMonsterTest, GetAllCookiesForURL) { ASSERT_TRUE(++it == cookies.end()); // Reading after a short wait should not update the access date. - EXPECT_TRUE(last_access_date == GetFirstCookieAccessDate(cm.get())); + EXPECT_EQ(last_access_date, GetFirstCookieAccessDate(cm.get())); } TEST_F(CookieMonsterTest, GetAllCookiesForURLPathMatching) { @@ -2471,7 +2492,7 @@ class MultiThreadedCookieMonsterTest : public CookieMonsterTest { const GURL& url, const CookieOptions& options, GetCookieListCallback* callback) { - cm->GetAllCookiesForURLWithOptionsAsync( + cm->GetCookieListForURLWithOptionsAsync( url, options, base::Bind(&GetCookieListCallback::Run, base::Unretained(callback))); } diff --git a/net/cookies/cookie_options.cc b/net/cookies/cookie_options.cc index b377c81ad8e3b0..103b768068e84e 100644 --- a/net/cookies/cookie_options.cc +++ b/net/cookies/cookie_options.cc @@ -12,6 +12,7 @@ CookieOptions::CookieOptions() : exclude_httponly_(true), include_same_site_(false), enforce_strict_secure_(false), + update_access_time_(true), server_time_() {} } // namespace net diff --git a/net/cookies/cookie_options.h b/net/cookies/cookie_options.h index 25dedb14e19048..801e958e268fac 100644 --- a/net/cookies/cookie_options.h +++ b/net/cookies/cookie_options.h @@ -20,12 +20,14 @@ class NET_EXPORT CookieOptions { // * Excludes HttpOnly cookies // * Excludes SameSite cookies // * Does not enforce prefix restrictions (e.g. "$Secure-*") + // * Updates last-accessed time. // // These settings can be altered by calling: // // * |set_{include,exclude}_httponly()| // * |set_include_same_site()| // * |set_enforce_prefixes()| + // * |set_do_not_update_access_time()| CookieOptions(); void set_exclude_httponly() { exclude_httponly_ = true; } @@ -50,10 +52,14 @@ class NET_EXPORT CookieOptions { bool has_server_time() const { return !server_time_.is_null(); } base::Time server_time() const { return server_time_; } + void set_do_not_update_access_time() { update_access_time_ = false; } + bool update_access_time() const { return update_access_time_; } + private: bool exclude_httponly_; bool include_same_site_; bool enforce_strict_secure_; + bool update_access_time_; base::Time server_time_; }; diff --git a/net/cookies/cookie_store.h b/net/cookies/cookie_store.h index c54148f372c89f..21c586bc270320 100644 --- a/net/cookies/cookie_store.h +++ b/net/cookies/cookie_store.h @@ -98,8 +98,9 @@ class NET_EXPORT CookieStore : public base::RefCountedThreadSafe { const CookieOptions& options, const GetCookiesCallback& callback) = 0; - // Invokes GetAllCookiesForURLWithOptions with options set to include HTTP - // only cookies. + // Returns all cookies associated with |url|, including http-only, and + // same-site cookies. The returned cookies are ordered by longest path, then + // by earliest creation date, and are not marked as having been accessed. virtual void GetAllCookiesForURLAsync( const GURL& url, const GetCookieListCallback& callback) = 0;