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

Unified Diff: ios/net/cookies/cookie_store_ios.mm

Issue 2649083002: Divide CookieStoreIOS into two different classes with different backends (Closed)
Patch Set: fix compilation Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: ios/net/cookies/cookie_store_ios.mm
diff --git a/ios/net/cookies/cookie_store_ios.mm b/ios/net/cookies/cookie_store_ios.mm
index e0e5280b454467f2be9868540100e41c88ab5d18..fb231140ed3e006115b3921dd3f391a7bd30866f 100644
--- a/ios/net/cookies/cookie_store_ios.mm
+++ b/ios/net/cookies/cookie_store_ios.mm
@@ -274,11 +274,8 @@ bool HasExplicitDomain(const std::string& cookie_line) {
#pragma mark -
#pragma mark CookieStoreIOS
-CookieStoreIOS::CookieStoreIOS(
- net::CookieMonster::PersistentCookieStore* persistent_store)
- : CookieStoreIOS(persistent_store,
- [NSHTTPCookieStorage sharedHTTPCookieStorage]) {
-}
+CookieStoreIOS::CookieStoreIOS(NSHTTPCookieStorage* cookie_storage)
+ : CookieStoreIOS(nullptr, cookie_storage) {}
CookieStoreIOS::~CookieStoreIOS() {
NotificationTrampoline::GetInstance()->RemoveObserver(this);
@@ -294,8 +291,7 @@ std::unique_ptr<CookieStoreIOS> CookieStoreIOS::CreateCookieStore(
// Create a cookie store with no persistent store backing. Then, populate
// it from the system's cookie jar.
std::unique_ptr<CookieStoreIOS> cookie_store(
- new CookieStoreIOS(nullptr, cookie_storage));
- cookie_store->synchronization_state_ = SYNCHRONIZED;
+ new CookieStoreIOS(cookie_storage));
cookie_store->FlushStore(base::Closure());
return cookie_store;
}
@@ -323,57 +319,49 @@ void CookieStoreIOS::SetCookieWithOptionsAsync(
const SetCookiesCallback& callback) {
DCHECK(thread_checker_.CalledOnValidThread());
- switch (synchronization_state_) {
- case NOT_SYNCHRONIZED:
- cookie_monster_->SetCookieWithOptionsAsync(url, cookie_line, options,
- WrapSetCallback(callback));
- break;
- case SYNCHRONIZED:
- // The exclude_httponly() option would only be used by a javascript
- // engine.
- DCHECK(!options.exclude_httponly());
- // If cookies are not allowed, they are stashed in the CookieMonster, and
- // should be written there instead.
- DCHECK(SystemCookiesAllowed());
-
- base::Time server_time =
- options.has_server_time() ? options.server_time() : base::Time();
- NSHTTPCookie* cookie =
- GetNSHTTPCookieFromCookieLine(cookie_line, url, server_time);
- DLOG_IF(WARNING, !cookie)
- << "Could not create cookie for line: " << cookie_line;
-
- // On iOS, [cookie domain] is not empty when the cookie domain is not
- // specified: it is inferred from the URL instead. The only case when it
- // is empty is when the domain attribute is incorrectly formatted.
- std::string domain_string(base::SysNSStringToUTF8([cookie domain]));
- std::string dummy;
- bool has_explicit_domain = HasExplicitDomain(cookie_line);
- bool has_valid_domain =
- net::cookie_util::GetCookieDomainWithString(
- url, domain_string, &dummy);
- // A cookie can be set if all of:
- // a) The cookie line is well-formed
- // b) The Domain attribute, if present, was not malformed
- // c) At least one of:
- // 1) The cookie had no explicit Domain, so the Domain was inferred
- // from the URL, or
- // 2) The cookie had an explicit Domain for which the URL is allowed
- // to set cookies.
- bool success = (cookie != nil) && !domain_string.empty() &&
- (!has_explicit_domain || has_valid_domain);
-
- if (success) {
- [system_store_ setCookie:cookie];
- creation_time_manager_->SetCreationTime(
- cookie,
- creation_time_manager_->MakeUniqueCreationTime(base::Time::Now()));
- }
-
- if (!callback.is_null())
- callback.Run(success);
- break;
+ // The exclude_httponly() option would only be used by a javascript
+ // engine.
+ DCHECK(!options.exclude_httponly());
+
+ // If cookies are not allowed, they are stashed in the CookieMonster, and
+ // should be written there instead.
+ DCHECK(SystemCookiesAllowed());
mef 2017/02/09 18:37:16 This check fails on Cronet tester: https://luci-lo
Eugene But (OOO till 7-30) 2017/02/10 00:10:56 Sorry, I missed this during review. CookieStoreIOS
mef 2017/02/10 16:33:57 sgtm. Looking at CookieStoreIOS::CreateCookieStore
+
+ base::Time server_time =
+ options.has_server_time() ? options.server_time() : base::Time();
+ NSHTTPCookie* cookie =
+ GetNSHTTPCookieFromCookieLine(cookie_line, url, server_time);
+ DLOG_IF(WARNING, !cookie) << "Could not create cookie for line: "
+ << cookie_line;
+
+ // On iOS, [cookie domain] is not empty when the cookie domain is not
+ // specified: it is inferred from the URL instead. The only case when it
+ // is empty is when the domain attribute is incorrectly formatted.
+ std::string domain_string(base::SysNSStringToUTF8([cookie domain]));
+ std::string dummy;
+ bool has_explicit_domain = HasExplicitDomain(cookie_line);
+ bool has_valid_domain =
+ net::cookie_util::GetCookieDomainWithString(url, domain_string, &dummy);
+ // A cookie can be set if all of:
+ // a) The cookie line is well-formed
+ // b) The Domain attribute, if present, was not malformed
+ // c) At least one of:
+ // 1) The cookie had no explicit Domain, so the Domain was inferred
+ // from the URL, or
+ // 2) The cookie had an explicit Domain for which the URL is allowed
+ // to set cookies.
+ bool success = (cookie != nil) && !domain_string.empty() &&
+ (!has_explicit_domain || has_valid_domain);
+
+ if (success) {
+ [system_store_ setCookie:cookie];
+ creation_time_manager_->SetCreationTime(
+ cookie,
+ creation_time_manager_->MakeUniqueCreationTime(base::Time::Now()));
}
+
+ if (!callback.is_null())
+ callback.Run(success);
}
void CookieStoreIOS::SetCookieWithDetailsAsync(
@@ -391,48 +379,36 @@ void CookieStoreIOS::SetCookieWithDetailsAsync(
CookiePriority priority,
const SetCookiesCallback& callback) {
DCHECK(thread_checker_.CalledOnValidThread());
+ // If cookies are not allowed, they are stashed in the CookieMonster, and
+ // should be written there instead.
+ DCHECK(SystemCookiesAllowed());
+
+ bool success = false;
+
+ if (creation_time.is_null())
+ creation_time = base::Time::Now();
- switch (synchronization_state_) {
- case NOT_SYNCHRONIZED:
- cookie_monster_->SetCookieWithDetailsAsync(
+ // First create a CanonicalCookie, to normalize the arguments,
+ // particularly domain and path, and perform validation.
+ std::unique_ptr<net::CanonicalCookie> canonical_cookie =
+ net::CanonicalCookie::Create(
url, name, value, domain, path, creation_time, expiration_time,
- last_access_time, secure, http_only, same_site, priority,
- WrapSetCallback(callback));
- break;
- case SYNCHRONIZED:
- // If cookies are not allowed, they are stashed in the CookieMonster, and
- // should be written there instead.
- DCHECK(SystemCookiesAllowed());
-
- bool success = false;
-
- if (creation_time.is_null())
- creation_time = base::Time::Now();
-
- // First create a CanonicalCookie, to normalize the arguments,
- // particularly domain and path, and perform validation.
- std::unique_ptr<net::CanonicalCookie> canonical_cookie =
- net::CanonicalCookie::Create(
- url, name, value, domain, path, creation_time, expiration_time,
- secure, http_only, same_site, priority);
-
- if (canonical_cookie) {
- NSHTTPCookie* cookie =
- SystemCookieFromCanonicalCookie(*canonical_cookie);
-
- if (cookie != nil) {
- [system_store_ setCookie:cookie];
- creation_time_manager_->SetCreationTime(
- cookie, creation_time_manager_->MakeUniqueCreationTime(
- canonical_cookie->CreationDate()));
- success = true;
- }
- }
-
- if (!callback.is_null())
- callback.Run(success);
- break;
+ secure, http_only, same_site, priority);
+
+ if (canonical_cookie) {
+ NSHTTPCookie* cookie = SystemCookieFromCanonicalCookie(*canonical_cookie);
+
+ if (cookie != nil) {
+ [system_store_ setCookie:cookie];
+ creation_time_manager_->SetCreationTime(
+ cookie, creation_time_manager_->MakeUniqueCreationTime(
+ canonical_cookie->CreationDate()));
+ success = true;
+ }
}
+
+ if (!callback.is_null())
+ callback.Run(success);
}
void CookieStoreIOS::GetCookiesWithOptionsAsync(
@@ -441,26 +417,19 @@ void CookieStoreIOS::GetCookiesWithOptionsAsync(
const GetCookiesCallback& callback) {
DCHECK(thread_checker_.CalledOnValidThread());
- switch (synchronization_state_) {
- case NOT_SYNCHRONIZED:
- cookie_monster_->GetCookiesWithOptionsAsync(url, options, callback);
- break;
- case SYNCHRONIZED:
- // If cookies are not allowed, they are stashed in the CookieMonster, and
- // should be read from there instead.
- DCHECK(SystemCookiesAllowed());
- // The exclude_httponly() option would only be used by a javascript
- // engine.
- DCHECK(!options.exclude_httponly());
-
- // TODO(mkwst): If/when iOS supports Same-Site cookies, we'll need to pass
- // options in here as well. https://crbug.com/459154
- NSArray* cookies = GetCookiesForURL(system_store_,
- url, creation_time_manager_.get());
- if (!callback.is_null())
- callback.Run(BuildCookieLineWithOptions(cookies, options));
- break;
- }
+ // If cookies are not allowed, they are stashed in the CookieMonster, and
+ // should be read from there instead.
+ DCHECK(SystemCookiesAllowed());
+ // The exclude_httponly() option would only be used by a javascript
+ // engine.
+ DCHECK(!options.exclude_httponly());
+
+ // TODO(mkwst): If/when iOS supports Same-Site cookies, we'll need to pass
+ // options in here as well. https://crbug.com/459154
+ NSArray* cookies =
+ GetCookiesForURL(system_store_, url, creation_time_manager_.get());
+ if (!callback.is_null())
+ callback.Run(BuildCookieLineWithOptions(cookies, options));
}
void CookieStoreIOS::GetCookieListWithOptionsAsync(
@@ -468,54 +437,36 @@ void CookieStoreIOS::GetCookieListWithOptionsAsync(
const net::CookieOptions& options,
const GetCookieListCallback& callback) {
DCHECK(thread_checker_.CalledOnValidThread());
-
- switch (synchronization_state_) {
- case NOT_SYNCHRONIZED:
- cookie_monster_->GetCookieListWithOptionsAsync(url, options, callback);
- break;
- case SYNCHRONIZED:
- if (!SystemCookiesAllowed()) {
- // If cookies are not allowed, the cookies are stashed in the
- // CookieMonster, so get them from there.
- cookie_monster_->GetCookieListWithOptionsAsync(url, options, callback);
- return;
- }
-
- // TODO(mkwst): If/when iOS supports Same-Site cookies, we'll need to pass
- // options in here as well. https://crbug.com/459154
- NSArray* cookies = GetCookiesForURL(system_store_,
- url, creation_time_manager_.get());
- net::CookieList cookie_list = CanonicalCookieListFromSystemCookies(
- cookies);
- if (!callback.is_null())
- callback.Run(cookie_list);
- break;
+ if (!SystemCookiesAllowed()) {
+ // If cookies are not allowed, the cookies are stashed in the
+ // CookieMonster, so get them from there.
+ cookie_monster_->GetCookieListWithOptionsAsync(url, options, callback);
+ return;
}
+
+ // TODO(mkwst): If/when iOS supports Same-Site cookies, we'll need to pass
+ // options in here as well. https://crbug.com/459154
+ NSArray* cookies =
+ GetCookiesForURL(system_store_, url, creation_time_manager_.get());
+ net::CookieList cookie_list = CanonicalCookieListFromSystemCookies(cookies);
+ if (!callback.is_null())
+ callback.Run(cookie_list);
}
void CookieStoreIOS::GetAllCookiesAsync(const GetCookieListCallback& callback) {
DCHECK(thread_checker_.CalledOnValidThread());
- switch (synchronization_state_) {
- case NOT_SYNCHRONIZED:
- cookie_monster_->GetAllCookiesAsync(callback);
- break;
- case SYNCHRONIZED:
- if (!SystemCookiesAllowed()) {
- // If cookies are not allowed, the cookies are stashed in the
- // CookieMonster, so get them from there.
- cookie_monster_->GetAllCookiesAsync(callback);
- return;
- }
-
- NSArray* cookies = GetAllCookies(system_store_,
- creation_time_manager_.get());
- net::CookieList cookie_list = CanonicalCookieListFromSystemCookies(
- cookies);
- if (!callback.is_null()) {
- callback.Run(cookie_list);
- }
- break;
+ if (!SystemCookiesAllowed()) {
+ // If cookies are not allowed, the cookies are stashed in the
+ // CookieMonster, so get them from there.
+ cookie_monster_->GetAllCookiesAsync(callback);
+ return;
+ }
+
+ NSArray* cookies = GetAllCookies(system_store_, creation_time_manager_.get());
+ net::CookieList cookie_list = CanonicalCookieListFromSystemCookies(cookies);
+ if (!callback.is_null()) {
+ callback.Run(cookie_list);
}
}
@@ -524,25 +475,17 @@ void CookieStoreIOS::DeleteCookieAsync(const GURL& url,
const base::Closure& callback) {
DCHECK(thread_checker_.CalledOnValidThread());
- switch (synchronization_state_) {
- case NOT_SYNCHRONIZED:
- cookie_monster_->DeleteCookieAsync(url, cookie_name,
- WrapClosure(callback));
- break;
- case SYNCHRONIZED:
- NSArray* cookies = GetCookiesForURL(system_store_,
- url, creation_time_manager_.get());
- for (NSHTTPCookie* cookie in cookies) {
- if ([[cookie name]
- isEqualToString:base::SysUTF8ToNSString(cookie_name)]) {
- [system_store_ deleteCookie:cookie];
- creation_time_manager_->DeleteCreationTime(cookie);
- }
- }
- if (!callback.is_null())
- callback.Run();
- break;
+ NSArray* cookies =
+ GetCookiesForURL(system_store_, url, creation_time_manager_.get());
+ for (NSHTTPCookie* cookie in cookies) {
+ if ([[cookie name] isEqualToString:base::SysUTF8ToNSString(cookie_name)]) {
+ [system_store_ deleteCookie:cookie];
+ creation_time_manager_->DeleteCreationTime(cookie);
+ }
}
+
+ if (!callback.is_null())
+ callback.Run();
}
void CookieStoreIOS::DeleteCanonicalCookieAsync(
@@ -550,17 +493,10 @@ void CookieStoreIOS::DeleteCanonicalCookieAsync(
const DeleteCallback& callback) {
DCHECK(thread_checker_.CalledOnValidThread());
- switch (synchronization_state_) {
- case NOT_SYNCHRONIZED:
- cookie_monster_->DeleteCanonicalCookieAsync(cookie,
- WrapDeleteCallback(callback));
- break;
- case SYNCHRONIZED:
- // This relies on the fact cookies are given unique creation dates.
- CookieFilterFunction filter = base::Bind(
- IsCookieCreatedBetween, cookie.CreationDate(), cookie.CreationDate());
- DeleteCookiesWithFilter(filter, callback);
- }
+ // This relies on the fact cookies are given unique creation dates.
+ CookieFilterFunction filter = base::Bind(
+ IsCookieCreatedBetween, cookie.CreationDate(), cookie.CreationDate());
+ DeleteCookiesWithFilter(filter, callback);
}
void CookieStoreIOS::DeleteAllCreatedBetweenAsync(
@@ -572,17 +508,9 @@ void CookieStoreIOS::DeleteAllCreatedBetweenAsync(
if (metrics_enabled_)
ResetCookieCountMetrics();
- switch (synchronization_state_) {
- case NOT_SYNCHRONIZED:
- cookie_monster_->DeleteAllCreatedBetweenAsync(
- delete_begin, delete_end, WrapDeleteCallback(callback));
- break;
- case SYNCHRONIZED:
- CookieFilterFunction filter =
- base::Bind(&IsCookieCreatedBetween, delete_begin, delete_end);
- DeleteCookiesWithFilter(filter, callback);
- break;
- }
+ CookieFilterFunction filter = base::Bind(
+ &IsCookieCreatedBetween, delete_begin, delete_end);
+ DeleteCookiesWithFilter(filter, callback);
}
void CookieStoreIOS::DeleteAllCreatedBetweenWithPredicateAsync(
@@ -595,18 +523,9 @@ void CookieStoreIOS::DeleteAllCreatedBetweenWithPredicateAsync(
if (metrics_enabled_)
ResetCookieCountMetrics();
- switch (synchronization_state_) {
- case NOT_SYNCHRONIZED:
- cookie_monster_->DeleteAllCreatedBetweenWithPredicateAsync(
- delete_begin, delete_end, predicate, WrapDeleteCallback(callback));
- break;
- case SYNCHRONIZED:
- CookieFilterFunction filter =
- base::Bind(IsCookieCreatedBetweenWithPredicate, delete_begin,
- delete_end, predicate);
- DeleteCookiesWithFilter(filter, callback);
- break;
- }
+ CookieFilterFunction filter = base::Bind(
+ IsCookieCreatedBetweenWithPredicate, delete_begin, delete_end, predicate);
+ DeleteCookiesWithFilter(filter, callback);
}
void CookieStoreIOS::DeleteSessionCookiesAsync(const DeleteCallback& callback) {
@@ -615,15 +534,8 @@ void CookieStoreIOS::DeleteSessionCookiesAsync(const DeleteCallback& callback) {
if (metrics_enabled_)
ResetCookieCountMetrics();
- switch (synchronization_state_) {
- case NOT_SYNCHRONIZED:
- cookie_monster_->DeleteSessionCookiesAsync(WrapDeleteCallback(callback));
- break;
- case SYNCHRONIZED:
- CookieFilterFunction filter = base::Bind(&IsCookieSessionCookie);
- DeleteCookiesWithFilter(filter, callback);
- break;
- }
+ CookieFilterFunction filter = base::Bind(&IsCookieSessionCookie);
+ DeleteCookiesWithFilter(filter, callback);
}
void CookieStoreIOS::FlushStore(const base::Closure& closure) {
@@ -639,7 +551,7 @@ void CookieStoreIOS::FlushStore(const base::Closure& closure) {
}
#pragma mark -
-#pragma mark Private methods
+#pragma mark Protected methods
CookieStoreIOS::CookieStoreIOS(
net::CookieMonster::PersistentCookieStore* persistent_store,
@@ -648,7 +560,6 @@ CookieStoreIOS::CookieStoreIOS(
system_store_(system_store),
creation_time_manager_(new CookieCreationTimeManager),
metrics_enabled_(false),
- synchronization_state_(NOT_SYNCHRONIZED),
cookie_cache_(new CookieCache()),
weak_factory_(this) {
DCHECK(system_store);
@@ -659,6 +570,29 @@ CookieStoreIOS::CookieStoreIOS(
cookie_monster_->SetForceKeepSessionState();
}
+CookieStoreIOS::SetCookiesCallback CookieStoreIOS::WrapSetCallback(
+ const SetCookiesCallback& callback) {
+ DCHECK(thread_checker_.CalledOnValidThread());
+ return base::Bind(&CookieStoreIOS::UpdateCachesAfterSet,
+ weak_factory_.GetWeakPtr(), callback);
+}
+
+CookieStoreIOS::DeleteCallback CookieStoreIOS::WrapDeleteCallback(
+ const DeleteCallback& callback) {
+ DCHECK(thread_checker_.CalledOnValidThread());
+ return base::Bind(&CookieStoreIOS::UpdateCachesAfterDelete,
+ weak_factory_.GetWeakPtr(), callback);
+}
+
+base::Closure CookieStoreIOS::WrapClosure(const base::Closure& callback) {
+ DCHECK(thread_checker_.CalledOnValidThread());
+ return base::Bind(&CookieStoreIOS::UpdateCachesAfterClosure,
+ weak_factory_.GetWeakPtr(), callback);
+}
+
+#pragma mark -
+#pragma mark Private methods
+
void CookieStoreIOS::ClearSystemStore() {
DCHECK(thread_checker_.CalledOnValidThread());
base::scoped_nsobject<NSArray> copy(
@@ -677,9 +611,6 @@ bool CookieStoreIOS::SystemCookiesAllowed() {
void CookieStoreIOS::WriteToCookieMonster(NSArray* system_cookies) {
DCHECK(thread_checker_.CalledOnValidThread());
- if (synchronization_state_ != SYNCHRONIZED)
- return;
-
// Copy the cookies from the global cookie store to |cookie_monster_|.
// Unlike the system store, CookieMonster requires unique creation times.
net::CookieList cookie_list;
@@ -699,7 +630,6 @@ void CookieStoreIOS::WriteToCookieMonster(NSArray* system_cookies) {
void CookieStoreIOS::DeleteCookiesWithFilter(const CookieFilterFunction& filter,
const DeleteCallback& callback) {
DCHECK(thread_checker_.CalledOnValidThread());
- DCHECK_EQ(SYNCHRONIZED, synchronization_state_);
NSArray* cookies = [system_store_ cookies];
// Collect the cookies to delete.
@@ -724,13 +654,6 @@ void CookieStoreIOS::DeleteCookiesWithFilter(const CookieFilterFunction& filter,
void CookieStoreIOS::OnSystemCookiesChanged() {
DCHECK(thread_checker_.CalledOnValidThread());
- // If the CookieStoreIOS is not synchronized or is not backed by
- // |NSHTTPCookieStorage sharedHTTPCookieStorage| this callback is irrelevant.
- if (synchronization_state_ != SYNCHRONIZED ||
- system_store_ != [NSHTTPCookieStorage sharedHTTPCookieStorage]) {
- return;
- }
-
for (const auto& hook_map_entry : hook_map_) {
std::pair<GURL, std::string> key = hook_map_entry.first;
std::vector<net::CanonicalCookie> removed_cookies;
@@ -914,24 +837,4 @@ CookieStoreIOS::CanonicalCookieListFromSystemCookies(NSArray* cookies) {
return cookie_list;
}
-CookieStoreIOS::SetCookiesCallback CookieStoreIOS::WrapSetCallback(
- const SetCookiesCallback& callback) {
- DCHECK(thread_checker_.CalledOnValidThread());
- return base::Bind(&CookieStoreIOS::UpdateCachesAfterSet,
- weak_factory_.GetWeakPtr(), callback);
-}
-
-CookieStoreIOS::DeleteCallback CookieStoreIOS::WrapDeleteCallback(
- const DeleteCallback& callback) {
- DCHECK(thread_checker_.CalledOnValidThread());
- return base::Bind(&CookieStoreIOS::UpdateCachesAfterDelete,
- weak_factory_.GetWeakPtr(), callback);
-}
-
-base::Closure CookieStoreIOS::WrapClosure(const base::Closure& callback) {
- DCHECK(thread_checker_.CalledOnValidThread());
- return base::Bind(&CookieStoreIOS::UpdateCachesAfterClosure,
- weak_factory_.GetWeakPtr(), callback);
-}
-
} // namespace net

Powered by Google App Engine
This is Rietveld 408576698