diff options
Diffstat (limited to 'chromium/net/cookies')
25 files changed, 2523 insertions, 596 deletions
diff --git a/chromium/net/cookies/OWNERS b/chromium/net/cookies/OWNERS index db28c8e2140..fb0dd2d56c8 100644 --- a/chromium/net/cookies/OWNERS +++ b/chromium/net/cookies/OWNERS @@ -1,6 +1,6 @@ estark@chromium.org mkwst@chromium.org mmenke@chromium.org -rdsmith@chromium.org +morlovich@chromium.org # COMPONENT: Internals>Network>Cookies diff --git a/chromium/net/cookies/canonical_cookie.cc b/chromium/net/cookies/canonical_cookie.cc index 0939cdd319f..ffd247ea73c 100644 --- a/chromium/net/cookies/canonical_cookie.cc +++ b/chromium/net/cookies/canonical_cookie.cc @@ -346,36 +346,7 @@ bool CanonicalCookie::IsOnPath(const std::string& url_path) const { } bool CanonicalCookie::IsDomainMatch(const std::string& host) const { - // Can domain match in two ways; as a domain cookie (where the cookie - // domain begins with ".") or as a host cookie (where it doesn't). - - // Some consumers of the CookieMonster expect to set cookies on - // URLs like http://.strange.url. To retrieve cookies in this instance, - // we allow matching as a host cookie even when the domain_ starts with - // a period. - if (host == domain_) - return true; - - // Domain cookie must have an initial ".". To match, it must be - // equal to url's host with initial period removed, or a suffix of - // it. - - // Arguably this should only apply to "http" or "https" cookies, but - // extension cookie tests currently use the funtionality, and if we - // ever decide to implement that it should be done by preventing - // such cookies from being set. - if (domain_.empty() || domain_[0] != '.') - return false; - - // The host with a "." prefixed. - if (domain_.compare(1, std::string::npos, host) == 0) - return true; - - // A pure suffix of the host (ok since we know the domain already - // starts with a ".") - return (host.length() > domain_.length() && - host.compare(host.length() - domain_.length(), - domain_.length(), domain_) == 0); + return cookie_util::IsDomainMatch(domain_, host); } bool CanonicalCookie::IncludeForRequestURL(const GURL& url, diff --git a/chromium/net/cookies/canonical_cookie_unittest.cc b/chromium/net/cookies/canonical_cookie_unittest.cc index 04c050bf215..97d25ef455b 100644 --- a/chromium/net/cookies/canonical_cookie_unittest.cc +++ b/chromium/net/cookies/canonical_cookie_unittest.cc @@ -148,21 +148,25 @@ TEST(CanonicalCookieTest, Create) { EXPECT_EQ(CookieSameSite::NO_RESTRICTION, cookie->SameSite()); } -TEST(CanonicalCookieTest, CreateInvalidSameSite) { +TEST(CanonicalCookieTest, CreateNonStandardSameSite) { GURL url("http://www.example.com/test/foo.html"); base::Time now = base::Time::Now(); std::unique_ptr<CanonicalCookie> cookie; CookieOptions options; - // Invalid 'SameSite' attribute values. options.set_same_site_cookie_mode( CookieOptions::SameSiteCookieMode::INCLUDE_STRICT_AND_LAX); - cookie = CanonicalCookie::Create(url, "A=2; SameSite=Invalid", now, options); - EXPECT_EQ(nullptr, cookie.get()); + // Non-standard value for the SameSite attribute. + cookie = + CanonicalCookie::Create(url, "A=2; SameSite=NonStandard", now, options); + EXPECT_TRUE(cookie.get()); + EXPECT_EQ(CookieSameSite::NO_RESTRICTION, cookie->SameSite()); + // Omit value for the SameSite attribute. cookie = CanonicalCookie::Create(url, "A=2; SameSite", now, options); - EXPECT_EQ(nullptr, cookie.get()); + EXPECT_TRUE(cookie.get()); + EXPECT_EQ(CookieSameSite::NO_RESTRICTION, cookie->SameSite()); } TEST(CanonicalCookieTest, CreateInvalidHttpOnly) { diff --git a/chromium/net/cookies/cookie_change_dispatcher.h b/chromium/net/cookies/cookie_change_dispatcher.h index 8bea41b29ed..8b48c046f6a 100644 --- a/chromium/net/cookies/cookie_change_dispatcher.h +++ b/chromium/net/cookies/cookie_change_dispatcher.h @@ -110,6 +110,11 @@ class CookieChangeDispatcher { const std::string& name, CookieChangeCallback callback) WARN_UNUSED_RESULT = 0; + // Observe changes to the cookies that would be sent for a request to |url|. + virtual std::unique_ptr<CookieChangeSubscription> AddCallbackForUrl( + const GURL& url, + CookieChangeCallback callback) WARN_UNUSED_RESULT = 0; + // Observe all the CookieStore's changes. // // The callback will not observe a few bookkeeping changes. diff --git a/chromium/net/cookies/cookie_change_dispatcher_test_helpers.cc b/chromium/net/cookies/cookie_change_dispatcher_test_helpers.cc new file mode 100644 index 00000000000..0db6daaa5e8 --- /dev/null +++ b/chromium/net/cookies/cookie_change_dispatcher_test_helpers.cc @@ -0,0 +1,33 @@ +// Copyright 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "net/cookies/cookie_change_dispatcher_test_helpers.h" + +#include "base/logging.h" + +namespace net { + +// Google Test helper. +std::ostream& operator<<(std::ostream& os, const CookieChangeCause& cause) { + switch (cause) { + case CookieChangeCause::INSERTED: + return os << "INSERTED"; + case CookieChangeCause::EXPLICIT: + return os << "EXPLICIT"; + case CookieChangeCause::UNKNOWN_DELETION: + return os << "UNKNOWN_DELETION"; + case CookieChangeCause::OVERWRITE: + return os << "OVERWRITE"; + case CookieChangeCause::EXPIRED: + return os << "EXPIRED"; + case CookieChangeCause::EVICTED: + return os << "EVICTED"; + case CookieChangeCause::EXPIRED_OVERWRITE: + return os << "EXPIRED_OVERWRITE"; + } + NOTREACHED(); + return os; +} + +} // namespace net diff --git a/chromium/net/cookies/cookie_change_dispatcher_test_helpers.h b/chromium/net/cookies/cookie_change_dispatcher_test_helpers.h new file mode 100644 index 00000000000..5ef6d9cd4d8 --- /dev/null +++ b/chromium/net/cookies/cookie_change_dispatcher_test_helpers.h @@ -0,0 +1,19 @@ +// Copyright 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef NET_COOKIES_COOKIE_CHANGE_DISPATCHER_TEST_HELPERS_H_ +#define NET_COOKIES_COOKIE_CHANGE_DISPATCHER_TEST_HELPERS_H_ + +#include <ostream> + +#include "net/cookies/cookie_change_dispatcher.h" + +namespace net { + +// Google Test helper for printing CookieChangeCause values. +std::ostream& operator<<(std::ostream& os, const CookieChangeCause& cause); + +} // namespace net + +#endif // NET_COOKIES_COOKIE_CHANGE_DISPATCHER_TEST_HELPERS_H_ diff --git a/chromium/net/cookies/cookie_constants.h b/chromium/net/cookies/cookie_constants.h index 61e1c0a0c0c..3f5b3cf7dda 100644 --- a/chromium/net/cookies/cookie_constants.h +++ b/chromium/net/cookies/cookie_constants.h @@ -37,7 +37,7 @@ NET_EXPORT std::string CookiePriorityToString(CookiePriority priority); // Defaults to COOKIE_PRIORITY_DEFAULT for empty or unrecognized strings. NET_EXPORT CookiePriority StringToCookiePriority(const std::string& priority); -// Converst the Set-Cookie header SameSite token |same_site| to a +// Converts the Set-Cookie header SameSite token |same_site| to a // CookieSameSite. Defaults to CookieSameSite::DEFAULT_MODE for empty or // unrecognized strings. NET_EXPORT CookieSameSite StringToCookieSameSite(const std::string& same_site); diff --git a/chromium/net/cookies/cookie_monster.cc b/chromium/net/cookies/cookie_monster.cc index ce09270fc62..46df1b24378 100644 --- a/chromium/net/cookies/cookie_monster.cc +++ b/chromium/net/cookies/cookie_monster.cc @@ -49,17 +49,21 @@ #include "base/bind.h" #include "base/callback.h" +#include "base/debug/dump_without_crashing.h" #include "base/location.h" #include "base/logging.h" #include "base/macros.h" #include "base/metrics/field_trial.h" #include "base/metrics/histogram.h" +#include "base/metrics/histogram_macros.h" #include "base/single_thread_task_runner.h" #include "base/stl_util.h" #include "base/strings/string_piece.h" #include "base/strings/string_util.h" #include "base/strings/stringprintf.h" #include "base/threading/thread_task_runner_handle.h" +#include "base/timer/elapsed_timer.h" +#include "base/trace_event/process_memory_dump.h" #include "net/base/registry_controlled_domains/registry_controlled_domain.h" #include "net/cookies/canonical_cookie.h" #include "net/cookies/cookie_monster_change_dispatcher.h" @@ -97,8 +101,8 @@ static const int kMinutesInTenYears = 10 * 365 * 24 * 60; namespace { -void MayeRunDeleteCallback(base::WeakPtr<net::CookieMonster> cookie_monster, - base::OnceClosure callback) { +void MaybeRunDeleteCallback(base::WeakPtr<net::CookieMonster> cookie_monster, + base::OnceClosure callback) { if (cookie_monster && callback) std::move(callback).Run(); } @@ -122,6 +126,22 @@ void MaybeRunCookieCallback(base::OnceCallback<void(T)> callback, std::move(callback).Run(result); } +// Wraps a OnceClosure -- specifically one used by +// |GetCookieListWithOptionsAsync()| -- with additional bound state to track the +// duration between when its creation and destruction time. +// See https://crbug.com/824024 for context. +base::OnceClosure InstrumentGetCookieListClosure(base::OnceClosure closure) { + return base::BindOnce( + [](std::unique_ptr<base::ElapsedTimer> timer, base::OnceClosure closure) { + UMA_HISTOGRAM_CUSTOM_TIMES("Cookie.GetCookieListCompletionTime", + timer->Elapsed(), + base::TimeDelta::FromMilliseconds(10), + base::TimeDelta::FromSeconds(60), 50); + std::move(closure).Run(); + }, + std::make_unique<base::ElapsedTimer>(), std::move(closure)); +} + } // namespace namespace net { @@ -331,31 +351,31 @@ size_t CountCookiesForPossibleDeletion( } // namespace -CookieMonster::CookieMonster(PersistentCookieStore* store) +CookieMonster::CookieMonster(scoped_refptr<PersistentCookieStore> store) : CookieMonster( - store, + std::move(store), nullptr, base::TimeDelta::FromSeconds(kDefaultAccessUpdateThresholdSeconds)) {} -CookieMonster::CookieMonster(PersistentCookieStore* store, +CookieMonster::CookieMonster(scoped_refptr<PersistentCookieStore> store, ChannelIDService* channel_id_service) : CookieMonster( - store, + std::move(store), channel_id_service, base::TimeDelta::FromSeconds(kDefaultAccessUpdateThresholdSeconds)) {} -CookieMonster::CookieMonster(PersistentCookieStore* store, +CookieMonster::CookieMonster(scoped_refptr<PersistentCookieStore> store, base::TimeDelta last_access_threshold) - : CookieMonster(store, nullptr, last_access_threshold) {} + : CookieMonster(std::move(store), nullptr, last_access_threshold) {} -CookieMonster::CookieMonster(PersistentCookieStore* store, +CookieMonster::CookieMonster(scoped_refptr<PersistentCookieStore> store, ChannelIDService* channel_id_service, base::TimeDelta last_access_threshold) : initialized_(false), started_fetching_all_cookies_(false), finished_fetching_all_cookies_(false), seen_global_task_(false), - store_(store), + store_(std::move(store)), last_access_threshold_(last_access_threshold), channel_id_service_(channel_id_service), last_statistic_record_time_(base::Time::Now()), @@ -446,12 +466,12 @@ void CookieMonster::GetCookieListWithOptionsAsync( const CookieOptions& options, GetCookieListCallback callback) { DoCookieCallbackForURL( - base::BindOnce( + InstrumentGetCookieListClosure(base::BindOnce( // base::Unretained is safe as DoCookieCallbackForURL stores // the callback on |*this|, so the callback will not outlive // the object. &CookieMonster::GetCookieListWithOptions, base::Unretained(this), url, - options, std::move(callback)), + options, std::move(callback))), url); } @@ -565,6 +585,32 @@ bool CookieMonster::IsEphemeral() { return store_.get() == nullptr; } +void CookieMonster::DumpMemoryStats( + base::trace_event::ProcessMemoryDump* pmd, + const std::string& parent_absolute_name) const { + const char kRelPath[] = "/cookie_monster"; + + pmd->CreateAllocatorDump(parent_absolute_name + kRelPath + "/cookies") + ->AddScalar(base::trace_event::MemoryAllocatorDump::kNameObjectCount, + base::trace_event::MemoryAllocatorDump::kUnitsObjects, + cookies_.size()); + + pmd->CreateAllocatorDump(parent_absolute_name + kRelPath + + "/tasks_pending_global") + ->AddScalar(base::trace_event::MemoryAllocatorDump::kNameObjectCount, + base::trace_event::MemoryAllocatorDump::kUnitsObjects, + tasks_pending_.size()); + + size_t total_pending_for_key = 0; + for (const auto& kv : tasks_pending_for_key_) + total_pending_for_key += kv.second.size(); + pmd->CreateAllocatorDump(parent_absolute_name + kRelPath + + "/tasks_pending_for_key") + ->AddScalar(base::trace_event::MemoryAllocatorDump::kNameObjectCount, + base::trace_event::MemoryAllocatorDump::kUnitsObjects, + total_pending_for_key); +} + CookieMonster::~CookieMonster() { DCHECK(thread_checker_.CalledOnValidThread()); @@ -652,7 +698,7 @@ void CookieMonster::DeleteAllCreatedBetween(const Time& delete_begin, } FlushStore( - base::BindOnce(&MayeRunDeleteCallback, weak_ptr_factory_.GetWeakPtr(), + base::BindOnce(&MaybeRunDeleteCallback, weak_ptr_factory_.GetWeakPtr(), callback ? base::BindOnce(std::move(callback), num_deleted) : base::OnceClosure())); } @@ -680,7 +726,7 @@ void CookieMonster::DeleteAllCreatedBetweenWithPredicate( } FlushStore( - base::BindOnce(&MayeRunDeleteCallback, weak_ptr_factory_.GetWeakPtr(), + base::BindOnce(&MaybeRunDeleteCallback, weak_ptr_factory_.GetWeakPtr(), callback ? base::BindOnce(std::move(callback), num_deleted) : base::OnceClosure())); } @@ -749,7 +795,7 @@ void CookieMonster::DeleteCookie(const GURL& url, } } - FlushStore(base::BindOnce(&MayeRunDeleteCallback, + FlushStore(base::BindOnce(&MaybeRunDeleteCallback, weak_ptr_factory_.GetWeakPtr(), // No callback null check needed as BindOnce // is not being called and MaybeRunDeleteCallback @@ -764,15 +810,21 @@ void CookieMonster::DeleteCanonicalCookie(const CanonicalCookie& cookie, uint32_t result = 0u; for (CookieMapItPair its = cookies_.equal_range(GetKey(cookie.Domain())); its.first != its.second; ++its.first) { - // The creation date acts as the unique index... - if (its.first->second->CreationDate() == cookie.CreationDate()) { + const std::unique_ptr<CanonicalCookie>& candidate = its.first->second; + // Historically, this has refused modification if the cookie has changed + // value in between the CanonicalCookie object was returned by a getter + // and when this ran. The later parts of the conditional (everything but + // the equivalence check) attempt to preserve this behavior. + if (candidate->IsEquivalent(cookie) && + candidate->CreationDate() == cookie.CreationDate() && + candidate->Value() == cookie.Value()) { InternalDeleteCookie(its.first, true, DELETE_COOKIE_EXPLICIT); result = 1u; break; } } FlushStore( - base::BindOnce(&MayeRunDeleteCallback, weak_ptr_factory_.GetWeakPtr(), + base::BindOnce(&MaybeRunDeleteCallback, weak_ptr_factory_.GetWeakPtr(), callback ? base::BindOnce(std::move(callback), result) : base::OnceClosure())); } @@ -794,7 +846,7 @@ void CookieMonster::DeleteSessionCookies(DeleteCallback callback) { } FlushStore( - base::BindOnce(&MayeRunDeleteCallback, weak_ptr_factory_.GetWeakPtr(), + base::BindOnce(&MaybeRunDeleteCallback, weak_ptr_factory_.GetWeakPtr(), callback ? base::BindOnce(std::move(callback), num_deleted) : base::OnceClosure())); } @@ -872,28 +924,17 @@ void CookieMonster::StoreLoadedCookies( CookieItVector cookies_with_control_chars; for (auto& cookie : cookies) { - int64_t cookie_creation_time = cookie->CreationDate().ToInternalValue(); - - if (creation_times_.insert(cookie_creation_time).second) { - CanonicalCookie* cookie_ptr = cookie.get(); - CookieMap::iterator inserted = InternalInsertCookie( - GetKey(cookie_ptr->Domain()), std::move(cookie), false); - const Time cookie_access_time(cookie_ptr->LastAccessDate()); - if (earliest_access_time_.is_null() || - cookie_access_time < earliest_access_time_) - earliest_access_time_ = cookie_access_time; - - if (ContainsControlCharacter(cookie_ptr->Name()) || - ContainsControlCharacter(cookie_ptr->Value())) { - cookies_with_control_chars.push_back(inserted); - } - } else { - LOG(ERROR) << base::StringPrintf( - "Found cookies with duplicate creation " - "times in backing store: " - "{name='%s', domain='%s', path='%s'}", - cookie->Name().c_str(), cookie->Domain().c_str(), - cookie->Path().c_str()); + CanonicalCookie* cookie_ptr = cookie.get(); + CookieMap::iterator inserted = InternalInsertCookie( + GetKey(cookie_ptr->Domain()), std::move(cookie), false); + const Time cookie_access_time(cookie_ptr->LastAccessDate()); + if (earliest_access_time_.is_null() || + cookie_access_time < earliest_access_time_) + earliest_access_time_ = cookie_access_time; + + if (ContainsControlCharacter(cookie_ptr->Name()) || + ContainsControlCharacter(cookie_ptr->Value())) { + cookies_with_control_chars.push_back(inserted); } } @@ -943,7 +984,6 @@ void CookieMonster::InvokeQueue() { DCHECK(tasks_pending_for_key_.empty()); finished_fetching_all_cookies_ = true; - creation_times_.clear(); keys_loaded_.clear(); } @@ -969,7 +1009,7 @@ void CookieMonster::TrimDuplicateCookiesForKey(const std::string& key, DCHECK(thread_checker_.CalledOnValidThread()); // Set of cookies ordered by creation time. - typedef std::set<CookieMap::iterator, OrderByCreationTimeDesc> CookieSet; + typedef std::multiset<CookieMap::iterator, OrderByCreationTimeDesc> CookieSet; // Helper map we populate to find the duplicates. typedef std::map<CookieSignature, CookieSet> EquivalenceMap; @@ -993,9 +1033,7 @@ void CookieMonster::TrimDuplicateCookiesForKey(const std::string& key, // We save the iterator into |cookies_| rather than the actual cookie // pointer, since we may need to delete it later. - bool insert_success = set.insert(it).second; - DCHECK(insert_success) - << "Duplicate creation times found in duplicate cookie name scan."; + set.insert(it); } // If there were no duplicates, we are done! @@ -1016,8 +1054,9 @@ void CookieMonster::TrimDuplicateCookiesForKey(const std::string& key, continue; // This cookiename/path has no duplicates. num_duplicates_found += dupes.size() - 1; - // Since |dups| is sorted by creation time (descending), the first cookie - // is the most recent one, so we will keep it. The rest are duplicates. + // Since |dupes| is sorted by creation time (descending), the first cookie + // is the most recent one (or tied for it), so we will keep it. The rest are + // duplicates. dupes.erase(dupes.begin()); LOG(ERROR) << base::StringPrintf( diff --git a/chromium/net/cookies/cookie_monster.h b/chromium/net/cookies/cookie_monster.h index 99edf03b24b..2a691c7697c 100644 --- a/chromium/net/cookies/cookie_monster.h +++ b/chromium/net/cookies/cookie_monster.h @@ -57,9 +57,6 @@ class CookieChangeDispatcher; // latter case, the cookie callback will be queued in tasks_pending_for_key_ // while PermanentCookieStore loads cookies for the specified domain key on DB // thread. -// -// TODO(deanm) Implement CookieMonster, the cookie database. -// - Verify that our domain enforcement and non-dotted handling is correct class NET_EXPORT CookieMonster : public CookieStore { public: class PersistentCookieStore; @@ -134,16 +131,16 @@ class NET_EXPORT CookieMonster : public CookieStore { // this class, but it must remain valid for the duration of the cookie // monster's existence. If |store| is NULL, then no backing store will be // updated. - explicit CookieMonster(PersistentCookieStore* store); + explicit CookieMonster(scoped_refptr<PersistentCookieStore> store); // Like above, but includes a non-owning pointer |channel_id_service| for the // corresponding ChannelIDService used with this CookieStore. The // |channel_id_service| must outlive the CookieMonster. - CookieMonster(PersistentCookieStore* store, + CookieMonster(scoped_refptr<PersistentCookieStore> store, ChannelIDService* channel_id_service); // Only used during unit testing. - CookieMonster(PersistentCookieStore* store, + CookieMonster(scoped_refptr<PersistentCookieStore> store, base::TimeDelta last_access_threshold); ~CookieMonster() override; @@ -207,6 +204,9 @@ class NET_EXPORT CookieMonster : public CookieStore { bool IsEphemeral() override; + void DumpMemoryStats(base::trace_event::ProcessMemoryDump* pmd, + const std::string& parent_absolute_name) const override; + // Find a key based on the given domain, which will be used to find all // cookies potentially relevant to it. This is used for lookup in cookies_ as // well as for PersistentCookieStore::LoadCookiesForKey. See comment on keys @@ -214,7 +214,7 @@ class NET_EXPORT CookieMonster : public CookieStore { static std::string GetKey(base::StringPiece domain); private: - CookieMonster(PersistentCookieStore* store, + CookieMonster(scoped_refptr<PersistentCookieStore> store, ChannelIDService* channel_id_service, base::TimeDelta last_access_threshold); @@ -643,12 +643,6 @@ class NET_EXPORT CookieMonster : public CookieStore { // wanted. Thus this value is not initialized. base::Time earliest_access_time_; - // During loading, holds the set of all loaded cookie creation times. Used to - // avoid ever letting cookies with duplicate creation times into the store; - // that way we don't have to worry about what sections of code are safe - // to call while it's in that state. - std::set<int64_t> creation_times_; - std::vector<std::string> cookieable_schemes_; ChannelIDService* channel_id_service_; diff --git a/chromium/net/cookies/cookie_monster_change_dispatcher.cc b/chromium/net/cookies/cookie_monster_change_dispatcher.cc index f3b1ef7dea8..2eadab6ca65 100644 --- a/chromium/net/cookies/cookie_monster_change_dispatcher.cc +++ b/chromium/net/cookies/cookie_monster_change_dispatcher.cc @@ -4,13 +4,11 @@ #include "net/cookies/cookie_monster_change_dispatcher.h" -#include <memory> -#include <string> - #include "base/bind.h" +#include "base/strings/string_piece.h" #include "base/task_runner.h" -#include "base/threading/thread_checker.h" #include "base/threading/thread_task_runner_handle.h" +#include "net/base/registry_controlled_domains/registry_controlled_domain.h" #include "net/cookies/canonical_cookie.h" #include "net/cookies/cookie_change_dispatcher.h" @@ -18,84 +16,126 @@ namespace net { namespace { -// This class owns the CookieChangeCallbackList::Subscription, -// thus guaranteeing destruction when it is destroyed. In addition, it -// wraps the callback for a particular subscription, guaranteeing that it -// won't be run even if a PostTask completes after the subscription has -// been destroyed. -class CookieMonsterChangeSubscription : public CookieChangeSubscription { - public: - using CookieChangeCallbackList = - CookieMonsterChangeDispatcher::CookieChangeCallbackList; - - CookieMonsterChangeSubscription(const CookieChangeCallback& callback) - : callback_(callback), weak_ptr_factory_(this) {} - ~CookieMonsterChangeSubscription() override { - DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - } +// Special key in GlobalDomainMap for global listeners. +constexpr base::StringPiece kGlobalDomainKey = base::StringPiece("\0", 1); - void SetCallbackSubscription( - std::unique_ptr<CookieChangeCallbackList::Subscription> subscription) { - DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); +// +constexpr base::StringPiece kGlobalNameKey = base::StringPiece("\0", 1); - subscription_ = std::move(subscription); - } +} // anonymous namespace - // The returned callback runs the callback passed to the constructor - // directly as long as this object hasn't been destroyed. - CookieChangeCallback WeakCallback() { - DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); +CookieMonsterChangeDispatcher::Subscription::Subscription( + base::WeakPtr<CookieMonsterChangeDispatcher> change_dispatcher, + std::string domain_key, + std::string name_key, + GURL url, + net::CookieChangeCallback callback) + : change_dispatcher_(std::move(change_dispatcher)), + domain_key_(std::move(domain_key)), + name_key_(std::move(name_key)), + url_(std::move(url)), + callback_(std::move(callback)), + task_runner_(base::ThreadTaskRunnerHandle::Get()), + weak_ptr_factory_(this) { + DCHECK(url_.is_valid() || url_.is_empty()); + DCHECK_EQ(url_.is_empty(), domain_key_ == kGlobalDomainKey); + + // The net::CookieOptions are hard-coded for now, but future APIs may set + // different options. For example, JavaScript observers will not be allowed to + // see HTTP-only changes. + options_.set_include_httponly(); + options_.set_same_site_cookie_mode( + CookieOptions::SameSiteCookieMode::INCLUDE_STRICT_AND_LAX); +} - return base::BindRepeating(&CookieMonsterChangeSubscription::DispatchChange, - weak_ptr_factory_.GetWeakPtr()); - } +CookieMonsterChangeDispatcher::Subscription::~Subscription() { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - private: - void DispatchChange(const CanonicalCookie& cookie, CookieChangeCause cause) { - DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + change_dispatcher_->UnlinkSubscription(this); +} - callback_.Run(cookie, cause); - } +void CookieMonsterChangeDispatcher::Subscription::DispatchChange( + const net::CanonicalCookie& cookie, + net::CookieChangeCause change_cause) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - const CookieChangeCallback callback_; - std::unique_ptr<CookieChangeCallbackList::Subscription> subscription_; + if (!url_.is_empty() && !cookie.IncludeForRequestURL(url_, options_)) + return; - THREAD_CHECKER(thread_checker_); - base::WeakPtrFactory<CookieMonsterChangeSubscription> weak_ptr_factory_; + // TODO(mmenke, pwnall): Run callbacks synchronously? + task_runner_->PostTask( + FROM_HERE, + base::BindOnce(&Subscription::DoDispatchChange, + weak_ptr_factory_.GetWeakPtr(), cookie, change_cause)); +} - DISALLOW_COPY_AND_ASSIGN(CookieMonsterChangeSubscription); -}; +void CookieMonsterChangeDispatcher::Subscription::DoDispatchChange( + const net::CanonicalCookie& cookie, + net::CookieChangeCause change_cause) const { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); -void RunAsync(scoped_refptr<base::TaskRunner> proxy, - const CookieChangeCallback& callback, - const CanonicalCookie& cookie, - CookieChangeCause cause) { - proxy->PostTask(FROM_HERE, base::BindRepeating(callback, cookie, cause)); + callback_.Run(cookie, change_cause); } -} // anonymous namespace +CookieMonsterChangeDispatcher::CookieMonsterChangeDispatcher() + : weak_ptr_factory_(this) {} -CookieMonsterChangeDispatcher::CookieMonsterChangeDispatcher() = default; -CookieMonsterChangeDispatcher::~CookieMonsterChangeDispatcher() = default; +CookieMonsterChangeDispatcher::~CookieMonsterChangeDispatcher() { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); +} + +// static +std::string CookieMonsterChangeDispatcher::DomainKey( + const std::string& domain) { + std::string domain_key = + net::registry_controlled_domains::GetDomainAndRegistry( + domain, net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES); + DCHECK_NE(domain_key, kGlobalDomainKey); + return domain_key; +} + +// static +std::string CookieMonsterChangeDispatcher::DomainKey(const GURL& url) { + std::string domain_key = + net::registry_controlled_domains::GetDomainAndRegistry( + url, net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES); + DCHECK_NE(domain_key, kGlobalDomainKey); + return domain_key; +} + +// static +std::string CookieMonsterChangeDispatcher::NameKey(std::string name) { + DCHECK_NE(name, kGlobalNameKey); + return name; +} std::unique_ptr<CookieChangeSubscription> CookieMonsterChangeDispatcher::AddCallbackForCookie( - const GURL& gurl, + const GURL& url, const std::string& name, CookieChangeCallback callback) { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - std::pair<GURL, std::string> key(gurl, name); - if (hook_map_.count(key) == 0) - hook_map_[key] = std::make_unique<CookieChangeCallbackList>(); + std::unique_ptr<Subscription> subscription = std::make_unique<Subscription>( + weak_ptr_factory_.GetWeakPtr(), DomainKey(url), NameKey(name), url, + std::move(callback)); - auto subscription = - std::make_unique<CookieMonsterChangeSubscription>(std::move(callback)); - subscription->SetCallbackSubscription(hook_map_[key]->Add( - base::BindRepeating(&RunAsync, base::ThreadTaskRunnerHandle::Get(), - subscription->WeakCallback()))); + LinkSubscription(subscription.get()); + return subscription; +} - return std::move(subscription); +std::unique_ptr<CookieChangeSubscription> +CookieMonsterChangeDispatcher::AddCallbackForUrl( + const GURL& url, + CookieChangeCallback callback) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + + std::unique_ptr<Subscription> subscription = std::make_unique<Subscription>( + weak_ptr_factory_.GetWeakPtr(), DomainKey(url), + std::string(kGlobalNameKey), url, std::move(callback)); + + LinkSubscription(subscription.get()); + return subscription; } std::unique_ptr<CookieChangeSubscription> @@ -103,12 +143,12 @@ CookieMonsterChangeDispatcher::AddCallbackForAllChanges( CookieChangeCallback callback) { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - auto subscription = - std::make_unique<CookieMonsterChangeSubscription>(std::move(callback)); - subscription->SetCallbackSubscription(global_hook_map_.Add( - base::BindRepeating(&RunAsync, base::ThreadTaskRunnerHandle::Get(), - subscription->WeakCallback()))); - return std::move(subscription); + std::unique_ptr<Subscription> subscription = std::make_unique<Subscription>( + weak_ptr_factory_.GetWeakPtr(), std::string(kGlobalDomainKey), + std::string(kGlobalNameKey), GURL(""), std::move(callback)); + + LinkSubscription(subscription.get()); + return subscription; } void CookieMonsterChangeDispatcher::DispatchChange( @@ -117,24 +157,80 @@ void CookieMonsterChangeDispatcher::DispatchChange( bool notify_global_hooks) { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - CookieOptions opts; - opts.set_include_httponly(); - opts.set_same_site_cookie_mode( - CookieOptions::SameSiteCookieMode::INCLUDE_STRICT_AND_LAX); - // Note that the callbacks in hook_map_ are wrapped with RunAsync(), so they - // are guaranteed to not take long - they just post a RunAsync task back to - // the appropriate thread's message loop and return. - // TODO(mmenke): Consider running these synchronously? - for (const auto& key_value_pair : hook_map_) { - const std::pair<GURL, std::string>& key = key_value_pair.first; - if (cookie.IncludeForRequestURL(key.first, opts) && - cookie.Name() == key.second) { - key_value_pair.second->Notify(cookie, cause); - } + DispatchChangeToDomainKey(cookie, cause, DomainKey(cookie.Domain())); + if (notify_global_hooks) + DispatchChangeToDomainKey(cookie, cause, std::string(kGlobalDomainKey)); +} + +void CookieMonsterChangeDispatcher::DispatchChangeToDomainKey( + const CanonicalCookie& cookie, + CookieChangeCause cause, + const std::string& domain_key) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + + CookieDomainMap::iterator it = cookie_domain_map_.find(domain_key); + if (it == cookie_domain_map_.end()) + return; + + DispatchChangeToNameKey(cookie, cause, it->second, NameKey(cookie.Name())); + DispatchChangeToNameKey(cookie, cause, it->second, + std::string(kGlobalNameKey)); +} + +void CookieMonsterChangeDispatcher::DispatchChangeToNameKey( + const CanonicalCookie& cookie, + CookieChangeCause cause, + CookieNameMap& cookie_name_map, + const std::string& name_key) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + + CookieNameMap::iterator it = cookie_name_map.find(name_key); + if (it == cookie_name_map.end()) + return; + + SubscriptionList& subscription_list = it->second; + for (base::LinkNode<Subscription>* node = subscription_list.head(); + node != subscription_list.end(); node = node->next()) { + node->value()->DispatchChange(cookie, cause); } +} - if (notify_global_hooks) - global_hook_map_.Notify(cookie, cause); +void CookieMonsterChangeDispatcher::LinkSubscription( + Subscription* subscription) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + + // The subscript operator creates empty maps if the lookups fail. This is + // exactly what this method needs. + CookieNameMap& cookie_name_map = + cookie_domain_map_[subscription->domain_key()]; + SubscriptionList& subscription_list = + cookie_name_map[subscription->name_key()]; + subscription_list.Append(subscription); +} + +void CookieMonsterChangeDispatcher::UnlinkSubscription( + Subscription* subscription) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + + CookieDomainMap::iterator cookie_domain_map_iterator = + cookie_domain_map_.find(subscription->domain_key()); + DCHECK(cookie_domain_map_iterator != cookie_domain_map_.end()); + + CookieNameMap& cookie_name_map = cookie_domain_map_iterator->second; + CookieNameMap::iterator cookie_name_map_iterator = + cookie_name_map.find(subscription->name_key()); + DCHECK(cookie_name_map_iterator != cookie_name_map.end()); + + SubscriptionList& subscription_list = cookie_name_map_iterator->second; + subscription->RemoveFromList(); + if (!subscription_list.empty()) + return; + + cookie_name_map.erase(cookie_name_map_iterator); + if (!cookie_name_map.empty()) + return; + + cookie_domain_map_.erase(cookie_domain_map_iterator); } } // namespace net diff --git a/chromium/net/cookies/cookie_monster_change_dispatcher.h b/chromium/net/cookies/cookie_monster_change_dispatcher.h index 74abe23e408..caf35e8d91e 100644 --- a/chromium/net/cookies/cookie_monster_change_dispatcher.h +++ b/chromium/net/cookies/cookie_monster_change_dispatcher.h @@ -12,6 +12,10 @@ #include "base/callback.h" #include "base/callback_list.h" #include "base/compiler_specific.h" +#include "base/containers/linked_list.h" +#include "base/memory/scoped_refptr.h" +#include "base/memory/weak_ptr.h" +#include "base/single_thread_task_runner.h" #include "base/threading/thread_checker.h" #include "net/cookies/cookie_change_dispatcher.h" #include "url/gurl.h" @@ -30,32 +34,123 @@ class CookieMonsterChangeDispatcher : public CookieChangeDispatcher { CookieMonsterChangeDispatcher(); ~CookieMonsterChangeDispatcher() override; + // The key in CookieNameMap for a cookie name. + static std::string NameKey(std::string name); + + // The key in CookieDomainName for a cookie domain. + static std::string DomainKey(const std::string& domain); + + // The key in CookieDomainName for a listener URL. + static std::string DomainKey(const GURL& url); + // net::CookieChangeDispatcher std::unique_ptr<CookieChangeSubscription> AddCallbackForCookie( const GURL& url, const std::string& name, CookieChangeCallback callback) override WARN_UNUSED_RESULT; + std::unique_ptr<CookieChangeSubscription> AddCallbackForUrl( + const GURL& url, + CookieChangeCallback callback) override WARN_UNUSED_RESULT; std::unique_ptr<CookieChangeSubscription> AddCallbackForAllChanges( CookieChangeCallback callback) override WARN_UNUSED_RESULT; - // Dispatch a cookie change to all interested listeners. - // // |notify_global_hooks| is true if the function should run the // global hooks in addition to the per-cookie hooks. - // TODO(pwnall): Remove |notify_global_hooks|. + // + // TODO(pwnall): Remove |notify_global_hooks| and fix consumers. void DispatchChange(const CanonicalCookie& cookie, CookieChangeCause cause, bool notify_global_hooks); private: - using CookieChangeHookMap = - std::map<std::pair<GURL, std::string>, - std::unique_ptr<CookieChangeCallbackList>>; - CookieChangeHookMap hook_map_; - CookieChangeCallbackList global_hook_map_; + class Subscription : public base::LinkNode<Subscription>, + public CookieChangeSubscription { + public: + Subscription(base::WeakPtr<CookieMonsterChangeDispatcher> change_dispatcher, + std::string domain_key, + std::string name_key, + GURL url, + net::CookieChangeCallback callback); + + ~Subscription() override; + + // The lookup key used in the domain subscription map. + // + // The empty string means no domain filtering. + const std::string& domain_key() const { return domain_key_; } + // The lookup key used in the name subscription map. + // + // The empty string means no name filtering. + const std::string& name_key() const { return name_key_; } + + // Dispatches a cookie change notification if the listener is interested. + void DispatchChange(const net::CanonicalCookie& cookie, + net::CookieChangeCause change_cause); + + private: + base::WeakPtr<CookieMonsterChangeDispatcher> change_dispatcher_; + const std::string domain_key_; // kGlobalDomainKey means no filtering. + const std::string name_key_; // kGlobalNameKey means no filtering. + const GURL url_; // empty() means no URL-based filtering. + net::CookieOptions options_; + const net::CookieChangeCallback callback_; + + void DoDispatchChange(const net::CanonicalCookie& cookie, + net::CookieChangeCause change_cause) const; + + // Used to post DoDispatchChange() calls to this subscription's thread. + scoped_refptr<base::SingleThreadTaskRunner> task_runner_; + + THREAD_CHECKER(thread_checker_); + + // Used to cancel delayed calls to DoDispatchChange() when the subscription + // gets destroyed. + base::WeakPtrFactory<Subscription> weak_ptr_factory_; + + DISALLOW_COPY_AND_ASSIGN(Subscription); + }; + + // The last level of the subscription data structures. + using SubscriptionList = base::LinkedList<Subscription>; + + // Buckets subscriptions according to cookie names. + // + // Map keys are cookie names, as we only support exact name matching. + using CookieNameMap = std::map<std::string, SubscriptionList>; + + // Buckets subscriptions according to cookie domains. + // + // Map keys are the eTLD+1 of cookie domains. Cookies are either host-locked, + // or visible to all the subdomain of a given domain. A cookie's scope cannot + // exceed eTLD+1, so we stop there. + using CookieDomainMap = std::map<std::string, CookieNameMap>; + + void DispatchChangeToDomainKey(const CanonicalCookie& cookie, + CookieChangeCause cause, + const std::string& domain_key); + + void DispatchChangeToNameKey(const CanonicalCookie& cookie, + CookieChangeCause cause, + CookieNameMap& name_map, + const std::string& name_key); + + // Inserts a subscription into the map. + // + // Called by the AddCallback* methods, after creating the Subscription. + void LinkSubscription(Subscription* subscription); + + // Removes a subscription from the map. + // + // Called by the Subscription destructor. + void UnlinkSubscription(Subscription* subscription); + + CookieDomainMap cookie_domain_map_; THREAD_CHECKER(thread_checker_); + // Vends weak pointers to subscriptions. + base::WeakPtrFactory<CookieMonsterChangeDispatcher> weak_ptr_factory_; + DISALLOW_COPY_AND_ASSIGN(CookieMonsterChangeDispatcher); }; diff --git a/chromium/net/cookies/cookie_monster_store_test.h b/chromium/net/cookies/cookie_monster_store_test.h index 6d8a8335a4b..05bdb93dcc6 100644 --- a/chromium/net/cookies/cookie_monster_store_test.h +++ b/chromium/net/cookies/cookie_monster_store_test.h @@ -137,6 +137,7 @@ void AddCookieToList(const GURL& url, // Just act like a backing database. Keep cookie information from // Add/Update/Delete and regurgitate it when Load is called. +// TODO(morlovich): This still assumes that creation times are unique. class MockSimplePersistentCookieStore : public CookieMonster::PersistentCookieStore { public: diff --git a/chromium/net/cookies/cookie_monster_unittest.cc b/chromium/net/cookies/cookie_monster_unittest.cc index dbd8f8c1d57..84a386da82e 100644 --- a/chromium/net/cookies/cookie_monster_unittest.cc +++ b/chromium/net/cookies/cookie_monster_unittest.cc @@ -11,6 +11,7 @@ #include <vector> #include "base/bind.h" +#include "base/bind_helpers.h" #include "base/containers/queue.h" #include "base/location.h" #include "base/memory/ref_counted.h" @@ -18,6 +19,7 @@ #include "base/metrics/histogram_samples.h" #include "base/run_loop.h" #include "base/single_thread_task_runner.h" +#include "base/strings/strcat.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_piece.h" #include "base/strings/string_split.h" @@ -34,6 +36,7 @@ #include "net/cookies/cookie_constants.h" #include "net/cookies/cookie_monster_store_test.h" // For CookieStore mock #include "net/cookies/cookie_store_change_unittest.h" +#include "net/cookies/cookie_store_test_helpers.h" #include "net/cookies/cookie_store_unittest.h" #include "net/cookies/cookie_util.h" #include "net/cookies/parsed_cookie.h" @@ -110,7 +113,7 @@ struct CookieMonsterTestTraits { return std::make_unique<CookieMonster>(nullptr); } - static void RunUntilIdle() { base::RunLoop().RunUntilIdle(); } + static void DeliverChangeNotifications() { base::RunLoop().RunUntilIdle(); } static const bool supports_http_only = true; static const bool supports_non_dotted_domains = true; @@ -119,8 +122,11 @@ struct CookieMonsterTestTraits { static const bool has_path_prefix_bug = false; static const bool forbids_setting_empty_name = false; static const bool supports_global_cookie_tracking = true; + static const bool supports_url_cookie_tracking = true; static const bool supports_named_cookie_tracking = true; static const bool supports_multiple_tracking_callbacks = true; + static const bool has_exact_change_cause = true; + static const bool has_exact_change_ordering = true; static const int creation_time_granularity_in_ms = 0; }; @@ -128,7 +134,13 @@ INSTANTIATE_TYPED_TEST_CASE_P(CookieMonster, CookieStoreTest, CookieMonsterTestTraits); INSTANTIATE_TYPED_TEST_CASE_P(CookieMonster, - CookieStoreChangeTest, + CookieStoreChangeGlobalTest, + CookieMonsterTestTraits); +INSTANTIATE_TYPED_TEST_CASE_P(CookieMonster, + CookieStoreChangeUrlTest, + CookieMonsterTestTraits); +INSTANTIATE_TYPED_TEST_CASE_P(CookieMonster, + CookieStoreChangeNamedTest, CookieMonsterTestTraits); template <typename T> @@ -1853,11 +1865,11 @@ TEST_F(CookieMonsterTest, DontImportDuplicateCookies) { } // Tests importing from a persistent cookie store that contains cookies -// with duplicate creation times. This situation should be handled by -// dropping the cookies before insertion/visibility to user. +// with duplicate creation times. This is OK now, but it still interacts +// with the de-duplication algorithm. // // This is a regression test for: http://crbug.com/43188. -TEST_F(CookieMonsterTest, DontImportDuplicateCreationTimes) { +TEST_F(CookieMonsterTest, ImportDuplicateCreationTimes) { scoped_refptr<MockPersistentCookieStore> store(new MockPersistentCookieStore); Time now(Time::Now()); @@ -2063,6 +2075,41 @@ TEST_F(CookieMonsterTest, BackingStoreCommunication) { } } +TEST_F(CookieMonsterTest, RestoreDifferentCookieSameCreationTime) { + // Test that we can restore different cookies with duplicate creation times. + base::Time current(base::Time::Now()); + scoped_refptr<MockPersistentCookieStore> store = + base::MakeRefCounted<MockPersistentCookieStore>(); + + { + CookieMonster cmout(store.get()); + GURL url("http://www.example.com/"); + EXPECT_TRUE( + SetCookieWithCreationTime(&cmout, url, "A=1; max-age=600", current)); + EXPECT_TRUE( + SetCookieWithCreationTime(&cmout, url, "B=2; max-age=600", current)); + } + + // Play back the cookies into store 2. + scoped_refptr<MockPersistentCookieStore> store2 = + base::MakeRefCounted<MockPersistentCookieStore>(); + std::vector<std::unique_ptr<CanonicalCookie>> load_expectation; + EXPECT_EQ(2u, store->commands().size()); + for (const CookieStoreCommand& command : store->commands()) { + ASSERT_EQ(command.type, CookieStoreCommand::ADD); + load_expectation.push_back( + std::make_unique<CanonicalCookie>(command.cookie)); + } + store2->SetLoadExpectation(true, std::move(load_expectation)); + + // Now read them in. Should get two cookies, not one. + { + CookieMonster cmin(store2.get()); + CookieList cookies(GetAllCookies(&cmin)); + ASSERT_EQ(2u, cookies.size()); + } +} + TEST_F(CookieMonsterTest, CookieListOrdering) { // Put a random set of cookies into a monster and make sure // they're returned in the right order. @@ -2393,67 +2440,11 @@ TEST_F(CookieMonsterTest, CheckOrderOfCookieTaskQueueWhenLoadingCompletes) { EXPECT_EQ(1u, get_cookie_list_callback2.cookies().size()); } -namespace { - -// Mock PersistentCookieStore that keeps track of the number of Flush() calls. -class FlushablePersistentStore : public CookieMonster::PersistentCookieStore { - public: - FlushablePersistentStore() : flush_count_(0) {} - - void Load(const LoadedCallback& loaded_callback) override { - std::vector<std::unique_ptr<CanonicalCookie>> out_cookies; - base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::Bind(loaded_callback, base::Passed(&out_cookies))); - } - - void LoadCookiesForKey(const std::string& key, - const LoadedCallback& loaded_callback) override { - Load(loaded_callback); - } - - void AddCookie(const CanonicalCookie&) override {} - void UpdateCookieAccessTime(const CanonicalCookie&) override {} - void DeleteCookie(const CanonicalCookie&) override {} - void SetForceKeepSessionState() override {} - void SetBeforeFlushCallback(base::RepeatingClosure callback) override {} - - void Flush(base::OnceClosure callback) override { - ++flush_count_; - if (!callback.is_null()) - std::move(callback).Run(); - } - - int flush_count() { return flush_count_; } - - private: - ~FlushablePersistentStore() override = default; - - volatile int flush_count_; -}; - -// Counts the number of times Callback() has been run. -class CallbackCounter : public base::RefCountedThreadSafe<CallbackCounter> { - public: - CallbackCounter() : callback_count_(0) {} - - void Callback() { ++callback_count_; } - - int callback_count() { return callback_count_; } - - private: - friend class base::RefCountedThreadSafe<CallbackCounter>; - ~CallbackCounter() = default; - - volatile int callback_count_; -}; - -} // namespace - // Test that FlushStore() is forwarded to the store and callbacks are posted. TEST_F(CookieMonsterTest, FlushStore) { - scoped_refptr<CallbackCounter> counter(new CallbackCounter()); - scoped_refptr<FlushablePersistentStore> store(new FlushablePersistentStore()); - std::unique_ptr<CookieMonster> cm(new CookieMonster(store.get())); + auto counter = base::MakeRefCounted<CallbackCounter>(); + auto store = base::MakeRefCounted<FlushablePersistentStore>(); + auto cm = std::make_unique<CookieMonster>(store); ASSERT_EQ(0, store->flush_count()); ASSERT_EQ(0, counter->callback_count()); @@ -2481,7 +2472,7 @@ TEST_F(CookieMonsterTest, FlushStore) { ASSERT_EQ(2, counter->callback_count()); // NULL callback is still safe. - cm->FlushStore(base::Closure()); + cm->FlushStore(base::DoNothing()); base::RunLoop().RunUntilIdle(); ASSERT_EQ(2, store->flush_count()); @@ -2490,7 +2481,7 @@ TEST_F(CookieMonsterTest, FlushStore) { // If there's no backing store, FlushStore() is always a safe no-op. cm.reset(new CookieMonster(nullptr)); GetAllCookies(cm.get()); // Force init. - cm->FlushStore(base::Closure()); + cm->FlushStore(base::DoNothing()); base::RunLoop().RunUntilIdle(); ASSERT_EQ(2, counter->callback_count()); @@ -3152,6 +3143,47 @@ TEST_F(CookieMonsterTest, SetCanonicalCookieDoesNotBlockForLoadAll) { } } +TEST_F(CookieMonsterTest, DeleteDuplicateCTime) { + const char* const kNames[] = {"A", "B", "C"}; + const size_t kNum = arraysize(kNames); + + // Tests that DeleteCanonicalCookie properly distinguishes different cookies + // (e.g. different name or path) with identical ctime on same domain. + // This gets tested a few times with different deletion target, to make sure + // that the implementation doesn't just happen to pick the right one because + // of implementation details. + for (size_t run = 0; run < kNum; ++run) { + CookieMonster cm(nullptr); + Time now = Time::Now(); + GURL url("http://www.example.com"); + + for (size_t i = 0; i < kNum; ++i) { + std::string cookie_string = + base::StrCat({kNames[i], "=", base::NumberToString(i)}); + EXPECT_TRUE(SetCookieWithCreationTime(&cm, url, cookie_string, now)); + } + + // Delete the run'th cookie. + CookieList all_cookies = + GetAllCookiesForURLWithOptions(&cm, url, CookieOptions()); + ASSERT_EQ(all_cookies.size(), kNum); + for (size_t i = 0; i < kNum; ++i) { + const CanonicalCookie& cookie = all_cookies[i]; + if (cookie.Name() == kNames[run]) { + EXPECT_TRUE(DeleteCanonicalCookie(&cm, cookie)); + } + } + + // Check that the right cookie got removed. + all_cookies = GetAllCookiesForURLWithOptions(&cm, url, CookieOptions()); + ASSERT_EQ(all_cookies.size(), kNum - 1); + for (size_t i = 0; i < kNum - 1; ++i) { + const CanonicalCookie& cookie = all_cookies[i]; + EXPECT_NE(cookie.Name(), kNames[run]); + } + } +} + class CookieMonsterNotificationTest : public CookieMonsterTest { public: CookieMonsterNotificationTest() diff --git a/chromium/net/cookies/cookie_store.cc b/chromium/net/cookies/cookie_store.cc index 6d273b94799..e536dbd276f 100644 --- a/chromium/net/cookies/cookie_store.cc +++ b/chromium/net/cookies/cookie_store.cc @@ -40,6 +40,10 @@ int CookieStore::GetChannelIDServiceID() { return channel_id_service_id_; } +void CookieStore::DumpMemoryStats( + base::trace_event::ProcessMemoryDump* pmd, + const std::string& parent_absolute_name) const {} + CookieStore::CookieStore() : channel_id_service_id_(-1) {} } // namespace net diff --git a/chromium/net/cookies/cookie_store.h b/chromium/net/cookies/cookie_store.h index cfe57f7f6b7..980da01ced5 100644 --- a/chromium/net/cookies/cookie_store.h +++ b/chromium/net/cookies/cookie_store.h @@ -21,6 +21,12 @@ class GURL; +namespace base { +namespace trace_event { +class ProcessMemoryDump; +} +} // namespace base + namespace net { class CookieChangeDispatcher; @@ -146,6 +152,10 @@ class NET_EXPORT CookieStore { void SetChannelIDServiceID(int id); int GetChannelIDServiceID(); + // Reports the estimate of dynamically allocated memory in bytes. + virtual void DumpMemoryStats(base::trace_event::ProcessMemoryDump* pmd, + const std::string& parent_absolute_name) const; + protected: CookieStore(); int channel_id_service_id_; diff --git a/chromium/net/cookies/cookie_store_change_unittest.h b/chromium/net/cookies/cookie_store_change_unittest.h index a613bb4c1b4..32df87dfa12 100644 --- a/chromium/net/cookies/cookie_store_change_unittest.h +++ b/chromium/net/cookies/cookie_store_change_unittest.h @@ -7,6 +7,7 @@ #include "base/bind.h" #include "net/cookies/canonical_cookie.h" +#include "net/cookies/cookie_change_dispatcher_test_helpers.h" #include "net/cookies/cookie_store.h" #include "net/cookies/cookie_store_unittest.h" #include "testing/gtest/include/gtest/gtest.h" @@ -14,24 +15,107 @@ namespace net { -template <class CookieStoreTestTraits> -class CookieStoreChangeTest : public CookieStoreTest<CookieStoreTestTraits> {}; -TYPED_TEST_CASE_P(CookieStoreChangeTest); - namespace { using CookieChange = std::pair<CanonicalCookie, CookieChangeCause>; -void OnCookieChange(std::vector<CookieChange>* changes, - const CanonicalCookie& cookie, - CookieChangeCause cause) { - CookieChange notification(cookie, cause); - changes->push_back(notification); +// Used to sort CookieChanges when testing stores without exact change ordering. +// +// The ordering relation must match the order in which the tests below issue +// cookie calls. Changes to this method should be tested by running the tests +// below with CookieMonsterTestTraits::has_exact_change_ordering set to both +// true and false. +bool CookieChangeLessThan(const CookieChange& lhs, const CookieChange& rhs) { + if (lhs.first.Name() != rhs.first.Name()) + return lhs.first.Name() < rhs.first.Name(); + + if (lhs.first.Value() != rhs.first.Value()) + return lhs.first.Value() < rhs.first.Value(); + + if (lhs.first.Domain() != rhs.first.Domain()) + return lhs.first.Domain() < rhs.first.Domain(); + + return lhs.second < rhs.second; } } // namespace -TYPED_TEST_P(CookieStoreChangeTest, Global_NoCookie) { +// Google Test supports at most 50 tests per typed case, so the tests here are +// broken up into multiple cases. +template <class CookieStoreTestTraits> +class CookieStoreChangeTestBase + : public CookieStoreTest<CookieStoreTestTraits> { + protected: + using CookieStoreTest<CookieStoreTestTraits>::FindAndDeleteCookie; + + // Drains all pending tasks on the run loop(s) involved in the test. + void DeliverChangeNotifications() { + CookieStoreTestTraits::DeliverChangeNotifications(); + } + + bool FindAndDeleteCookie(CookieStore* cs, + const std::string& domain, + const std::string& name, + const std::string& path) { + for (auto& cookie : this->GetAllCookies(cs)) { + if (cookie.Domain() == domain && cookie.Name() == name && + cookie.Path() == path) { + return this->DeleteCanonicalCookie(cs, cookie); + } + } + + return false; + } + + // Could be static, but it's actually easier to have it be a member function. + ::testing::AssertionResult MatchesCause(CookieChangeCause expected_cause, + CookieChangeCause actual_cause) { + if (!CookieChangeCauseIsDeletion(expected_cause) || + CookieStoreTestTraits::has_exact_change_cause) { + if (expected_cause == actual_cause) + return ::testing::AssertionSuccess(); + return ::testing::AssertionFailure() + << "expected " << expected_cause << " got " << actual_cause; + } + if (CookieChangeCauseIsDeletion(actual_cause)) + return ::testing::AssertionSuccess(); + return ::testing::AssertionFailure() + << "expected a deletion cause, got " << actual_cause; + } + + static void OnCookieChange(std::vector<CookieChange>* changes, + const CanonicalCookie& cookie, + CookieChangeCause cause) { + CookieChange notification(cookie, cause); + + if (CookieStoreTestTraits::has_exact_change_ordering) { + changes->push_back(notification); + } else { + // Assumes the vector is sorted before the insertion. If true, the vector + // will remain sorted. + changes->insert(std::upper_bound(changes->begin(), changes->end(), + notification, CookieChangeLessThan), + notification); + } + } +}; + +template <class CookieStoreTestTraits> +class CookieStoreChangeGlobalTest + : public CookieStoreChangeTestBase<CookieStoreTestTraits> {}; +TYPED_TEST_CASE_P(CookieStoreChangeGlobalTest); + +template <class CookieStoreTestTraits> +class CookieStoreChangeUrlTest + : public CookieStoreChangeTestBase<CookieStoreTestTraits> {}; +TYPED_TEST_CASE_P(CookieStoreChangeUrlTest); + +template <class CookieStoreTestTraits> +class CookieStoreChangeNamedTest + : public CookieStoreChangeTestBase<CookieStoreTestTraits> {}; +TYPED_TEST_CASE_P(CookieStoreChangeNamedTest); + +TYPED_TEST_P(CookieStoreChangeGlobalTest, NoCookie) { if (!TypeParam::supports_global_cookie_tracking) return; @@ -39,27 +123,29 @@ TYPED_TEST_P(CookieStoreChangeTest, Global_NoCookie) { std::vector<CookieChange> cookie_changes; std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForAllChanges(base::BindRepeating( - &OnCookieChange, base::Unretained(&cookie_changes))); - this->RunUntilIdle(); + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); EXPECT_EQ(0u, cookie_changes.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Global_InitialCookie) { +TYPED_TEST_P(CookieStoreChangeGlobalTest, InitialCookie) { if (!TypeParam::supports_global_cookie_tracking) return; CookieStore* cs = this->GetCookieStore(); std::vector<CookieChange> cookie_changes; - this->SetCookie(cs, this->http_www_foo_.url(), "abc=def"); - this->RunUntilIdle(); + this->SetCookie(cs, this->http_www_foo_.url(), "A=B"); + this->DeliverChangeNotifications(); std::unique_ptr<CookieChangeSubscription> subscription( cs->GetChangeDispatcher().AddCallbackForAllChanges(base::BindRepeating( - &OnCookieChange, base::Unretained(&cookie_changes)))); - this->RunUntilIdle(); + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes)))); + this->DeliverChangeNotifications(); EXPECT_EQ(0u, cookie_changes.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Global_Insert) { +TYPED_TEST_P(CookieStoreChangeGlobalTest, InsertOne) { if (!TypeParam::supports_global_cookie_tracking) return; @@ -67,75 +153,150 @@ TYPED_TEST_P(CookieStoreChangeTest, Global_Insert) { std::vector<CookieChange> cookie_changes; std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForAllChanges(base::BindRepeating( - &OnCookieChange, base::Unretained(&cookie_changes))); + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); + ASSERT_EQ(0u, cookie_changes.size()); + + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + this->DeliverChangeNotifications(); + + ASSERT_EQ(1u, cookie_changes.size()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[0].second)); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); + EXPECT_EQ("A", cookie_changes[0].first.Name()); + EXPECT_EQ("B", cookie_changes[0].first.Value()); +} + +TYPED_TEST_P(CookieStoreChangeGlobalTest, InsertMany) { + if (!TypeParam::supports_global_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForAllChanges(base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "C=D")); EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "E=F")); EXPECT_TRUE(this->SetCookie(cs, this->http_bar_com_.url(), "G=H")); + this->DeliverChangeNotifications(); + + // Check that the cookie changes are dispatched before calling GetCookies. + // This is not an ASSERT because the following expectations produce useful + // debugging information if they fail. + EXPECT_EQ(4u, cookie_changes.size()); EXPECT_EQ("A=B; C=D; E=F", this->GetCookies(cs, this->http_www_foo_.url())); EXPECT_EQ("G=H", this->GetCookies(cs, this->http_bar_com_.url())); - this->RunUntilIdle(); - ASSERT_EQ(4u, cookie_changes.size()); - EXPECT_EQ(CookieChangeCause::INSERTED, cookie_changes[0].second); + + ASSERT_LE(1u, cookie_changes.size()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[0].second)); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); EXPECT_EQ("A", cookie_changes[0].first.Name()); EXPECT_EQ("B", cookie_changes[0].first.Value()); + + ASSERT_LE(2u, cookie_changes.size()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[1].first.Domain()); - EXPECT_EQ(CookieChangeCause::INSERTED, cookie_changes[1].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[1].second)); EXPECT_EQ("C", cookie_changes[1].first.Name()); EXPECT_EQ("D", cookie_changes[1].first.Value()); + + ASSERT_LE(3u, cookie_changes.size()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[2].first.Domain()); - EXPECT_EQ(CookieChangeCause::INSERTED, cookie_changes[2].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[2].second)); EXPECT_EQ("E", cookie_changes[2].first.Name()); EXPECT_EQ("F", cookie_changes[2].first.Value()); + + ASSERT_LE(4u, cookie_changes.size()); EXPECT_EQ(this->http_bar_com_.url().host(), cookie_changes[3].first.Domain()); - EXPECT_EQ(CookieChangeCause::INSERTED, cookie_changes[3].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[3].second)); EXPECT_EQ("G", cookie_changes[3].first.Name()); EXPECT_EQ("H", cookie_changes[3].first.Value()); + + EXPECT_EQ(4u, cookie_changes.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Global_Delete) { +TYPED_TEST_P(CookieStoreChangeGlobalTest, DeleteOne) { if (!TypeParam::supports_global_cookie_tracking) return; CookieStore* cs = this->GetCookieStore(); std::vector<CookieChange> cookie_changes; - EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); - EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "C=D")); - EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "E=F")); - EXPECT_TRUE(this->SetCookie(cs, this->http_bar_com_.url(), "G=H")); - std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForAllChanges(base::BindRepeating( - &OnCookieChange, base::Unretained(&cookie_changes))); - this->RunUntilIdle(); - ASSERT_EQ(0u, cookie_changes.size()); + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + this->DeliverChangeNotifications(); + EXPECT_EQ(1u, cookie_changes.size()); + cookie_changes.clear(); EXPECT_TRUE( - this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), "C")); - EXPECT_EQ("A=B; E=F", this->GetCookies(cs, this->http_www_foo_.url())); - EXPECT_EQ("G=H", this->GetCookies(cs, this->http_bar_com_.url())); - this->RunUntilIdle(); + this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), "A")); + this->DeliverChangeNotifications(); + ASSERT_EQ(1u, cookie_changes.size()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); - EXPECT_EQ(CookieChangeCause::EXPLICIT, cookie_changes[0].second); - EXPECT_EQ("C", cookie_changes[0].first.Name()); - EXPECT_EQ("D", cookie_changes[0].first.Value()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::EXPLICIT, + cookie_changes[0].second)); + EXPECT_EQ("A", cookie_changes[0].first.Name()); + EXPECT_EQ("B", cookie_changes[0].first.Value()); +} + +TYPED_TEST_P(CookieStoreChangeGlobalTest, DeleteTwo) { + if (!TypeParam::supports_global_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForAllChanges(base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "C=D")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "E=F")); + EXPECT_TRUE(this->SetCookie(cs, this->http_bar_com_.url(), "G=H")); + this->DeliverChangeNotifications(); + EXPECT_EQ(4u, cookie_changes.size()); cookie_changes.clear(); EXPECT_TRUE( + this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), "C")); + EXPECT_TRUE( this->FindAndDeleteCookie(cs, this->http_bar_com_.url().host(), "G")); + this->DeliverChangeNotifications(); + + // Check that the cookie changes are dispatched before calling GetCookies. + // This is not an ASSERT because the following expectations produce useful + // debugging information if they fail. + EXPECT_EQ(2u, cookie_changes.size()); EXPECT_EQ("A=B; E=F", this->GetCookies(cs, this->http_www_foo_.url())); EXPECT_EQ("", this->GetCookies(cs, this->http_bar_com_.url())); - this->RunUntilIdle(); - ASSERT_EQ(1u, cookie_changes.size()); - EXPECT_EQ(this->http_bar_com_.url().host(), cookie_changes[0].first.Domain()); - EXPECT_EQ(CookieChangeCause::EXPLICIT, cookie_changes[0].second); - EXPECT_EQ("G", cookie_changes[0].first.Name()); - EXPECT_EQ("H", cookie_changes[0].first.Value()); + + ASSERT_LE(1u, cookie_changes.size()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::EXPLICIT, + cookie_changes[0].second)); + EXPECT_EQ("C", cookie_changes[0].first.Name()); + EXPECT_EQ("D", cookie_changes[0].first.Value()); + + ASSERT_EQ(2u, cookie_changes.size()); + EXPECT_EQ(this->http_bar_com_.url().host(), cookie_changes[1].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::EXPLICIT, + cookie_changes[1].second)); + EXPECT_EQ("G", cookie_changes[1].first.Name()); + EXPECT_EQ("H", cookie_changes[1].first.Value()); } -TYPED_TEST_P(CookieStoreChangeTest, Global_Overwrite) { +TYPED_TEST_P(CookieStoreChangeGlobalTest, Overwrite) { if (!TypeParam::supports_global_cookie_tracking) return; @@ -143,31 +304,39 @@ TYPED_TEST_P(CookieStoreChangeTest, Global_Overwrite) { std::vector<CookieChange> cookie_changes; std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForAllChanges(base::BindRepeating( - &OnCookieChange, base::Unretained(&cookie_changes))); - this->RunUntilIdle(); + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes.size()); EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes.size()); cookie_changes.clear(); // Replacing an existing cookie is actually a two-phase delete + set // operation, so we get an extra notification. EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=C")); - this->RunUntilIdle(); - ASSERT_EQ(2u, cookie_changes.size()); + this->DeliverChangeNotifications(); + + ASSERT_LE(1u, cookie_changes.size()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); - EXPECT_EQ(CookieChangeCause::OVERWRITE, cookie_changes[0].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::OVERWRITE, + cookie_changes[0].second)); EXPECT_EQ("A", cookie_changes[0].first.Name()); EXPECT_EQ("B", cookie_changes[0].first.Value()); + + ASSERT_LE(2u, cookie_changes.size()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[1].first.Domain()); - EXPECT_EQ(CookieChangeCause::INSERTED, cookie_changes[1].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[1].second)); EXPECT_EQ("A", cookie_changes[1].first.Name()); EXPECT_EQ("C", cookie_changes[1].first.Value()); + + EXPECT_EQ(2u, cookie_changes.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Global_OverwriteWithHttpOnly) { +TYPED_TEST_P(CookieStoreChangeGlobalTest, OverwriteWithHttpOnly) { if (!TypeParam::supports_global_cookie_tracking) return; @@ -176,18 +345,21 @@ TYPED_TEST_P(CookieStoreChangeTest, Global_OverwriteWithHttpOnly) { std::vector<CookieChange> cookie_changes; std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForAllChanges(base::BindRepeating( - &OnCookieChange, base::Unretained(&cookie_changes))); - this->RunUntilIdle(); + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes.size()); EXPECT_TRUE( this->SetCookie(cs, this->http_www_foo_.url(), "A=B; path=/path1")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes.size()); - EXPECT_EQ(CookieChangeCause::INSERTED, cookie_changes[0].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[0].second)); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); EXPECT_EQ("A", cookie_changes[0].first.Name()); EXPECT_EQ("B", cookie_changes[0].first.Value()); + EXPECT_FALSE(cookie_changes[0].first.IsHttpOnly()); cookie_changes.clear(); // Insert a cookie "A" for path "/path1", that is httponly. This should @@ -197,19 +369,28 @@ TYPED_TEST_P(CookieStoreChangeTest, Global_OverwriteWithHttpOnly) { EXPECT_TRUE(this->SetCookieWithOptions(cs, this->http_www_foo_.url(), "A=C; path=/path1; httponly", allow_httponly)); - this->RunUntilIdle(); - ASSERT_EQ(2u, cookie_changes.size()); + this->DeliverChangeNotifications(); + + ASSERT_LE(1u, cookie_changes.size()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); - EXPECT_EQ(CookieChangeCause::OVERWRITE, cookie_changes[0].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::OVERWRITE, + cookie_changes[0].second)); EXPECT_EQ("A", cookie_changes[0].first.Name()); EXPECT_EQ("B", cookie_changes[0].first.Value()); + EXPECT_FALSE(cookie_changes[0].first.IsHttpOnly()); + + ASSERT_LE(2u, cookie_changes.size()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[1].first.Domain()); - EXPECT_EQ(CookieChangeCause::INSERTED, cookie_changes[1].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[1].second)); EXPECT_EQ("A", cookie_changes[1].first.Name()); EXPECT_EQ("C", cookie_changes[1].first.Value()); + EXPECT_TRUE(cookie_changes[1].first.IsHttpOnly()); + + EXPECT_EQ(2u, cookie_changes.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Global_Deregister) { +TYPED_TEST_P(CookieStoreChangeGlobalTest, Deregister) { if (!TypeParam::supports_global_cookie_tracking) return; @@ -218,13 +399,14 @@ TYPED_TEST_P(CookieStoreChangeTest, Global_Deregister) { std::vector<CookieChange> cookie_changes; std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForAllChanges(base::BindRepeating( - &OnCookieChange, base::Unretained(&cookie_changes))); - this->RunUntilIdle(); + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes.size()); // Insert a cookie and make sure it is seen. EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes.size()); EXPECT_EQ("A", cookie_changes[0].first.Name()); EXPECT_EQ("B", cookie_changes[0].first.Value()); @@ -235,12 +417,12 @@ TYPED_TEST_P(CookieStoreChangeTest, Global_Deregister) { // Insert a second cookie and make sure that it's not visible. EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "C=D")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); EXPECT_EQ(0u, cookie_changes.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Global_DeregisterMultiple) { +TYPED_TEST_P(CookieStoreChangeGlobalTest, DeregisterMultiple) { if (!TypeParam::supports_global_cookie_tracking || !TypeParam::supports_multiple_tracking_callbacks) return; @@ -251,19 +433,21 @@ TYPED_TEST_P(CookieStoreChangeTest, Global_DeregisterMultiple) { std::vector<CookieChange> cookie_changes_1; std::unique_ptr<CookieChangeSubscription> subscription1 = cs->GetChangeDispatcher().AddCallbackForAllChanges(base::BindRepeating( - &OnCookieChange, base::Unretained(&cookie_changes_1))); + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); std::vector<CookieChange> cookie_changes_2; std::unique_ptr<CookieChangeSubscription> subscription2 = cs->GetChangeDispatcher().AddCallbackForAllChanges(base::BindRepeating( - &OnCookieChange, base::Unretained(&cookie_changes_2))); - this->RunUntilIdle(); + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes_1.size()); ASSERT_EQ(0u, cookie_changes_2.size()); // Insert a cookie and make sure it's seen. EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ("A", cookie_changes_1[0].first.Name()); EXPECT_EQ("B", cookie_changes_1[0].first.Value()); @@ -280,7 +464,7 @@ TYPED_TEST_P(CookieStoreChangeTest, Global_DeregisterMultiple) { // Insert a second cookie and make sure that it's only visible in one // change array. EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "C=D")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ("C", cookie_changes_1[0].first.Name()); EXPECT_EQ("D", cookie_changes_1[0].first.Value()); @@ -289,10 +473,39 @@ TYPED_TEST_P(CookieStoreChangeTest, Global_DeregisterMultiple) { ASSERT_EQ(0u, cookie_changes_2.size()); } -// Confirm that deregistering a subscription blocks the notification -// if the deregistration happened after the change but before the -// notification was received. -TYPED_TEST_P(CookieStoreChangeTest, Global_DeregisterRace) { +// Confirm that a listener does not receive notifications for changes that +// happened right before the subscription was established. +TYPED_TEST_P(CookieStoreChangeGlobalTest, DispatchRace) { + if (!TypeParam::supports_global_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + + // This cookie insertion should not be seen. + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + // DeliverChangeNotifications() must NOT be called before the subscription is + // established. + + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForAllChanges(base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "C=D")); + this->DeliverChangeNotifications(); + + EXPECT_LE(1u, cookie_changes.size()); + EXPECT_EQ("C", cookie_changes[0].first.Name()); + EXPECT_EQ("D", cookie_changes[0].first.Value()); + + ASSERT_EQ(1u, cookie_changes.size()); +} + +// Confirm that deregistering a subscription blocks the notification if the +// deregistration happened after the change but before the notification was +// received. +TYPED_TEST_P(CookieStoreChangeGlobalTest, DeregisterRace) { if (!TypeParam::supports_global_cookie_tracking) return; @@ -301,13 +514,14 @@ TYPED_TEST_P(CookieStoreChangeTest, Global_DeregisterRace) { std::vector<CookieChange> cookie_changes; std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForAllChanges(base::BindRepeating( - &OnCookieChange, base::Unretained(&cookie_changes))); - this->RunUntilIdle(); + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes.size()); // Insert a cookie and make sure it's seen. EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes.size()); EXPECT_EQ("A", cookie_changes[0].first.Name()); EXPECT_EQ("B", cookie_changes[0].first.Value()); @@ -328,11 +542,11 @@ TYPED_TEST_P(CookieStoreChangeTest, Global_DeregisterRace) { // valid. Destroy the subscription so as to lose the race and make sure the // task posted arrives after the subscription was destroyed. subscription.reset(); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Global_DeregisterRaceMultiple) { +TYPED_TEST_P(CookieStoreChangeGlobalTest, DeregisterRaceMultiple) { if (!TypeParam::supports_global_cookie_tracking || !TypeParam::supports_multiple_tracking_callbacks) return; @@ -343,17 +557,19 @@ TYPED_TEST_P(CookieStoreChangeTest, Global_DeregisterRaceMultiple) { std::vector<CookieChange> cookie_changes_1, cookie_changes_2; std::unique_ptr<CookieChangeSubscription> subscription1 = cs->GetChangeDispatcher().AddCallbackForAllChanges(base::BindRepeating( - &OnCookieChange, base::Unretained(&cookie_changes_1))); + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); std::unique_ptr<CookieChangeSubscription> subscription2 = cs->GetChangeDispatcher().AddCallbackForAllChanges(base::BindRepeating( - &OnCookieChange, base::Unretained(&cookie_changes_2))); - this->RunUntilIdle(); + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes_1.size()); ASSERT_EQ(0u, cookie_changes_2.size()); // Insert a cookie and make sure it's seen. EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ("A", cookie_changes_1[0].first.Name()); @@ -372,17 +588,15 @@ TYPED_TEST_P(CookieStoreChangeTest, Global_DeregisterRaceMultiple) { // Note that by the API contract it's perfectly valid to have received the // notification immediately, i.e. synchronously with the cookie change. In // that case, there's nothing to test. - if (1u == cookie_changes_2.size()) { - LOG(ERROR) << "Nothing to test."; + if (1u == cookie_changes_2.size()) return; - } // A task was posted by the SetCookie() above, but has not yet arrived. If it // arrived before the subscription is destroyed, callback execution would be // valid. Destroy one of the subscriptions so as to lose the race and make // sure the task posted arrives after the subscription was destroyed. subscription2.reset(); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ("C", cookie_changes_1[0].first.Name()); EXPECT_EQ("D", cookie_changes_1[0].first.Value()); @@ -391,7 +605,7 @@ TYPED_TEST_P(CookieStoreChangeTest, Global_DeregisterRaceMultiple) { ASSERT_EQ(0u, cookie_changes_2.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Global_MultipleSubscriptions) { +TYPED_TEST_P(CookieStoreChangeGlobalTest, MultipleSubscriptions) { if (!TypeParam::supports_global_cookie_tracking || !TypeParam::supports_multiple_tracking_callbacks) return; @@ -401,25 +615,988 @@ TYPED_TEST_P(CookieStoreChangeTest, Global_MultipleSubscriptions) { std::vector<CookieChange> cookie_changes_1, cookie_changes_2; std::unique_ptr<CookieChangeSubscription> subscription1 = cs->GetChangeDispatcher().AddCallbackForAllChanges(base::BindRepeating( - &OnCookieChange, base::Unretained(&cookie_changes_1))); + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); std::unique_ptr<CookieChangeSubscription> subscription2 = cs->GetChangeDispatcher().AddCallbackForAllChanges(base::BindRepeating( - &OnCookieChange, base::Unretained(&cookie_changes_2))); - this->RunUntilIdle(); + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); + this->DeliverChangeNotifications(); - EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "abc=def")); - this->RunUntilIdle(); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + this->DeliverChangeNotifications(); ASSERT_EQ(1U, cookie_changes_1.size()); - EXPECT_EQ("abc", cookie_changes_1[0].first.Name()); - EXPECT_EQ("def", cookie_changes_1[0].first.Value()); + EXPECT_EQ("A", cookie_changes_1[0].first.Name()); + EXPECT_EQ("B", cookie_changes_1[0].first.Value()); ASSERT_EQ(1U, cookie_changes_2.size()); - EXPECT_EQ("abc", cookie_changes_2[0].first.Name()); - EXPECT_EQ("def", cookie_changes_2[0].first.Value()); + EXPECT_EQ("A", cookie_changes_2[0].first.Name()); + EXPECT_EQ("B", cookie_changes_2[0].first.Value()); } -TYPED_TEST_P(CookieStoreChangeTest, Named_NoCookie) { +TYPED_TEST_P(CookieStoreChangeUrlTest, NoCookie) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); + EXPECT_EQ(0u, cookie_changes.size()); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, InitialCookie) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + std::vector<CookieChange> cookie_changes; + this->SetCookie(cs, this->http_www_foo_.url(), "A=B"); + this->DeliverChangeNotifications(); + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); + EXPECT_EQ(0u, cookie_changes.size()); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, InsertOne) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); + ASSERT_EQ(0u, cookie_changes.size()); + + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + this->DeliverChangeNotifications(); + ASSERT_EQ(1u, cookie_changes.size()); + + EXPECT_EQ("A", cookie_changes[0].first.Name()); + EXPECT_EQ("B", cookie_changes[0].first.Value()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[0].second)); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, InsertMany) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "C=D")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "E=F")); + this->DeliverChangeNotifications(); + + ASSERT_LE(1u, cookie_changes.size()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[0].second)); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); + EXPECT_EQ("A", cookie_changes[0].first.Name()); + EXPECT_EQ("B", cookie_changes[0].first.Value()); + + ASSERT_LE(2u, cookie_changes.size()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[1].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[1].second)); + EXPECT_EQ("C", cookie_changes[1].first.Name()); + EXPECT_EQ("D", cookie_changes[1].first.Value()); + + ASSERT_LE(3u, cookie_changes.size()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[2].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[2].second)); + EXPECT_EQ("E", cookie_changes[2].first.Name()); + EXPECT_EQ("F", cookie_changes[2].first.Value()); + + EXPECT_EQ(3u, cookie_changes.size()); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, InsertFiltering) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->www_foo_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); + ASSERT_EQ(0u, cookie_changes.size()); + + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B; path=/")); + EXPECT_TRUE(this->SetCookie(cs, this->http_bar_com_.url(), "C=D; path=/")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "E=F; path=/bar")); + EXPECT_TRUE( + this->SetCookie(cs, this->http_www_foo_.url(), "G=H; path=/foo/bar")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "I=J; path=/foo")); + EXPECT_TRUE( + this->SetCookie(cs, this->http_www_foo_.url(), "K=L; domain=foo.com")); + this->DeliverChangeNotifications(); + + ASSERT_LE(1u, cookie_changes.size()); + EXPECT_EQ("A", cookie_changes[0].first.Name()); + EXPECT_EQ("B", cookie_changes[0].first.Value()); + EXPECT_EQ("/", cookie_changes[0].first.Path()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[0].second)); + + ASSERT_LE(2u, cookie_changes.size()); + EXPECT_EQ("I", cookie_changes[1].first.Name()); + EXPECT_EQ("J", cookie_changes[1].first.Value()); + EXPECT_EQ("/foo", cookie_changes[1].first.Path()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[1].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[1].second)); + + ASSERT_LE(3u, cookie_changes.size()); + EXPECT_EQ("K", cookie_changes[2].first.Name()); + EXPECT_EQ("L", cookie_changes[2].first.Value()); + EXPECT_EQ("/", cookie_changes[2].first.Path()); + EXPECT_EQ(".foo.com", cookie_changes[2].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[2].second)); + + EXPECT_EQ(3u, cookie_changes.size()); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, DeleteOne) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + this->DeliverChangeNotifications(); + EXPECT_EQ(1u, cookie_changes.size()); + cookie_changes.clear(); + + EXPECT_TRUE( + this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), "A")); + this->DeliverChangeNotifications(); + + ASSERT_EQ(1u, cookie_changes.size()); + EXPECT_EQ("A", cookie_changes[0].first.Name()); + EXPECT_EQ("B", cookie_changes[0].first.Value()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); + ASSERT_TRUE(this->MatchesCause(CookieChangeCause::EXPLICIT, + cookie_changes[0].second)); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, DeleteTwo) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "C=D")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "E=F")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "G=H")); + this->DeliverChangeNotifications(); + EXPECT_EQ(4u, cookie_changes.size()); + cookie_changes.clear(); + + EXPECT_TRUE( + this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), "C")); + EXPECT_TRUE( + this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), "G")); + this->DeliverChangeNotifications(); + + // Check that the cookie changes are dispatched before calling GetCookies. + // This is not an ASSERT because the following expectations produce useful + // debugging information if they fail. + EXPECT_EQ(2u, cookie_changes.size()); + EXPECT_EQ("A=B; E=F", this->GetCookies(cs, this->http_www_foo_.url())); + + ASSERT_LE(1u, cookie_changes.size()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::EXPLICIT, + cookie_changes[0].second)); + EXPECT_EQ("C", cookie_changes[0].first.Name()); + EXPECT_EQ("D", cookie_changes[0].first.Value()); + + ASSERT_EQ(2u, cookie_changes.size()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[1].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::EXPLICIT, + cookie_changes[1].second)); + EXPECT_EQ("G", cookie_changes[1].first.Name()); + EXPECT_EQ("H", cookie_changes[1].first.Value()); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, DeleteFiltering) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->www_foo_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B; path=/")); + EXPECT_TRUE(this->SetCookie(cs, this->http_bar_com_.url(), "C=D; path=/")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "E=F; path=/bar")); + EXPECT_TRUE( + this->SetCookie(cs, this->http_www_foo_.url(), "G=H; path=/foo/bar")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "I=J; path=/foo")); + EXPECT_TRUE( + this->SetCookie(cs, this->http_www_foo_.url(), "K=L; domain=foo.com")); + this->DeliverChangeNotifications(); + EXPECT_EQ(3u, cookie_changes.size()); + cookie_changes.clear(); + + EXPECT_TRUE( + this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), "A")); + EXPECT_TRUE( + this->FindAndDeleteCookie(cs, this->http_bar_com_.url().host(), "C")); + EXPECT_TRUE( + this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), "E")); + EXPECT_TRUE( + this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), "G")); + EXPECT_TRUE( + this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), "I")); + EXPECT_TRUE(this->FindAndDeleteCookie(cs, ".foo.com", "K")); + this->DeliverChangeNotifications(); + + ASSERT_LE(1u, cookie_changes.size()); + EXPECT_EQ("A", cookie_changes[0].first.Name()); + EXPECT_EQ("B", cookie_changes[0].first.Value()); + EXPECT_EQ("/", cookie_changes[0].first.Path()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::EXPLICIT, + cookie_changes[0].second)); + + ASSERT_LE(2u, cookie_changes.size()); + EXPECT_EQ("I", cookie_changes[1].first.Name()); + EXPECT_EQ("J", cookie_changes[1].first.Value()); + EXPECT_EQ("/foo", cookie_changes[1].first.Path()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[1].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::EXPLICIT, + cookie_changes[1].second)); + + ASSERT_LE(3u, cookie_changes.size()); + EXPECT_EQ("K", cookie_changes[2].first.Name()); + EXPECT_EQ("L", cookie_changes[2].first.Value()); + EXPECT_EQ("/", cookie_changes[2].first.Path()); + EXPECT_EQ(".foo.com", cookie_changes[2].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::EXPLICIT, + cookie_changes[2].second)); + + EXPECT_EQ(3u, cookie_changes.size()); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, Overwrite) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); + ASSERT_EQ(0u, cookie_changes.size()); + + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + this->DeliverChangeNotifications(); + ASSERT_EQ(1u, cookie_changes.size()); + cookie_changes.clear(); + + // Replacing an existing cookie is actually a two-phase delete + set + // operation, so we get an extra notification. + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=C")); + this->DeliverChangeNotifications(); + + ASSERT_LE(1u, cookie_changes.size()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::OVERWRITE, + cookie_changes[0].second)); + EXPECT_EQ("A", cookie_changes[0].first.Name()); + EXPECT_EQ("B", cookie_changes[0].first.Value()); + + ASSERT_LE(2u, cookie_changes.size()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[1].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[1].second)); + EXPECT_EQ("A", cookie_changes[1].first.Name()); + EXPECT_EQ("C", cookie_changes[1].first.Value()); + + EXPECT_EQ(2u, cookie_changes.size()); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, OverwriteFiltering) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->www_foo_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); + ASSERT_EQ(0u, cookie_changes.size()); + + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B; path=/")); + EXPECT_TRUE(this->SetCookie(cs, this->http_bar_com_.url(), "C=D; path=/")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "E=F; path=/bar")); + EXPECT_TRUE( + this->SetCookie(cs, this->http_www_foo_.url(), "G=H; path=/foo/bar")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "I=J; path=/foo")); + EXPECT_TRUE( + this->SetCookie(cs, this->http_www_foo_.url(), "K=L; domain=foo.com")); + this->DeliverChangeNotifications(); + EXPECT_EQ(3u, cookie_changes.size()); + cookie_changes.clear(); + + // Replacing an existing cookie is actually a two-phase delete + set + // operation, so we get two notifications per overwrite. + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=b; path=/")); + EXPECT_TRUE(this->SetCookie(cs, this->http_bar_com_.url(), "C=d; path=/")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "E=f; path=/bar")); + EXPECT_TRUE( + this->SetCookie(cs, this->http_www_foo_.url(), "G=h; path=/foo/bar")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "I=j; path=/foo")); + EXPECT_TRUE( + this->SetCookie(cs, this->http_www_foo_.url(), "K=l; domain=foo.com")); + this->DeliverChangeNotifications(); + + ASSERT_LE(1u, cookie_changes.size()); + EXPECT_EQ("A", cookie_changes[0].first.Name()); + EXPECT_EQ("B", cookie_changes[0].first.Value()); + EXPECT_EQ("/", cookie_changes[0].first.Path()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::OVERWRITE, + cookie_changes[0].second)); + + ASSERT_LE(2u, cookie_changes.size()); + EXPECT_EQ("A", cookie_changes[1].first.Name()); + EXPECT_EQ("b", cookie_changes[1].first.Value()); + EXPECT_EQ("/", cookie_changes[1].first.Path()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[1].first.Domain()); + EXPECT_EQ(CookieChangeCause::INSERTED, cookie_changes[1].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[1].second)); + + ASSERT_LE(3u, cookie_changes.size()); + EXPECT_EQ("I", cookie_changes[2].first.Name()); + EXPECT_EQ("J", cookie_changes[2].first.Value()); + EXPECT_EQ("/foo", cookie_changes[2].first.Path()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[2].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::OVERWRITE, + cookie_changes[2].second)); + + ASSERT_LE(4u, cookie_changes.size()); + EXPECT_EQ("I", cookie_changes[3].first.Name()); + EXPECT_EQ("j", cookie_changes[3].first.Value()); + EXPECT_EQ("/foo", cookie_changes[3].first.Path()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[3].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[3].second)); + + ASSERT_LE(5u, cookie_changes.size()); + EXPECT_EQ("K", cookie_changes[4].first.Name()); + EXPECT_EQ("L", cookie_changes[4].first.Value()); + EXPECT_EQ("/", cookie_changes[4].first.Path()); + EXPECT_EQ(".foo.com", cookie_changes[4].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::OVERWRITE, + cookie_changes[4].second)); + + ASSERT_LE(6u, cookie_changes.size()); + EXPECT_EQ("K", cookie_changes[5].first.Name()); + EXPECT_EQ("l", cookie_changes[5].first.Value()); + EXPECT_EQ("/", cookie_changes[5].first.Path()); + EXPECT_EQ(".foo.com", cookie_changes[5].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[5].second)); + + EXPECT_EQ(6u, cookie_changes.size()); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, OverwriteWithHttpOnly) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + // Insert a cookie "A" for path "/foo". + CookieStore* cs = this->GetCookieStore(); + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->www_foo_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); + ASSERT_EQ(0u, cookie_changes.size()); + + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B; path=/foo")); + this->DeliverChangeNotifications(); + ASSERT_EQ(1u, cookie_changes.size()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[0].second)); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); + EXPECT_EQ("A", cookie_changes[0].first.Name()); + EXPECT_EQ("B", cookie_changes[0].first.Value()); + EXPECT_FALSE(cookie_changes[0].first.IsHttpOnly()); + cookie_changes.clear(); + + // Insert a cookie "A" for path "/foo", that is httponly. This should + // overwrite the non-http-only version. + CookieOptions allow_httponly; + allow_httponly.set_include_httponly(); + EXPECT_TRUE(this->SetCookieWithOptions(cs, this->http_www_foo_.url(), + "A=C; path=/foo; httponly", + allow_httponly)); + this->DeliverChangeNotifications(); + + ASSERT_LE(1u, cookie_changes.size()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::OVERWRITE, + cookie_changes[0].second)); + EXPECT_EQ("A", cookie_changes[0].first.Name()); + EXPECT_EQ("B", cookie_changes[0].first.Value()); + EXPECT_FALSE(cookie_changes[0].first.IsHttpOnly()); + + ASSERT_LE(2u, cookie_changes.size()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[1].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[1].second)); + EXPECT_EQ("A", cookie_changes[1].first.Name()); + EXPECT_EQ("C", cookie_changes[1].first.Value()); + EXPECT_TRUE(cookie_changes[1].first.IsHttpOnly()); + + EXPECT_EQ(2u, cookie_changes.size()); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, Deregister) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); + ASSERT_EQ(0u, cookie_changes.size()); + + // Insert a cookie and make sure it is seen. + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + this->DeliverChangeNotifications(); + ASSERT_EQ(1u, cookie_changes.size()); + EXPECT_EQ("A", cookie_changes[0].first.Name()); + EXPECT_EQ("B", cookie_changes[0].first.Value()); + cookie_changes.clear(); + + // De-register the subscription. + subscription.reset(); + + // Insert a second cookie and make sure it's not visible. + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "C=D")); + this->DeliverChangeNotifications(); + + EXPECT_EQ(0u, cookie_changes.size()); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, DeregisterMultiple) { + if (!TypeParam::supports_url_cookie_tracking || + !TypeParam::supports_multiple_tracking_callbacks) + return; + + CookieStore* cs = this->GetCookieStore(); + + // Register two subscriptions. + std::vector<CookieChange> cookie_changes_1, cookie_changes_2; + std::unique_ptr<CookieChangeSubscription> subscription1 = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); + std::unique_ptr<CookieChangeSubscription> subscription2 = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); + this->DeliverChangeNotifications(); + ASSERT_EQ(0u, cookie_changes_1.size()); + ASSERT_EQ(0u, cookie_changes_2.size()); + + // Insert a cookie and make sure it's seen. + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + this->DeliverChangeNotifications(); + ASSERT_EQ(1u, cookie_changes_1.size()); + EXPECT_EQ("A", cookie_changes_1[0].first.Name()); + EXPECT_EQ("B", cookie_changes_1[0].first.Value()); + cookie_changes_1.clear(); + + ASSERT_EQ(1u, cookie_changes_2.size()); + EXPECT_EQ("A", cookie_changes_2[0].first.Name()); + EXPECT_EQ("B", cookie_changes_2[0].first.Value()); + cookie_changes_2.clear(); + + // De-register the second registration. + subscription2.reset(); + + // Insert a second cookie and make sure that it's only visible in one + // change array. + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "C=D")); + this->DeliverChangeNotifications(); + ASSERT_EQ(1u, cookie_changes_1.size()); + EXPECT_EQ("C", cookie_changes_1[0].first.Name()); + EXPECT_EQ("D", cookie_changes_1[0].first.Value()); + + EXPECT_EQ(0u, cookie_changes_2.size()); +} + +// Confirm that a listener does not receive notifications for changes that +// happened right before the subscription was established. +TYPED_TEST_P(CookieStoreChangeUrlTest, DispatchRace) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + + // This cookie insertion should not be seen. + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + // DeliverChangeNotifications() must NOT be called before the subscription is + // established. + + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "C=D")); + this->DeliverChangeNotifications(); + + EXPECT_LE(1u, cookie_changes.size()); + EXPECT_EQ("C", cookie_changes[0].first.Name()); + EXPECT_EQ("D", cookie_changes[0].first.Value()); + + ASSERT_EQ(1u, cookie_changes.size()); +} + +// Confirm that deregistering a subscription blocks the notification if the +// deregistration happened after the change but before the notification was +// received. +TYPED_TEST_P(CookieStoreChangeUrlTest, DeregisterRace) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); + ASSERT_EQ(0u, cookie_changes.size()); + + // Insert a cookie and make sure it's seen. + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + this->DeliverChangeNotifications(); + ASSERT_EQ(1u, cookie_changes.size()); + EXPECT_EQ("A", cookie_changes[0].first.Name()); + EXPECT_EQ("B", cookie_changes[0].first.Value()); + cookie_changes.clear(); + + // Insert a cookie, confirm it is not seen, deregister the subscription, run + // until idle, and confirm the cookie is still not seen. + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "C=D")); + + // Note that by the API contract it's perfectly valid to have received the + // notification immediately, i.e. synchronously with the cookie change. In + // that case, there's nothing to test. + if (1u == cookie_changes.size()) + return; + + // A task was posted by the SetCookie() above, but has not yet arrived. If it + // arrived before the subscription is destroyed, callback execution would be + // valid. Destroy the subscription so as to lose the race and make sure the + // task posted arrives after the subscription was destroyed. + subscription.reset(); + this->DeliverChangeNotifications(); + ASSERT_EQ(0u, cookie_changes.size()); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, DeregisterRaceMultiple) { + if (!TypeParam::supports_url_cookie_tracking || + !TypeParam::supports_multiple_tracking_callbacks) + return; + + CookieStore* cs = this->GetCookieStore(); + + // Register two subscriptions. + std::vector<CookieChange> cookie_changes_1, cookie_changes_2; + std::unique_ptr<CookieChangeSubscription> subscription1 = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); + std::unique_ptr<CookieChangeSubscription> subscription2 = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); + this->DeliverChangeNotifications(); + ASSERT_EQ(0u, cookie_changes_1.size()); + ASSERT_EQ(0u, cookie_changes_2.size()); + + // Insert a cookie and make sure it's seen. + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + this->DeliverChangeNotifications(); + + ASSERT_EQ(1u, cookie_changes_1.size()); + EXPECT_EQ("A", cookie_changes_1[0].first.Name()); + EXPECT_EQ("B", cookie_changes_1[0].first.Value()); + cookie_changes_1.clear(); + + ASSERT_EQ(1u, cookie_changes_2.size()); + EXPECT_EQ("A", cookie_changes_2[0].first.Name()); + EXPECT_EQ("B", cookie_changes_2[0].first.Value()); + cookie_changes_2.clear(); + + // Insert a cookie, confirm it is not seen, deregister a subscription, run + // until idle, and confirm the cookie is still not seen. + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "C=D")); + + // Note that by the API contract it's perfectly valid to have received the + // notification immediately, i.e. synchronously with the cookie change. In + // that case, there's nothing to test. + if (1u == cookie_changes_2.size()) + return; + + // A task was posted by the SetCookie() above, but has not yet arrived. If it + // arrived before the subscription is destroyed, callback execution would be + // valid. Destroy one of the subscriptions so as to lose the race and make + // sure the task posted arrives after the subscription was destroyed. + subscription2.reset(); + this->DeliverChangeNotifications(); + ASSERT_EQ(1u, cookie_changes_1.size()); + EXPECT_EQ("C", cookie_changes_1[0].first.Name()); + EXPECT_EQ("D", cookie_changes_1[0].first.Value()); + + // No late notification was received. + ASSERT_EQ(0u, cookie_changes_2.size()); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, DifferentSubscriptionsDisjoint) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + + std::vector<CookieChange> cookie_changes_1, cookie_changes_2; + std::unique_ptr<CookieChangeSubscription> subscription1 = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); + std::unique_ptr<CookieChangeSubscription> subscription2 = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_bar_com_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); + this->DeliverChangeNotifications(); + ASSERT_EQ(0u, cookie_changes_1.size()); + ASSERT_EQ(0u, cookie_changes_2.size()); + + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + this->DeliverChangeNotifications(); + EXPECT_EQ(1u, cookie_changes_1.size()); + EXPECT_EQ(0u, cookie_changes_2.size()); + + EXPECT_TRUE(this->SetCookie(cs, this->http_bar_com_.url(), "C=D")); + this->DeliverChangeNotifications(); + + ASSERT_EQ(1u, cookie_changes_1.size()); + EXPECT_EQ("A", cookie_changes_1[0].first.Name()); + EXPECT_EQ("B", cookie_changes_1[0].first.Value()); + EXPECT_EQ(this->http_www_foo_.url().host(), + cookie_changes_1[0].first.Domain()); + + ASSERT_EQ(1u, cookie_changes_2.size()); + EXPECT_EQ("C", cookie_changes_2[0].first.Name()); + EXPECT_EQ("D", cookie_changes_2[0].first.Value()); + EXPECT_EQ(this->http_bar_com_.url().host(), + cookie_changes_2[0].first.Domain()); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, DifferentSubscriptionsDomains) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + + std::vector<CookieChange> cookie_changes_1, cookie_changes_2; + std::unique_ptr<CookieChangeSubscription> subscription1 = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); + std::unique_ptr<CookieChangeSubscription> subscription2 = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_bar_com_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); + this->DeliverChangeNotifications(); + ASSERT_EQ(0u, cookie_changes_1.size()); + ASSERT_EQ(0u, cookie_changes_2.size()); + + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + this->DeliverChangeNotifications(); + EXPECT_EQ(1u, cookie_changes_1.size()); + EXPECT_EQ(0u, cookie_changes_2.size()); + + EXPECT_TRUE(this->SetCookie(cs, this->http_bar_com_.url(), "C=D")); + this->DeliverChangeNotifications(); + + ASSERT_EQ(1u, cookie_changes_1.size()); + EXPECT_EQ("A", cookie_changes_1[0].first.Name()); + EXPECT_EQ("B", cookie_changes_1[0].first.Value()); + EXPECT_EQ(this->http_www_foo_.url().host(), + cookie_changes_1[0].first.Domain()); + + ASSERT_EQ(1u, cookie_changes_2.size()); + EXPECT_EQ("C", cookie_changes_2[0].first.Name()); + EXPECT_EQ("D", cookie_changes_2[0].first.Value()); + EXPECT_EQ(this->http_bar_com_.url().host(), + cookie_changes_2[0].first.Domain()); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, DifferentSubscriptionsPaths) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + + std::vector<CookieChange> cookie_changes_1, cookie_changes_2; + std::unique_ptr<CookieChangeSubscription> subscription1 = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); + std::unique_ptr<CookieChangeSubscription> subscription2 = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->www_foo_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); + this->DeliverChangeNotifications(); + ASSERT_EQ(0u, cookie_changes_1.size()); + ASSERT_EQ(0u, cookie_changes_2.size()); + + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + this->DeliverChangeNotifications(); + EXPECT_EQ(1u, cookie_changes_1.size()); + EXPECT_EQ(1u, cookie_changes_2.size()); + + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "C=D; path=/foo")); + this->DeliverChangeNotifications(); + + ASSERT_EQ(1u, cookie_changes_1.size()); + EXPECT_EQ("A", cookie_changes_1[0].first.Name()); + EXPECT_EQ("B", cookie_changes_1[0].first.Value()); + EXPECT_EQ("/", cookie_changes_1[0].first.Path()); + EXPECT_EQ(this->http_www_foo_.url().host(), + cookie_changes_1[0].first.Domain()); + + ASSERT_LE(1u, cookie_changes_2.size()); + EXPECT_EQ("A", cookie_changes_2[0].first.Name()); + EXPECT_EQ("B", cookie_changes_2[0].first.Value()); + EXPECT_EQ("/", cookie_changes_2[0].first.Path()); + EXPECT_EQ(this->http_www_foo_.url().host(), + cookie_changes_2[0].first.Domain()); + + ASSERT_LE(2u, cookie_changes_2.size()); + EXPECT_EQ("C", cookie_changes_2[1].first.Name()); + EXPECT_EQ("D", cookie_changes_2[1].first.Value()); + EXPECT_EQ("/foo", cookie_changes_2[1].first.Path()); + EXPECT_EQ(this->http_www_foo_.url().host(), + cookie_changes_2[1].first.Domain()); + + EXPECT_EQ(2u, cookie_changes_2.size()); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, DifferentSubscriptionsFiltering) { + if (!TypeParam::supports_url_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + + std::vector<CookieChange> cookie_changes_1, cookie_changes_2; + std::vector<CookieChange> cookie_changes_3; + std::unique_ptr<CookieChangeSubscription> subscription1 = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); + std::unique_ptr<CookieChangeSubscription> subscription2 = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_bar_com_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); + std::unique_ptr<CookieChangeSubscription> subscription3 = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->www_foo_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_3))); + this->DeliverChangeNotifications(); + ASSERT_EQ(0u, cookie_changes_1.size()); + ASSERT_EQ(0u, cookie_changes_2.size()); + EXPECT_EQ(0u, cookie_changes_3.size()); + + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + this->DeliverChangeNotifications(); + EXPECT_EQ(1u, cookie_changes_1.size()); + EXPECT_EQ(0u, cookie_changes_2.size()); + EXPECT_EQ(1u, cookie_changes_3.size()); + + EXPECT_TRUE(this->SetCookie(cs, this->http_bar_com_.url(), "C=D")); + this->DeliverChangeNotifications(); + EXPECT_EQ(1u, cookie_changes_1.size()); + EXPECT_EQ(1u, cookie_changes_2.size()); + EXPECT_EQ(1u, cookie_changes_3.size()); + + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "E=F; path=/foo")); + this->DeliverChangeNotifications(); + + ASSERT_LE(1u, cookie_changes_1.size()); + EXPECT_EQ("A", cookie_changes_1[0].first.Name()); + EXPECT_EQ("B", cookie_changes_1[0].first.Value()); + EXPECT_EQ(this->http_www_foo_.url().host(), + cookie_changes_1[0].first.Domain()); + EXPECT_EQ(1u, cookie_changes_1.size()); + + ASSERT_LE(1u, cookie_changes_2.size()); + EXPECT_EQ("C", cookie_changes_2[0].first.Name()); + EXPECT_EQ("D", cookie_changes_2[0].first.Value()); + EXPECT_EQ(this->http_bar_com_.url().host(), + cookie_changes_2[0].first.Domain()); + EXPECT_EQ(1u, cookie_changes_2.size()); + + ASSERT_LE(1u, cookie_changes_3.size()); + EXPECT_EQ("A", cookie_changes_3[0].first.Name()); + EXPECT_EQ("B", cookie_changes_3[0].first.Value()); + EXPECT_EQ("/", cookie_changes_3[0].first.Path()); + EXPECT_EQ(this->http_www_foo_.url().host(), + cookie_changes_3[0].first.Domain()); + + ASSERT_LE(2u, cookie_changes_3.size()); + EXPECT_EQ("E", cookie_changes_3[1].first.Name()); + EXPECT_EQ("F", cookie_changes_3[1].first.Value()); + EXPECT_EQ("/foo", cookie_changes_3[1].first.Path()); + EXPECT_EQ(this->http_www_foo_.url().host(), + cookie_changes_3[1].first.Domain()); + + EXPECT_EQ(2u, cookie_changes_3.size()); +} + +TYPED_TEST_P(CookieStoreChangeUrlTest, MultipleSubscriptions) { + if (!TypeParam::supports_url_cookie_tracking || + !TypeParam::supports_multiple_tracking_callbacks) + return; + + CookieStore* cs = this->GetCookieStore(); + + std::vector<CookieChange> cookie_changes_1, cookie_changes_2; + std::unique_ptr<CookieChangeSubscription> subscription1 = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); + std::unique_ptr<CookieChangeSubscription> subscription2 = + cs->GetChangeDispatcher().AddCallbackForUrl( + this->http_www_foo_.url(), + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); + this->DeliverChangeNotifications(); + + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "A=B")); + this->DeliverChangeNotifications(); + + ASSERT_EQ(1U, cookie_changes_1.size()); + EXPECT_EQ("A", cookie_changes_1[0].first.Name()); + EXPECT_EQ("B", cookie_changes_1[0].first.Value()); + + ASSERT_EQ(1U, cookie_changes_2.size()); + EXPECT_EQ("A", cookie_changes_2[0].first.Name()); + EXPECT_EQ("B", cookie_changes_2[0].first.Value()); +} + +TYPED_TEST_P(CookieStoreChangeNamedTest, NoCookie) { if (!TypeParam::supports_named_cookie_tracking) return; @@ -428,30 +1605,32 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_NoCookie) { std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForCookie( this->http_www_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes))); - this->RunUntilIdle(); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); EXPECT_EQ(0u, cookie_changes.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Named_InitialCookie) { +TYPED_TEST_P(CookieStoreChangeNamedTest, InitialCookie) { if (!TypeParam::supports_named_cookie_tracking) return; CookieStore* cs = this->GetCookieStore(); std::vector<CookieChange> cookie_changes; this->SetCookie(cs, this->http_www_foo_.url(), "abc=def"); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForCookie( this->http_www_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes))); - this->RunUntilIdle(); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); EXPECT_EQ(0u, cookie_changes.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Named_Insert) { +TYPED_TEST_P(CookieStoreChangeNamedTest, InsertOne) { if (!TypeParam::supports_named_cookie_tracking) return; @@ -460,22 +1639,63 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_Insert) { std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForCookie( this->http_www_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes))); - this->RunUntilIdle(); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes.size()); EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "abc=def")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes.size()); EXPECT_EQ("abc", cookie_changes[0].first.Name()); EXPECT_EQ("def", cookie_changes[0].first.Value()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); - EXPECT_EQ(CookieChangeCause::INSERTED, cookie_changes[0].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[0].second)); +} + +TYPED_TEST_P(CookieStoreChangeNamedTest, InsertTwo) { + if (!TypeParam::supports_named_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForCookie( + this->www_foo_foo_.url(), "abc", + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); + ASSERT_EQ(0u, cookie_changes.size()); + + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "abc=def")); + EXPECT_TRUE( + this->SetCookie(cs, this->http_www_foo_.url(), "abc=hij; path=/foo")); + this->DeliverChangeNotifications(); + + ASSERT_LE(1u, cookie_changes.size()); + EXPECT_EQ("abc", cookie_changes[0].first.Name()); + EXPECT_EQ("def", cookie_changes[0].first.Value()); + EXPECT_EQ("/", cookie_changes[0].first.Path()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[0].second)); + + ASSERT_LE(2u, cookie_changes.size()); + EXPECT_EQ("abc", cookie_changes[1].first.Name()); + EXPECT_EQ("hij", cookie_changes[1].first.Value()); + EXPECT_EQ("/foo", cookie_changes[1].first.Path()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[1].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[1].second)); + + EXPECT_EQ(2u, cookie_changes.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Named_InsertFiltering) { +TYPED_TEST_P(CookieStoreChangeNamedTest, InsertFiltering) { if (!TypeParam::supports_named_cookie_tracking) return; @@ -484,9 +1704,10 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_InsertFiltering) { std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForCookie( this->www_foo_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes))); - this->RunUntilIdle(); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes.size()); EXPECT_TRUE( @@ -495,26 +1716,43 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_InsertFiltering) { this->SetCookie(cs, this->http_bar_com_.url(), "abc=ghi; path=/")); EXPECT_TRUE( this->SetCookie(cs, this->http_www_foo_.url(), "abc=jkl; path=/bar")); + EXPECT_TRUE( + this->SetCookie(cs, this->http_www_foo_.url(), "abc=mno; path=/foo/bar")); EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "xyz=zyx")); EXPECT_TRUE( - this->SetCookie(cs, this->http_www_foo_.url(), "abc=mno; path=/foo")); - this->RunUntilIdle(); - ASSERT_EQ(2u, cookie_changes.size()); + this->SetCookie(cs, this->http_www_foo_.url(), "abc=pqr; path=/foo")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), + "abc=stu; domain=foo.com")); + this->DeliverChangeNotifications(); + ASSERT_LE(1u, cookie_changes.size()); EXPECT_EQ("abc", cookie_changes[0].first.Name()); EXPECT_EQ("def", cookie_changes[0].first.Value()); EXPECT_EQ("/", cookie_changes[0].first.Path()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); - EXPECT_EQ(CookieChangeCause::INSERTED, cookie_changes[0].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[0].second)); + ASSERT_LE(2u, cookie_changes.size()); EXPECT_EQ("abc", cookie_changes[1].first.Name()); - EXPECT_EQ("mno", cookie_changes[1].first.Value()); + EXPECT_EQ("pqr", cookie_changes[1].first.Value()); EXPECT_EQ("/foo", cookie_changes[1].first.Path()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[1].first.Domain()); - EXPECT_EQ(CookieChangeCause::INSERTED, cookie_changes[1].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[1].second)); + + ASSERT_LE(3u, cookie_changes.size()); + EXPECT_EQ("abc", cookie_changes[2].first.Name()); + EXPECT_EQ("stu", cookie_changes[2].first.Value()); + EXPECT_EQ("/", cookie_changes[2].first.Path()); + EXPECT_EQ(".foo.com", cookie_changes[2].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[2].second)); + + EXPECT_EQ(3u, cookie_changes.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Named_Delete) { +TYPED_TEST_P(CookieStoreChangeNamedTest, DeleteOne) { if (!TypeParam::supports_named_cookie_tracking) return; @@ -523,25 +1761,69 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_Delete) { std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForCookie( this->http_www_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes))); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "abc=def")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); EXPECT_EQ(1u, cookie_changes.size()); cookie_changes.clear(); EXPECT_TRUE( this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), "abc")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); + ASSERT_EQ(1u, cookie_changes.size()); + EXPECT_EQ("abc", cookie_changes[0].first.Name()); + EXPECT_EQ("def", cookie_changes[0].first.Value()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::EXPLICIT, + cookie_changes[0].second)); +} + +TYPED_TEST_P(CookieStoreChangeNamedTest, DeleteTwo) { + if (!TypeParam::supports_named_cookie_tracking) + return; + CookieStore* cs = this->GetCookieStore(); + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForCookie( + this->www_foo_foo_.url(), "abc", + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "abc=def")); + EXPECT_TRUE( + this->SetCookie(cs, this->http_www_foo_.url(), "abc=hij; path=/foo")); + this->DeliverChangeNotifications(); + EXPECT_EQ(2u, cookie_changes.size()); + cookie_changes.clear(); + + EXPECT_TRUE(this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), + "abc", "/")); + EXPECT_TRUE(this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), + "abc", "/foo")); + this->DeliverChangeNotifications(); + + ASSERT_LE(1u, cookie_changes.size()); EXPECT_EQ("abc", cookie_changes[0].first.Name()); EXPECT_EQ("def", cookie_changes[0].first.Value()); + EXPECT_EQ("/", cookie_changes[0].first.Path()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); - EXPECT_EQ(CookieChangeCause::EXPLICIT, cookie_changes[0].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::EXPLICIT, + cookie_changes[0].second)); + + ASSERT_EQ(2u, cookie_changes.size()); + EXPECT_EQ("abc", cookie_changes[1].first.Name()); + EXPECT_EQ("hij", cookie_changes[1].first.Value()); + EXPECT_EQ("/foo", cookie_changes[1].first.Path()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[1].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::EXPLICIT, + cookie_changes[1].second)); } -TYPED_TEST_P(CookieStoreChangeTest, Named_DeleteFiltering) { +TYPED_TEST_P(CookieStoreChangeNamedTest, DeleteFiltering) { if (!TypeParam::supports_named_cookie_tracking) return; @@ -549,32 +1831,67 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DeleteFiltering) { std::vector<CookieChange> cookie_changes; std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForCookie( - this->http_www_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes))); - EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "xyz=zyx")); - EXPECT_TRUE(this->SetCookie(cs, this->http_bar_com_.url(), "abc=def")); - EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "abc=hij")); - this->RunUntilIdle(); - EXPECT_EQ(1u, cookie_changes.size()); + this->www_foo_foo_.url(), "abc", + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + EXPECT_TRUE( + this->SetCookie(cs, this->http_www_foo_.url(), "xyz=zyx; path=/")); + EXPECT_TRUE( + this->SetCookie(cs, this->http_bar_com_.url(), "abc=def; path=/")); + EXPECT_TRUE( + this->SetCookie(cs, this->http_www_foo_.url(), "abc=hij; path=/foo/bar")); + EXPECT_TRUE( + this->SetCookie(cs, this->http_www_foo_.url(), "abc=mno; path=/foo")); + EXPECT_TRUE( + this->SetCookie(cs, this->http_www_foo_.url(), "abc=pqr; path=/")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), + "abc=stu; domain=foo.com")); + this->DeliverChangeNotifications(); + EXPECT_EQ(3u, cookie_changes.size()); cookie_changes.clear(); EXPECT_TRUE( this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), "xyz")); EXPECT_TRUE( this->FindAndDeleteCookie(cs, this->http_bar_com_.url().host(), "abc")); - EXPECT_TRUE( - this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), "abc")); - this->RunUntilIdle(); - ASSERT_EQ(1u, cookie_changes.size()); - + EXPECT_TRUE(this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), + "abc", "/foo/bar")); + EXPECT_TRUE(this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), + "abc", "/foo")); + EXPECT_TRUE(this->FindAndDeleteCookie(cs, this->http_www_foo_.url().host(), + "abc", "/")); + EXPECT_TRUE(this->FindAndDeleteCookie(cs, ".foo.com", "abc", "/")); + this->DeliverChangeNotifications(); + + ASSERT_LE(1u, cookie_changes.size()); EXPECT_EQ("abc", cookie_changes[0].first.Name()); - EXPECT_EQ("hij", cookie_changes[0].first.Value()); + EXPECT_EQ("mno", cookie_changes[0].first.Value()); + EXPECT_EQ("/foo", cookie_changes[0].first.Path()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); - EXPECT_EQ(CookieChangeCause::EXPLICIT, cookie_changes[0].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::EXPLICIT, + cookie_changes[0].second)); + + ASSERT_LE(2u, cookie_changes.size()); + EXPECT_EQ("abc", cookie_changes[1].first.Name()); + EXPECT_EQ("pqr", cookie_changes[1].first.Value()); + EXPECT_EQ("/", cookie_changes[1].first.Path()); + EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[1].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::EXPLICIT, + cookie_changes[1].second)); + + ASSERT_LE(3u, cookie_changes.size()); + EXPECT_EQ("abc", cookie_changes[2].first.Name()); + EXPECT_EQ("stu", cookie_changes[2].first.Value()); + EXPECT_EQ("/", cookie_changes[2].first.Path()); + EXPECT_EQ(".foo.com", cookie_changes[2].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::EXPLICIT, + cookie_changes[2].second)); + + EXPECT_EQ(3u, cookie_changes.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Named_Overwrite) { +TYPED_TEST_P(CookieStoreChangeNamedTest, Overwrite) { if (!TypeParam::supports_named_cookie_tracking) return; @@ -583,34 +1900,40 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_Overwrite) { std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForCookie( this->http_www_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes))); - this->RunUntilIdle(); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes.size()); EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "abc=def")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); EXPECT_EQ(1u, cookie_changes.size()); cookie_changes.clear(); // Replacing an existing cookie is actually a two-phase delete + set // operation, so we get an extra notification. EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "abc=ghi")); - this->RunUntilIdle(); - ASSERT_EQ(2u, cookie_changes.size()); + this->DeliverChangeNotifications(); + EXPECT_LE(1u, cookie_changes.size()); EXPECT_EQ("abc", cookie_changes[0].first.Name()); EXPECT_EQ("def", cookie_changes[0].first.Value()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); - EXPECT_EQ(CookieChangeCause::OVERWRITE, cookie_changes[0].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::OVERWRITE, + cookie_changes[0].second)); + EXPECT_LE(2u, cookie_changes.size()); EXPECT_EQ("abc", cookie_changes[1].first.Name()); EXPECT_EQ("ghi", cookie_changes[1].first.Value()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[1].first.Domain()); - EXPECT_EQ(CookieChangeCause::INSERTED, cookie_changes[1].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[1].second)); + + EXPECT_EQ(2u, cookie_changes.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Named_OverwriteFiltering) { +TYPED_TEST_P(CookieStoreChangeNamedTest, OverwriteFiltering) { if (!TypeParam::supports_named_cookie_tracking) return; @@ -619,108 +1942,152 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_OverwriteFiltering) { std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForCookie( this->www_foo_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes))); - this->RunUntilIdle(); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes.size()); EXPECT_TRUE( - this->SetCookie(cs, this->http_www_foo_.url(), "abc=def; path=/")); + this->SetCookie(cs, this->http_www_foo_.url(), "xyz=zyx1; path=/")); EXPECT_TRUE( - this->SetCookie(cs, this->http_bar_com_.url(), "abc=ghi; path=/")); + this->SetCookie(cs, this->http_bar_com_.url(), "abc=def1; path=/")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), + "abc=hij1; path=/foo/bar")); EXPECT_TRUE( - this->SetCookie(cs, this->http_www_foo_.url(), "abc=jkl; path=/bar")); - EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "xyz=zyx")); + this->SetCookie(cs, this->http_www_foo_.url(), "abc=mno1; path=/foo")); EXPECT_TRUE( - this->SetCookie(cs, this->http_www_foo_.url(), "abc=mno; path=/foo")); - this->RunUntilIdle(); - EXPECT_EQ(2u, cookie_changes.size()); + this->SetCookie(cs, this->http_www_foo_.url(), "abc=pqr1; path=/")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), + "abc=stu1; domain=foo.com")); + this->DeliverChangeNotifications(); + EXPECT_EQ(3u, cookie_changes.size()); cookie_changes.clear(); // Replacing an existing cookie is actually a two-phase delete + set // operation, so we get two notifications per overwrite. EXPECT_TRUE( - this->SetCookie(cs, this->http_www_foo_.url(), "abc=pqr; path=/")); + this->SetCookie(cs, this->http_www_foo_.url(), "xyz=zyx2; path=/")); EXPECT_TRUE( - this->SetCookie(cs, this->http_bar_com_.url(), "abc=stu; path=/")); + this->SetCookie(cs, this->http_bar_com_.url(), "abc=def2; path=/")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), + "abc=hij2; path=/foo/bar")); EXPECT_TRUE( - this->SetCookie(cs, this->http_www_foo_.url(), "abc=vwx; path=/bar")); - EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "xyz=zyx")); + this->SetCookie(cs, this->http_www_foo_.url(), "abc=mno2; path=/foo")); EXPECT_TRUE( - this->SetCookie(cs, this->http_www_foo_.url(), "abc=yz0; path=/foo")); - this->RunUntilIdle(); - ASSERT_EQ(4u, cookie_changes.size()); + this->SetCookie(cs, this->http_www_foo_.url(), "abc=pqr2; path=/")); + EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), + "abc=stu2; domain=foo.com")); + this->DeliverChangeNotifications(); + ASSERT_LE(1u, cookie_changes.size()); EXPECT_EQ("abc", cookie_changes[0].first.Name()); - EXPECT_EQ("def", cookie_changes[0].first.Value()); - EXPECT_EQ("/", cookie_changes[0].first.Path()); + EXPECT_EQ("mno1", cookie_changes[0].first.Value()); + EXPECT_EQ("/foo", cookie_changes[0].first.Path()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); - EXPECT_EQ(CookieChangeCause::OVERWRITE, cookie_changes[0].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::OVERWRITE, + cookie_changes[0].second)); + ASSERT_LE(2u, cookie_changes.size()); EXPECT_EQ("abc", cookie_changes[1].first.Name()); - EXPECT_EQ("pqr", cookie_changes[1].first.Value()); - EXPECT_EQ("/", cookie_changes[1].first.Path()); + EXPECT_EQ("mno2", cookie_changes[1].first.Value()); + EXPECT_EQ("/foo", cookie_changes[1].first.Path()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[1].first.Domain()); - EXPECT_EQ(CookieChangeCause::INSERTED, cookie_changes[1].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[1].second)); + ASSERT_LE(3u, cookie_changes.size()); EXPECT_EQ("abc", cookie_changes[2].first.Name()); - EXPECT_EQ("mno", cookie_changes[2].first.Value()); - EXPECT_EQ("/foo", cookie_changes[2].first.Path()); + EXPECT_EQ("pqr1", cookie_changes[2].first.Value()); + EXPECT_EQ("/", cookie_changes[2].first.Path()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[2].first.Domain()); - EXPECT_EQ(CookieChangeCause::OVERWRITE, cookie_changes[2].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::OVERWRITE, + cookie_changes[2].second)); + ASSERT_LE(4u, cookie_changes.size()); EXPECT_EQ("abc", cookie_changes[3].first.Name()); - EXPECT_EQ("yz0", cookie_changes[3].first.Value()); - EXPECT_EQ("/foo", cookie_changes[3].first.Path()); + EXPECT_EQ("pqr2", cookie_changes[3].first.Value()); + EXPECT_EQ("/", cookie_changes[3].first.Path()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[3].first.Domain()); - EXPECT_EQ(CookieChangeCause::INSERTED, cookie_changes[3].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[3].second)); + + ASSERT_LE(5u, cookie_changes.size()); + EXPECT_EQ("abc", cookie_changes[4].first.Name()); + EXPECT_EQ("stu1", cookie_changes[4].first.Value()); + EXPECT_EQ("/", cookie_changes[4].first.Path()); + EXPECT_EQ(".foo.com", cookie_changes[4].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::OVERWRITE, + cookie_changes[4].second)); + + ASSERT_LE(6u, cookie_changes.size()); + EXPECT_EQ("abc", cookie_changes[5].first.Name()); + EXPECT_EQ("stu2", cookie_changes[5].first.Value()); + EXPECT_EQ("/", cookie_changes[5].first.Path()); + EXPECT_EQ(".foo.com", cookie_changes[5].first.Domain()); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[5].second)); + + EXPECT_EQ(6u, cookie_changes.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Named_OverwriteWithHttpOnly) { +TYPED_TEST_P(CookieStoreChangeNamedTest, OverwriteWithHttpOnly) { if (!TypeParam::supports_named_cookie_tracking) return; - // Insert a cookie "abc" for path "/foo" + // Insert a cookie "abc" for path "/foo". CookieStore* cs = this->GetCookieStore(); std::vector<CookieChange> cookie_changes; std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForCookie( this->www_foo_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes))); - this->RunUntilIdle(); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes.size()); EXPECT_TRUE( this->SetCookie(cs, this->http_www_foo_.url(), "abc=def; path=/foo")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes.size()); - EXPECT_EQ(CookieChangeCause::INSERTED, cookie_changes[0].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[0].second)); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); EXPECT_EQ("abc", cookie_changes[0].first.Name()); EXPECT_EQ("def", cookie_changes[0].first.Value()); + EXPECT_FALSE(cookie_changes[0].first.IsHttpOnly()); cookie_changes.clear(); - // Insert a cookie "a" for path "/path1", that is httponly. This should + // Insert a cookie "a" for path "/foo", that is httponly. This should // overwrite the non-http-only version. CookieOptions allow_httponly; allow_httponly.set_include_httponly(); EXPECT_TRUE(this->SetCookieWithOptions(cs, this->http_www_foo_.url(), "abc=hij; path=/foo; httponly", allow_httponly)); - this->RunUntilIdle(); - ASSERT_EQ(2u, cookie_changes.size()); + this->DeliverChangeNotifications(); + + ASSERT_LE(1u, cookie_changes.size()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[0].first.Domain()); - EXPECT_EQ(CookieChangeCause::OVERWRITE, cookie_changes[0].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::OVERWRITE, + cookie_changes[0].second)); EXPECT_EQ("abc", cookie_changes[0].first.Name()); EXPECT_EQ("def", cookie_changes[0].first.Value()); + EXPECT_FALSE(cookie_changes[0].first.IsHttpOnly()); + + ASSERT_LE(2u, cookie_changes.size()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes[1].first.Domain()); - EXPECT_EQ(CookieChangeCause::INSERTED, cookie_changes[1].second); + EXPECT_TRUE(this->MatchesCause(CookieChangeCause::INSERTED, + cookie_changes[1].second)); EXPECT_EQ("abc", cookie_changes[1].first.Name()); EXPECT_EQ("hij", cookie_changes[1].first.Value()); + EXPECT_TRUE(cookie_changes[1].first.IsHttpOnly()); + + EXPECT_EQ(2u, cookie_changes.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Named_Deregister) { +TYPED_TEST_P(CookieStoreChangeNamedTest, Deregister) { if (!TypeParam::supports_named_cookie_tracking) return; @@ -730,15 +2097,16 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_Deregister) { std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForCookie( this->www_foo_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes))); - this->RunUntilIdle(); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes.size()); // Insert a cookie and make sure it is seen. EXPECT_TRUE( this->SetCookie(cs, this->http_www_foo_.url(), "abc=def; path=/foo")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes.size()); EXPECT_EQ("abc", cookie_changes[0].first.Name()); EXPECT_EQ("def", cookie_changes[0].first.Value()); @@ -751,12 +2119,12 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_Deregister) { // Insert a second cookie and make sure it's not visible. EXPECT_TRUE( this->SetCookie(cs, this->http_www_foo_.url(), "abc=hij; path=/")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); EXPECT_EQ(0u, cookie_changes.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Named_DeregisterMultiple) { +TYPED_TEST_P(CookieStoreChangeNamedTest, DeregisterMultiple) { if (!TypeParam::supports_named_cookie_tracking || !TypeParam::supports_multiple_tracking_callbacks) return; @@ -768,21 +2136,23 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DeregisterMultiple) { std::unique_ptr<CookieChangeSubscription> subscription1 = cs->GetChangeDispatcher().AddCallbackForCookie( this->www_foo_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_1))); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); std::unique_ptr<CookieChangeSubscription> subscription2 = cs->GetChangeDispatcher().AddCallbackForCookie( this->www_foo_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_2))); - this->RunUntilIdle(); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes_1.size()); ASSERT_EQ(0u, cookie_changes_2.size()); // Insert a cookie and make sure it's seen. EXPECT_TRUE( this->SetCookie(cs, this->http_www_foo_.url(), "abc=def; path=/foo")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ("abc", cookie_changes_1[0].first.Name()); EXPECT_EQ("def", cookie_changes_1[0].first.Value()); @@ -802,7 +2172,7 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DeregisterMultiple) { // change array. EXPECT_TRUE( this->SetCookie(cs, this->http_www_foo_.url(), "abc=hij; path=/")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ("abc", cookie_changes_1[0].first.Name()); EXPECT_EQ("hij", cookie_changes_1[0].first.Value()); @@ -811,10 +2181,44 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DeregisterMultiple) { EXPECT_EQ(0u, cookie_changes_2.size()); } -// Confirm that deregistering a subscription blocks the notification -// if the deregistration happened after the change but before the -// notification was received. -TYPED_TEST_P(CookieStoreChangeTest, Named_DeregisterRace) { +// Confirm that a listener does not receive notifications for changes that +// happened right before the subscription was established. +TYPED_TEST_P(CookieStoreChangeNamedTest, DispatchRace) { + if (!TypeParam::supports_named_cookie_tracking) + return; + + CookieStore* cs = this->GetCookieStore(); + + // This cookie insertion should not be seen. + EXPECT_TRUE( + this->SetCookie(cs, this->http_www_foo_.url(), "abc=def; path=/foo")); + // DeliverChangeNotifications() must NOT be called before the subscription is + // established. + + std::vector<CookieChange> cookie_changes; + std::unique_ptr<CookieChangeSubscription> subscription = + cs->GetChangeDispatcher().AddCallbackForCookie( + this->www_foo_foo_.url(), "abc", + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + + EXPECT_TRUE( + this->SetCookie(cs, this->http_www_foo_.url(), "abc=hij; path=/")); + this->DeliverChangeNotifications(); + + EXPECT_LE(1u, cookie_changes.size()); + EXPECT_EQ("abc", cookie_changes[0].first.Name()); + EXPECT_EQ("hij", cookie_changes[0].first.Value()); + EXPECT_EQ("/", cookie_changes[0].first.Path()); + + ASSERT_EQ(1u, cookie_changes.size()); +} + +// Confirm that deregistering a subscription blocks the notification if the +// deregistration happened after the change but before the notification was +// received. +TYPED_TEST_P(CookieStoreChangeNamedTest, DeregisterRace) { if (!TypeParam::supports_named_cookie_tracking) return; @@ -824,15 +2228,16 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DeregisterRace) { std::unique_ptr<CookieChangeSubscription> subscription = cs->GetChangeDispatcher().AddCallbackForCookie( this->www_foo_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes))); - this->RunUntilIdle(); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes.size()); // Insert a cookie and make sure it's seen. EXPECT_TRUE( this->SetCookie(cs, this->http_www_foo_.url(), "abc=def; path=/foo")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes.size()); EXPECT_EQ("abc", cookie_changes[0].first.Name()); EXPECT_EQ("def", cookie_changes[0].first.Value()); @@ -855,11 +2260,11 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DeregisterRace) { // valid. Destroy the subscription so as to lose the race and make sure the // task posted arrives after the subscription was destroyed. subscription.reset(); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Named_DeregisterRaceMultiple) { +TYPED_TEST_P(CookieStoreChangeNamedTest, DeregisterRaceMultiple) { if (!TypeParam::supports_named_cookie_tracking || !TypeParam::supports_multiple_tracking_callbacks) return; @@ -870,21 +2275,23 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DeregisterRaceMultiple) { std::unique_ptr<CookieChangeSubscription> subscription1 = cs->GetChangeDispatcher().AddCallbackForCookie( this->www_foo_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_1))); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); std::unique_ptr<CookieChangeSubscription> subscription2 = cs->GetChangeDispatcher().AddCallbackForCookie( this->www_foo_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_2))); - this->RunUntilIdle(); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes_1.size()); ASSERT_EQ(0u, cookie_changes_2.size()); // Insert a cookie and make sure it's seen. EXPECT_TRUE( this->SetCookie(cs, this->http_www_foo_.url(), "abc=def; path=/foo")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ("abc", cookie_changes_1[0].first.Name()); @@ -906,17 +2313,15 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DeregisterRaceMultiple) { // Note that by the API contract it's perfectly valid to have received the // notification immediately, i.e. synchronously with the cookie change. In // that case, there's nothing to test. - if (1u == cookie_changes_2.size()) { - LOG(ERROR) << "Nothing to test."; + if (1u == cookie_changes_2.size()) return; - } // A task was posted by the SetCookie() above, but has not yet arrived. If it // arrived before the subscription is destroyed, callback execution would be // valid. Destroy one of the subscriptions so as to lose the race and make // sure the task posted arrives after the subscription was destroyed. subscription2.reset(); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ("abc", cookie_changes_1[0].first.Name()); EXPECT_EQ("hij", cookie_changes_1[0].first.Value()); @@ -926,7 +2331,7 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DeregisterRaceMultiple) { ASSERT_EQ(0u, cookie_changes_2.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsDisjoint) { +TYPED_TEST_P(CookieStoreChangeNamedTest, DifferentSubscriptionsDisjoint) { if (!TypeParam::supports_named_cookie_tracking) return; @@ -936,24 +2341,26 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsDisjoint) { std::unique_ptr<CookieChangeSubscription> subscription1 = cs->GetChangeDispatcher().AddCallbackForCookie( this->http_www_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_1))); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); std::unique_ptr<CookieChangeSubscription> subscription2 = cs->GetChangeDispatcher().AddCallbackForCookie( this->http_bar_com_.url(), "ghi", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_2))); - this->RunUntilIdle(); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes_1.size()); ASSERT_EQ(0u, cookie_changes_2.size()); EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "abc=def")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); EXPECT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ(0u, cookie_changes_2.size()); EXPECT_TRUE(this->SetCookie(cs, this->http_bar_com_.url(), "ghi=jkl")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ("abc", cookie_changes_1[0].first.Name()); @@ -968,7 +2375,7 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsDisjoint) { cookie_changes_2[0].first.Domain()); } -TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsDomains) { +TYPED_TEST_P(CookieStoreChangeNamedTest, DifferentSubscriptionsDomains) { if (!TypeParam::supports_named_cookie_tracking) return; @@ -978,24 +2385,26 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsDomains) { std::unique_ptr<CookieChangeSubscription> subscription1 = cs->GetChangeDispatcher().AddCallbackForCookie( this->http_www_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_1))); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); std::unique_ptr<CookieChangeSubscription> subscription2 = cs->GetChangeDispatcher().AddCallbackForCookie( this->http_bar_com_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_2))); - this->RunUntilIdle(); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes_1.size()); ASSERT_EQ(0u, cookie_changes_2.size()); EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "abc=def")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); EXPECT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ(0u, cookie_changes_2.size()); EXPECT_TRUE(this->SetCookie(cs, this->http_bar_com_.url(), "abc=ghi")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ("abc", cookie_changes_1[0].first.Name()); @@ -1010,7 +2419,7 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsDomains) { cookie_changes_2[0].first.Domain()); } -TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsNames) { +TYPED_TEST_P(CookieStoreChangeNamedTest, DifferentSubscriptionsNames) { if (!TypeParam::supports_named_cookie_tracking) return; @@ -1020,24 +2429,26 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsNames) { std::unique_ptr<CookieChangeSubscription> subscription1 = cs->GetChangeDispatcher().AddCallbackForCookie( this->http_www_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_1))); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); std::unique_ptr<CookieChangeSubscription> subscription2 = cs->GetChangeDispatcher().AddCallbackForCookie( this->http_www_foo_.url(), "ghi", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_2))); - this->RunUntilIdle(); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes_1.size()); ASSERT_EQ(0u, cookie_changes_2.size()); EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "abc=def")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); EXPECT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ(0u, cookie_changes_2.size()); EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "ghi=jkl")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ("abc", cookie_changes_1[0].first.Name()); @@ -1052,7 +2463,7 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsNames) { cookie_changes_2[0].first.Domain()); } -TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsPaths) { +TYPED_TEST_P(CookieStoreChangeNamedTest, DifferentSubscriptionsPaths) { if (!TypeParam::supports_named_cookie_tracking) return; @@ -1062,25 +2473,27 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsPaths) { std::unique_ptr<CookieChangeSubscription> subscription1 = cs->GetChangeDispatcher().AddCallbackForCookie( this->http_www_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_1))); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); std::unique_ptr<CookieChangeSubscription> subscription2 = cs->GetChangeDispatcher().AddCallbackForCookie( this->www_foo_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_2))); - this->RunUntilIdle(); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes_1.size()); ASSERT_EQ(0u, cookie_changes_2.size()); EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "abc=def")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); EXPECT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ(1u, cookie_changes_2.size()); EXPECT_TRUE( this->SetCookie(cs, this->http_www_foo_.url(), "abc=ghi; path=/foo")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ("abc", cookie_changes_1[0].first.Name()); @@ -1096,15 +2509,17 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsPaths) { EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes_2[0].first.Domain()); - ASSERT_EQ(2u, cookie_changes_2.size()); + ASSERT_LE(2u, cookie_changes_2.size()); EXPECT_EQ("abc", cookie_changes_2[1].first.Name()); EXPECT_EQ("ghi", cookie_changes_2[1].first.Value()); EXPECT_EQ("/foo", cookie_changes_2[1].first.Path()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes_2[1].first.Domain()); + + EXPECT_EQ(2u, cookie_changes_2.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsFiltering) { +TYPED_TEST_P(CookieStoreChangeNamedTest, DifferentSubscriptionsFiltering) { if (!TypeParam::supports_named_cookie_tracking) return; @@ -1115,31 +2530,35 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsFiltering) { std::unique_ptr<CookieChangeSubscription> subscription1 = cs->GetChangeDispatcher().AddCallbackForCookie( this->http_www_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_1))); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); std::unique_ptr<CookieChangeSubscription> subscription2 = cs->GetChangeDispatcher().AddCallbackForCookie( this->http_www_foo_.url(), "hij", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_2))); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); std::unique_ptr<CookieChangeSubscription> subscription3 = cs->GetChangeDispatcher().AddCallbackForCookie( this->http_bar_com_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_3))); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_3))); std::unique_ptr<CookieChangeSubscription> subscription4 = cs->GetChangeDispatcher().AddCallbackForCookie( this->www_foo_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_4))); - this->RunUntilIdle(); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_4))); + this->DeliverChangeNotifications(); ASSERT_EQ(0u, cookie_changes_1.size()); ASSERT_EQ(0u, cookie_changes_2.size()); EXPECT_EQ(0u, cookie_changes_3.size()); EXPECT_EQ(0u, cookie_changes_4.size()); EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "abc=def")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); EXPECT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ(0u, cookie_changes_2.size()); EXPECT_EQ(0u, cookie_changes_3.size()); @@ -1147,7 +2566,7 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsFiltering) { EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "xyz=zyx")); EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "hij=mno")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); EXPECT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ(1u, cookie_changes_2.size()); EXPECT_EQ(0u, cookie_changes_3.size()); @@ -1156,7 +2575,7 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsFiltering) { EXPECT_TRUE(this->SetCookie(cs, this->http_bar_com_.url(), "hij=pqr")); EXPECT_TRUE(this->SetCookie(cs, this->http_bar_com_.url(), "xyz=zyx")); EXPECT_TRUE(this->SetCookie(cs, this->http_bar_com_.url(), "abc=stu")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); EXPECT_EQ(1u, cookie_changes_1.size()); EXPECT_EQ(1u, cookie_changes_2.size()); EXPECT_EQ(1u, cookie_changes_3.size()); @@ -1164,25 +2583,28 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsFiltering) { EXPECT_TRUE( this->SetCookie(cs, this->http_www_foo_.url(), "abc=vwx; path=/foo")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); - ASSERT_EQ(1u, cookie_changes_1.size()); + ASSERT_LE(1u, cookie_changes_1.size()); EXPECT_EQ("abc", cookie_changes_1[0].first.Name()); EXPECT_EQ("def", cookie_changes_1[0].first.Value()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes_1[0].first.Domain()); + EXPECT_EQ(1u, cookie_changes_1.size()); - ASSERT_EQ(1u, cookie_changes_2.size()); + ASSERT_LE(1u, cookie_changes_2.size()); EXPECT_EQ("hij", cookie_changes_2[0].first.Name()); EXPECT_EQ("mno", cookie_changes_2[0].first.Value()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes_2[0].first.Domain()); + EXPECT_EQ(1u, cookie_changes_2.size()); - ASSERT_EQ(1u, cookie_changes_3.size()); + ASSERT_LE(1u, cookie_changes_3.size()); EXPECT_EQ("abc", cookie_changes_3[0].first.Name()); EXPECT_EQ("stu", cookie_changes_3[0].first.Value()); EXPECT_EQ(this->http_bar_com_.url().host(), cookie_changes_3[0].first.Domain()); + EXPECT_EQ(1u, cookie_changes_3.size()); ASSERT_LE(1u, cookie_changes_4.size()); EXPECT_EQ("abc", cookie_changes_4[0].first.Name()); @@ -1191,15 +2613,17 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_DifferentSubscriptionsFiltering) { EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes_4[0].first.Domain()); - ASSERT_EQ(2u, cookie_changes_4.size()); + ASSERT_LE(2u, cookie_changes_4.size()); EXPECT_EQ("abc", cookie_changes_4[1].first.Name()); EXPECT_EQ("vwx", cookie_changes_4[1].first.Value()); EXPECT_EQ("/foo", cookie_changes_4[1].first.Path()); EXPECT_EQ(this->http_www_foo_.url().host(), cookie_changes_4[1].first.Domain()); + + EXPECT_EQ(2u, cookie_changes_4.size()); } -TYPED_TEST_P(CookieStoreChangeTest, Named_MultipleSubscriptions) { +TYPED_TEST_P(CookieStoreChangeNamedTest, MultipleSubscriptions) { if (!TypeParam::supports_named_cookie_tracking || !TypeParam::supports_multiple_tracking_callbacks) return; @@ -1210,18 +2634,20 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_MultipleSubscriptions) { std::unique_ptr<CookieChangeSubscription> subscription1 = cs->GetChangeDispatcher().AddCallbackForCookie( this->http_www_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_1))); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_1))); std::unique_ptr<CookieChangeSubscription> subscription2 = cs->GetChangeDispatcher().AddCallbackForCookie( this->http_www_foo_.url(), "abc", - base::BindRepeating(&OnCookieChange, - base::Unretained(&cookie_changes_2))); - this->RunUntilIdle(); + base::BindRepeating( + &CookieStoreChangeTestBase<TypeParam>::OnCookieChange, + base::Unretained(&cookie_changes_2))); + this->DeliverChangeNotifications(); EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "xyz=zyx")); EXPECT_TRUE(this->SetCookie(cs, this->http_www_foo_.url(), "abc=def")); - this->RunUntilIdle(); + this->DeliverChangeNotifications(); ASSERT_EQ(1U, cookie_changes_1.size()); EXPECT_EQ("abc", cookie_changes_1[0].first.Name()); @@ -1234,37 +2660,68 @@ TYPED_TEST_P(CookieStoreChangeTest, Named_MultipleSubscriptions) { cookie_changes_2.clear(); } -REGISTER_TYPED_TEST_CASE_P(CookieStoreChangeTest, - Global_NoCookie, - Global_InitialCookie, - Global_Insert, - Global_Delete, - Global_Overwrite, - Global_OverwriteWithHttpOnly, - Global_Deregister, - Global_DeregisterMultiple, - Global_DeregisterRace, - Global_DeregisterRaceMultiple, - Global_MultipleSubscriptions, - Named_NoCookie, - Named_InitialCookie, - Named_Insert, - Named_InsertFiltering, - Named_Delete, - Named_DeleteFiltering, - Named_Overwrite, - Named_OverwriteFiltering, - Named_OverwriteWithHttpOnly, - Named_Deregister, - Named_DeregisterMultiple, - Named_DeregisterRace, - Named_DeregisterRaceMultiple, - Named_DifferentSubscriptionsDisjoint, - Named_DifferentSubscriptionsDomains, - Named_DifferentSubscriptionsNames, - Named_DifferentSubscriptionsPaths, - Named_DifferentSubscriptionsFiltering, - Named_MultipleSubscriptions); +REGISTER_TYPED_TEST_CASE_P(CookieStoreChangeGlobalTest, + NoCookie, + InitialCookie, + InsertOne, + InsertMany, + DeleteOne, + DeleteTwo, + Overwrite, + OverwriteWithHttpOnly, + Deregister, + DeregisterMultiple, + DispatchRace, + DeregisterRace, + DeregisterRaceMultiple, + MultipleSubscriptions); + +REGISTER_TYPED_TEST_CASE_P(CookieStoreChangeUrlTest, + NoCookie, + InitialCookie, + InsertOne, + InsertMany, + InsertFiltering, + DeleteOne, + DeleteTwo, + DeleteFiltering, + Overwrite, + OverwriteFiltering, + OverwriteWithHttpOnly, + Deregister, + DeregisterMultiple, + DispatchRace, + DeregisterRace, + DeregisterRaceMultiple, + DifferentSubscriptionsDisjoint, + DifferentSubscriptionsDomains, + DifferentSubscriptionsPaths, + DifferentSubscriptionsFiltering, + MultipleSubscriptions); + +REGISTER_TYPED_TEST_CASE_P(CookieStoreChangeNamedTest, + NoCookie, + InitialCookie, + InsertOne, + InsertTwo, + InsertFiltering, + DeleteOne, + DeleteTwo, + DeleteFiltering, + Overwrite, + OverwriteFiltering, + OverwriteWithHttpOnly, + Deregister, + DeregisterMultiple, + DispatchRace, + DeregisterRace, + DeregisterRaceMultiple, + DifferentSubscriptionsDisjoint, + DifferentSubscriptionsDomains, + DifferentSubscriptionsNames, + DifferentSubscriptionsPaths, + DifferentSubscriptionsFiltering, + MultipleSubscriptions); } // namespace net diff --git a/chromium/net/cookies/cookie_store_test_helpers.cc b/chromium/net/cookies/cookie_store_test_helpers.cc index 13e0f20b64a..66d66609240 100644 --- a/chromium/net/cookies/cookie_store_test_helpers.cc +++ b/chromium/net/cookies/cookie_store_test_helpers.cc @@ -49,6 +49,13 @@ DelayedCookieMonsterChangeDispatcher::AddCallbackForCookie( return nullptr; } std::unique_ptr<CookieChangeSubscription> +DelayedCookieMonsterChangeDispatcher::AddCallbackForUrl( + const GURL& url, + CookieChangeCallback callback) { + ADD_FAILURE(); + return nullptr; +} +std::unique_ptr<CookieChangeSubscription> DelayedCookieMonsterChangeDispatcher::AddCallbackForAllChanges( CookieChangeCallback callback) { ADD_FAILURE(); @@ -220,4 +227,62 @@ std::string CookieURLHelper::Format(const std::string& format_string) const { return new_string; } +// +// FlushablePersistentStore +// +FlushablePersistentStore::FlushablePersistentStore() : flush_count_(0) {} + +void FlushablePersistentStore::Load(const LoadedCallback& loaded_callback) { + std::vector<std::unique_ptr<CanonicalCookie>> out_cookies; + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::BindOnce(loaded_callback, std::move(out_cookies))); +} + +void FlushablePersistentStore::LoadCookiesForKey( + const std::string& key, + const LoadedCallback& loaded_callback) { + Load(loaded_callback); +} + +void FlushablePersistentStore::AddCookie(const CanonicalCookie&) {} + +void FlushablePersistentStore::UpdateCookieAccessTime(const CanonicalCookie&) {} + +void FlushablePersistentStore::DeleteCookie(const CanonicalCookie&) {} + +void FlushablePersistentStore::SetForceKeepSessionState() {} + +void FlushablePersistentStore::SetBeforeFlushCallback( + base::RepeatingClosure callback) {} + +void FlushablePersistentStore::Flush(base::OnceClosure callback) { + base::AutoLock lock(flush_count_lock_); + ++flush_count_; + std::move(callback).Run(); +} + +int FlushablePersistentStore::flush_count() { + base::AutoLock lock(flush_count_lock_); + return flush_count_; +} + +FlushablePersistentStore::~FlushablePersistentStore() = default; + +// +// CallbackCounter +// +CallbackCounter::CallbackCounter() : callback_count_(0) {} + +void CallbackCounter::Callback() { + base::AutoLock lock(callback_count_lock_); + ++callback_count_; +} + +int CallbackCounter::callback_count() { + base::AutoLock lock(callback_count_lock_); + return callback_count_; +} + +CallbackCounter::~CallbackCounter() = default; + } // namespace net diff --git a/chromium/net/cookies/cookie_store_test_helpers.h b/chromium/net/cookies/cookie_store_test_helpers.h index e833147ab0e..440f70cf8ce 100644 --- a/chromium/net/cookies/cookie_store_test_helpers.h +++ b/chromium/net/cookies/cookie_store_test_helpers.h @@ -12,6 +12,7 @@ #include "base/callback_forward.h" #include "base/macros.h" +#include "base/synchronization/lock.h" #include "net/cookies/cookie_change_dispatcher.h" #include "testing/gtest/include/gtest/gtest.h" @@ -29,6 +30,9 @@ class DelayedCookieMonsterChangeDispatcher : public CookieChangeDispatcher { const GURL& url, const std::string& name, CookieChangeCallback callback) override WARN_UNUSED_RESULT; + std::unique_ptr<CookieChangeSubscription> AddCallbackForUrl( + const GURL& url, + CookieChangeCallback callback) override WARN_UNUSED_RESULT; std::unique_ptr<CookieChangeSubscription> AddCallbackForAllChanges( CookieChangeCallback callback) override WARN_UNUSED_RESULT; @@ -144,6 +148,46 @@ class CookieURLHelper { const std::string domain_and_registry_; }; +// Mock PersistentCookieStore that keeps track of the number of Flush() calls. +class FlushablePersistentStore : public CookieMonster::PersistentCookieStore { + public: + FlushablePersistentStore(); + + // CookieMonster::PersistentCookieStore implementation: + void Load(const LoadedCallback& loaded_callback) override; + void LoadCookiesForKey(const std::string& key, + const LoadedCallback& loaded_callback) override; + void AddCookie(const CanonicalCookie&) override; + void UpdateCookieAccessTime(const CanonicalCookie&) override; + void DeleteCookie(const CanonicalCookie&) override; + void SetForceKeepSessionState() override; + void SetBeforeFlushCallback(base::RepeatingClosure callback) override; + void Flush(base::OnceClosure callback) override; + + int flush_count(); + + private: + ~FlushablePersistentStore() override; + + int flush_count_; + base::Lock flush_count_lock_; // Protects |flush_count_|. +}; + +// Counts the number of times Callback() has been run. +class CallbackCounter : public base::RefCountedThreadSafe<CallbackCounter> { + public: + CallbackCounter(); + void Callback(); + int callback_count(); + + private: + friend class base::RefCountedThreadSafe<CallbackCounter>; + ~CallbackCounter(); + + int callback_count_; + base::Lock callback_count_lock_; // Protects |callback_count_|. +}; + } // namespace net #endif // NET_COOKIES_COOKIE_STORE_TEST_HELPERS_H_ diff --git a/chromium/net/cookies/cookie_store_unittest.h b/chromium/net/cookies/cookie_store_unittest.h index 1d36cbe603a..c715a19ce3b 100644 --- a/chromium/net/cookies/cookie_store_unittest.h +++ b/chromium/net/cookies/cookie_store_unittest.h @@ -47,8 +47,8 @@ const char kValidCookieLine[] = "A=B; path=/"; // // Factory function. Will be called at most once per test. // static std::unique_ptr<CookieStore> Create(); // -// // Drains the run loop(s) involved in the test. -// static void RunUntilIdle(); +// // Drains the run loop(s) used to deliver cookie change notifications. +// static void DeliverChangeNotifications(); // // // The cookie store supports cookies with the exclude_httponly() option. // static const bool supports_http_only; @@ -75,6 +75,10 @@ const char kValidCookieLine[] = "A=B; path=/"; // // calls to CookieStore::AddCallbackForAllChanges()). // static const bool supports_global_cookie_tracking; // +// // The cookie store supports tracking of cookie changes for an URL (i.e. +// // calls to CookieStore::AddCallbackForUrl()). +// static const bool supports_url_cookie_tracking; +// // // The cookie store supports tracking of named cookie changes (i.e. // // calls to CookieStore::AddCallbackForCookie()). // static const bool supports_named_cookie_tracking; @@ -82,6 +86,16 @@ const char kValidCookieLine[] = "A=B; path=/"; // // The cookie store supports more than one callback per cookie change type. // static const bool supports_multiple_tracking_callbacks; // +// // The cookie store correctly distinguishes between OVERWRITE and EXPLICIT +// // (deletion) change causes. +// static const bool has_exact_change_cause; +// +// // The cookie store is guaranteed to deliver cookie changes in the order +// // in which calls were issued. This only applies to changes coming from +// // _different_ calls. If a call results in a cookie overwrite, the deletion +// // change must still be issued before the insertion change. +// static const bool has_exact_change_ordering; +// // // Time to wait between two cookie insertions to ensure that cookies have // // different creation times. // static const int creation_time_granularity_in_ms; @@ -298,9 +312,6 @@ class CookieStoreTest : public testing::Test { return cookie_store_.get(); } - // Drains all pending tasks on the run loop(s) involved in the test. - void RunUntilIdle() { CookieStoreTestTraits::RunUntilIdle(); } - // Compares two cookie lines. void MatchCookieLines(const std::string& line1, const std::string& line2) { EXPECT_EQ(TokenizeCookieLine(line1), TokenizeCookieLine(line2)); diff --git a/chromium/net/cookies/cookie_util.cc b/chromium/net/cookies/cookie_util.cc index cf133b61913..0a1ecac9b95 100644 --- a/chromium/net/cookies/cookie_util.cc +++ b/chromium/net/cookies/cookie_util.cc @@ -273,6 +273,39 @@ GURL CookieOriginToURL(const std::string& domain, bool is_https) { return GURL(scheme + "://" + host); } +bool IsDomainMatch(const std::string& domain, const std::string& host) { + // Can domain match in two ways; as a domain cookie (where the cookie + // domain begins with ".") or as a host cookie (where it doesn't). + + // Some consumers of the CookieMonster expect to set cookies on + // URLs like http://.strange.url. To retrieve cookies in this instance, + // we allow matching as a host cookie even when the domain_ starts with + // a period. + if (host == domain) + return true; + + // Domain cookie must have an initial ".". To match, it must be + // equal to url's host with initial period removed, or a suffix of + // it. + + // Arguably this should only apply to "http" or "https" cookies, but + // extension cookie tests currently use the funtionality, and if we + // ever decide to implement that it should be done by preventing + // such cookies from being set. + if (domain.empty() || domain[0] != '.') + return false; + + // The host with a "." prefixed. + if (domain.compare(1, std::string::npos, host) == 0) + return true; + + // A pure suffix of the host (ok since we know the domain already + // starts with a ".") + return (host.length() > domain.length() && + host.compare(host.length() - domain.length(), domain.length(), + domain) == 0); +} + void ParseRequestCookieLine(const std::string& header_value, ParsedRequestCookies* parsed_cookies) { std::string::const_iterator i = header_value.begin(); diff --git a/chromium/net/cookies/cookie_util.h b/chromium/net/cookies/cookie_util.h index 60e5f661cd4..18a3bfbeb50 100644 --- a/chromium/net/cookies/cookie_util.h +++ b/chromium/net/cookies/cookie_util.h @@ -48,6 +48,11 @@ NET_EXPORT base::Time ParseCookieExpirationTime(const std::string& time_string); // Convenience for converting a cookie origin (domain and https pair) to a URL. NET_EXPORT GURL CookieOriginToURL(const std::string& domain, bool is_https); +// Returns true if the cookie |domain| matches the given |host| as described +// in section 5.1.3 of RFC 6265. +NET_EXPORT bool IsDomainMatch(const std::string& domain, + const std::string& host); + // A ParsedRequestCookie consists of the key and value of the cookie. typedef std::pair<base::StringPiece, base::StringPiece> ParsedRequestCookie; typedef std::vector<ParsedRequestCookie> ParsedRequestCookies; diff --git a/chromium/net/cookies/cookie_util_unittest.cc b/chromium/net/cookies/cookie_util_unittest.cc index 7d55dd9e9d2..79c952bd04b 100644 --- a/chromium/net/cookies/cookie_util_unittest.cc +++ b/chromium/net/cookies/cookie_util_unittest.cc @@ -250,6 +250,19 @@ TEST(CookieUtilTest, TestGetEffectiveDomain) { cookie_util::GetEffectiveDomain("ftp", "www.example.com")); } +TEST(CookieUtilTest, TestIsDomainMatch) { + EXPECT_TRUE(cookie_util::IsDomainMatch("example.com", "example.com")); + EXPECT_FALSE(cookie_util::IsDomainMatch("www.example.com", "example.com")); + + EXPECT_TRUE(cookie_util::IsDomainMatch(".example.com", "example.com")); + EXPECT_TRUE(cookie_util::IsDomainMatch(".example.com", "www.example.com")); + EXPECT_FALSE(cookie_util::IsDomainMatch(".www.example.com", "example.com")); + + EXPECT_FALSE(cookie_util::IsDomainMatch("example.com", "example.de")); + EXPECT_FALSE(cookie_util::IsDomainMatch(".example.com", "example.de")); + EXPECT_FALSE(cookie_util::IsDomainMatch(".example.de", "example.de.vu")); +} + } // namespace } // namespace net diff --git a/chromium/net/cookies/parsed_cookie.cc b/chromium/net/cookies/parsed_cookie.cc index 1be10eed753..450e869e8ba 100644 --- a/chromium/net/cookies/parsed_cookie.cc +++ b/chromium/net/cookies/parsed_cookie.cc @@ -150,7 +150,7 @@ ParsedCookie::ParsedCookie(const std::string& cookie_line) ParsedCookie::~ParsedCookie() = default; bool ParsedCookie::IsValid() const { - return !pairs_.empty() && IsSameSiteAttributeValid(); + return !pairs_.empty(); } CookieSameSite ParsedCookie::SameSite() const { @@ -501,8 +501,4 @@ void ParsedCookie::ClearAttributePair(size_t index) { pairs_.erase(pairs_.begin() + index); } -bool ParsedCookie::IsSameSiteAttributeValid() const { - return same_site_index_ == 0 || SameSite() != CookieSameSite::DEFAULT_MODE; -} - } // namespace net diff --git a/chromium/net/cookies/parsed_cookie.h b/chromium/net/cookies/parsed_cookie.h index 8bc5e99b678..d6b951a1598 100644 --- a/chromium/net/cookies/parsed_cookie.h +++ b/chromium/net/cookies/parsed_cookie.h @@ -131,10 +131,6 @@ class NET_EXPORT ParsedCookie { // |index| refers to a position in |pairs_|. void ClearAttributePair(size_t index); - // Returns false if a 'SameSite' attribute is present, but has an unrecognized - // value. In particular, this includes attributes with empty values. - bool IsSameSiteAttributeValid() const; - PairList pairs_; // These will default to 0, but that should never be valid since the // 0th index is the user supplied token/value, not an attribute. diff --git a/chromium/net/cookies/parsed_cookie_unittest.cc b/chromium/net/cookies/parsed_cookie_unittest.cc index e1e3f9beca7..fc7d206a340 100644 --- a/chromium/net/cookies/parsed_cookie_unittest.cc +++ b/chromium/net/cookies/parsed_cookie_unittest.cc @@ -467,7 +467,7 @@ TEST(ParsedCookieTest, SetSameSite) { EXPECT_EQ("name=value", pc.ToCookieLine()); EXPECT_EQ(CookieSameSite::DEFAULT_MODE, pc.SameSite()); - // Test each priority, expect case-insensitive compare. + // Test each samesite directive, expect case-insensitive compare. EXPECT_TRUE(pc.SetSameSite("strict")); EXPECT_EQ("name=value; samesite=strict", pc.ToCookieLine()); EXPECT_EQ(CookieSameSite::STRICT_MODE, pc.SameSite()); @@ -483,24 +483,28 @@ TEST(ParsedCookieTest, SetSameSite) { EXPECT_EQ(CookieSameSite::LAX_MODE, pc.SameSite()); EXPECT_TRUE(pc.IsValid()); + // Remove the SameSite attribute. EXPECT_TRUE(pc.SetSameSite("")); EXPECT_EQ("name=value", pc.ToCookieLine()); EXPECT_EQ(CookieSameSite::DEFAULT_MODE, pc.SameSite()); EXPECT_TRUE(pc.IsValid()); EXPECT_TRUE(pc.SetSameSite("Blah")); - EXPECT_FALSE(pc.IsValid()); + EXPECT_EQ("name=value; samesite=Blah", pc.ToCookieLine()); + EXPECT_EQ(CookieSameSite::NO_RESTRICTION, pc.SameSite()); + EXPECT_TRUE(pc.IsValid()); } -TEST(ParsedCookieTest, InvalidSameSiteValue) { +TEST(ParsedCookieTest, SameSiteValues) { struct TestCase { const char* cookie; bool valid; CookieSameSite mode; } cases[]{{"n=v; samesite=strict", true, CookieSameSite::STRICT_MODE}, {"n=v; samesite=lax", true, CookieSameSite::LAX_MODE}, - {"n=v; samesite=boo", false, CookieSameSite::DEFAULT_MODE}, - {"n=v; samesite", false, CookieSameSite::DEFAULT_MODE}}; + {"n=v; samesite=boo", true, CookieSameSite::NO_RESTRICTION}, + {"n=v; samesite", true, CookieSameSite::NO_RESTRICTION}, + {"n=v", true, CookieSameSite::DEFAULT_MODE}}; for (const auto& test : cases) { SCOPED_TRACE(test.cookie); |