Skip to content

GitLab

T
tangled
Commit d649da83 authored by eugenebut's avatar eugenebut Committed by Commit bot
Browse files
Options

[ios] Cleaned up comments and tests for CookieStoreIOS synchronization. 

In UIWebView world CookieStoreIOS could dynamically change its state
and either be synchronized with NSHTTPCookieStorage or unsynchronized.

In WKWebView world CookieStoreIOS never changes its state. It is either
always synchronized (for sign in) or always unsynchronized (for regular
browsing).

Notable changes:
- updated comments to reflect that sync state is not dynamic
- removed RoundTripTest which tested non-existing dynamic synchronization flow
- removed other tests which tests dynamic synchronization

BUG=676144

Review-Url: https://codereview.chromium.org/2604763003
Cr-Commit-Position: refs/heads/master@{#440851}
parent b23fa511
Hide whitespace changes
Inline Side-by-side
Showing with 8 additions and 249 deletions
ios/net/cookies/cookie_store_ios.h
View file @ d649da83
...@@ -41,24 +41,16 @@ class CookieNotificationObserver { ...@@ -41,24 +41,16 @@ class CookieNotificationObserver {
// The CookieStoreIOS is an implementation of CookieStore relying on // The CookieStoreIOS is an implementation of CookieStore relying on
// NSHTTPCookieStorage, ensuring that the cookies are consistent between the // NSHTTPCookieStorage, ensuring that the cookies are consistent between the
// network stack and the UIWebViews. // network stack and NSHTTPCookieStorage.
// On iOS, the Chrome CookieMonster is not used in conjunction with UIWebView,
// because UIWebView expects the cookies to be in the shared
// NSHTTPCookieStorage. In particular, javascript may read and write cookies
// there.
// CookieStoreIOS is not thread safe. // CookieStoreIOS is not thread safe.
// //
// At any given time, a CookieStoreIOS can either be synchronized with the // CookieStoreIOS can be created synchronized with the system cookie store (via
// system cookie store or not. If a CookieStoreIOS is not synchronized with the // CreateCookieStore) or not (other constructors). If a CookieStoreIOS is not
// system store, changes are written back to the backing CookieStore. If a // synchronized with the system store, changes are written back to the backing
// CookieStoreIOS is synchronized with the system store, changes are written // CookieStore. If a CookieStoreIOS is synchronized with the system store,
// directly to the system cookie store, then propagated to the backing store by // changes are written directly to the system cookie store, then propagated to
// OnSystemCookiesChanged, which is called by the system store once the change // the backing store by OnSystemCookiesChanged, which is called by the system
// to the system store is written back. // store once the change to the system store is written back.
//
// To unsynchronize, CookieStoreIOS copies the system cookie store into its
// backing CookieStore. To synchronize, CookieStoreIOS clears the system cookie
// store, copies its backing CookieStore into the system cookie store.
class CookieStoreIOS : public net::CookieStore, class CookieStoreIOS : public net::CookieStore,
public CookieNotificationObserver { public CookieNotificationObserver {
public: public:
......
ios/net/cookies/cookie_store_ios_unittest.mm
View file @ d649da83
...@@ -69,160 +69,6 @@ struct InactiveCookieStoreIOSTestTraits { ...@@ -69,160 +69,6 @@ struct InactiveCookieStoreIOSTestTraits {
base::MessageLoop loop_; base::MessageLoop loop_;
}; };
// RoundTripTestCookieStore is un-synchronized and re-synchronized after all
// cookie operations. This means all system cookies are converted to Chrome
// cookies and converted back.
// The purpose of this class is to test that cookie conversions do not break the
// cookie store.
class RoundTripTestCookieStore : public net::CookieStore {
public:
RoundTripTestCookieStore()
: store_(new CookieStoreIOS(nullptr)),
dummy_store_(new CookieStoreIOS(nullptr)) {
store_->SetSynchronizedWithSystemStore(true);
}
~RoundTripTestCookieStore() override {
store_->SetSynchronizedWithSystemStore(false);
}
// Inherited CookieStore methods.
void SetCookieWithOptionsAsync(const GURL& url,
const std::string& cookie_line,
const net::CookieOptions& options,
const SetCookiesCallback& callback) override {
RoundTrip();
store_->SetCookieWithOptionsAsync(url, cookie_line, options, callback);
}
void SetCookieWithDetailsAsync(const GURL& url,
const std::string& name,
const std::string& value,
const std::string& domain,
const std::string& path,
base::Time creation_time,
base::Time expiration_time,
base::Time last_access_time,
bool secure,
bool http_only,
CookieSameSite same_site,
bool enforce_strict_secure,
CookiePriority priority,
const SetCookiesCallback& callback) override {
RoundTrip();
store_->SetCookieWithDetailsAsync(
url, name, value, domain, path, creation_time, expiration_time,
last_access_time, secure, http_only, same_site, enforce_strict_secure,
priority, callback);
}
void GetCookiesWithOptionsAsync(const GURL& url,
const net::CookieOptions& options,
const GetCookiesCallback& callback) override {
RoundTrip();
store_->GetCookiesWithOptionsAsync(url, options, callback);
}
void GetCookieListWithOptionsAsync(
const GURL& url,
const net::CookieOptions& options,
const GetCookieListCallback& callback) override {
RoundTrip();
store_->GetCookieListWithOptionsAsync(url, options, callback);
}
void GetAllCookiesAsync(const GetCookieListCallback& callback) override {
RoundTrip();
store_->GetAllCookiesAsync(callback);
}
void DeleteCookieAsync(const GURL& url,
const std::string& cookie_name,
const base::Closure& callback) override {
RoundTrip();
store_->DeleteCookieAsync(url, cookie_name, callback);
}
void DeleteCanonicalCookieAsync(
const CanonicalCookie& cookie,
const DeleteCallback& callback) override {
RoundTrip();
store_->DeleteCanonicalCookieAsync(cookie, callback);
}
void DeleteAllCreatedBetweenAsync(const base::Time& delete_begin,
const base::Time& delete_end,
const DeleteCallback& callback) override {
RoundTrip();
store_->DeleteAllCreatedBetweenAsync(delete_begin, delete_end, callback);
}
void DeleteAllCreatedBetweenWithPredicateAsync(
const base::Time& delete_begin,
const base::Time& delete_end,
const CookiePredicate& predicate,
const DeleteCallback& callback) override {
RoundTrip();
store_->DeleteAllCreatedBetweenWithPredicateAsync(delete_begin, delete_end,
predicate, callback);
}
void DeleteSessionCookiesAsync(const DeleteCallback& callback) override {
RoundTrip();
store_->DeleteSessionCookiesAsync(callback);
}
void FlushStore(const base::Closure& callback) override {
RoundTrip();
store_->FlushStore(callback);
}
std::unique_ptr<CookieStore::CookieChangedSubscription> AddCallbackForCookie(
const GURL& url,
const std::string& name,
const CookieChangedCallback& callback) override {
return std::unique_ptr<CookieStore::CookieChangedSubscription>();
}
bool IsEphemeral() override {
return store_->IsEphemeral();
}
private:
void RoundTrip() {
store_->SetSynchronizedWithSystemStore(false);
dummy_store_->SetSynchronizedWithSystemStore(true);
// Check that the system store is empty, because it is synchronized with
// |dummy_store_| which is empty.
NSHTTPCookieStorage* store = [NSHTTPCookieStorage sharedHTTPCookieStorage];
EXPECT_EQ(0u, [[store cookies] count]);
dummy_store_->SetSynchronizedWithSystemStore(false);
store_->SetSynchronizedWithSystemStore(true);
}
std::unique_ptr<CookieStoreIOS> store_;
// |dummy_store_| is not directly used, but is needed to make |store_|
// inactive.
std::unique_ptr<CookieStoreIOS> dummy_store_;
};
struct RoundTripTestCookieStoreTraits {
static std::unique_ptr<net::CookieStore> Create() {
ClearCookies();
return base::MakeUnique<RoundTripTestCookieStore>();
}
static const bool is_cookie_monster = false;
static const bool supports_http_only = false;
static const bool supports_non_dotted_domains = false;
static const bool preserves_trailing_dots = false;
static const bool filters_schemes = false;
static const bool has_path_prefix_bug = true;
static const int creation_time_granularity_in_ms = 1000;
static const int enforces_prefixes = true;
static const bool enforce_strict_secure = false;
};
} // namespace net } // namespace net
namespace net { namespace net {
...@@ -235,10 +81,6 @@ INSTANTIATE_TYPED_TEST_CASE_P(InactiveCookieStoreIOS, ...@@ -235,10 +81,6 @@ INSTANTIATE_TYPED_TEST_CASE_P(InactiveCookieStoreIOS,
CookieStoreTest, CookieStoreTest,
InactiveCookieStoreIOSTestTraits); InactiveCookieStoreIOSTestTraits);
INSTANTIATE_TYPED_TEST_CASE_P(RoundTripTestCookieStore,
CookieStoreTest,
RoundTripTestCookieStoreTraits);
} // namespace net } // namespace net
namespace { namespace {
...@@ -583,81 +425,6 @@ TEST_F(CookieStoreIOSWithBackend, Synchronizing) { ...@@ -583,81 +425,6 @@ TEST_F(CookieStoreIOSWithBackend, Synchronizing) {
store_->SetSynchronizedWithSystemStore(false); store_->SetSynchronizedWithSystemStore(false);
} }
// Tests that Synchronization can be "aborted" (i.e. the cookie store is
// unsynchronized while synchronization is in progress).
TEST_F(CookieStoreIOSWithBackend, SyncThenUnsync) {
ClearCookies();
std::unique_ptr<CookieStoreIOS> dummy_store(new CookieStoreIOS(nullptr));
// Switch back and forth before synchronization can complete.
store_->SetSynchronizedWithSystemStore(true);
store_->SetSynchronizedWithSystemStore(false);
dummy_store->SetSynchronizedWithSystemStore(true);
backend_->RunLoadedCallback();
// No cookie leak in the system store.
NSHTTPCookieStorage* store = [NSHTTPCookieStorage sharedHTTPCookieStorage];
EXPECT_EQ(0u, [[store cookies] count]);
// No cookie lost.
GetCookieCallback callback;
GetCookies(base::Bind(&GetCookieCallback::Run, base::Unretained(&callback)));
EXPECT_TRUE(callback.did_run());
EXPECT_EQ("a=b", callback.cookie_line());
dummy_store->SetSynchronizedWithSystemStore(false);
}
// Tests that Synchronization can be "aborted" while there are pending tasks
// (i.e. the cookie store is unsynchronized while synchronization is in progress
// and there are pending tasks).
TEST_F(CookieStoreIOSWithBackend, SyncThenUnsyncWithPendingTasks) {
ClearCookies();
std::unique_ptr<CookieStoreIOS> dummy_store(new CookieStoreIOS(nullptr));
// Start synchornization.
store_->SetSynchronizedWithSystemStore(true);
// Create a pending task while synchronization is in progress.
GetCookieCallback callback;
GetCookies(base::Bind(&GetCookieCallback::Run, base::Unretained(&callback)));
// Cancel the synchronization.
store_->SetSynchronizedWithSystemStore(false);
dummy_store->SetSynchronizedWithSystemStore(true);
// Synchronization completes after being cancelled.
backend_->RunLoadedCallback();
// The task is not lost.
EXPECT_TRUE(callback.did_run());
EXPECT_EQ("a=b", callback.cookie_line());
dummy_store->SetSynchronizedWithSystemStore(false);
}
TEST_F(CookieStoreIOSWithBackend, UnSynchronizeBeforeLoadComplete) {
ClearCookies();
// Switch back and forth before synchronization can complete.
store_->SetSynchronizedWithSystemStore(true);
store_->SetSynchronizedWithSystemStore(false);
backend_->RunLoadedCallback();
// No cookie lost.
GetCookieCallback callback;
GetCookies(base::Bind(&GetCookieCallback::Run, base::Unretained(&callback)));
EXPECT_TRUE(callback.did_run());
EXPECT_EQ("a=b", callback.cookie_line());
}
TEST_F(CookieStoreIOSWithBackend, UnSynchronize) {
ClearCookies();
store_->SetSynchronizedWithSystemStore(true);
backend_->RunLoadedCallback();
store_->SetSynchronizedWithSystemStore(false);
// No cookie lost.
GetCookieCallback callback;
GetCookies(base::Bind(&GetCookieCallback::Run, base::Unretained(&callback)));
EXPECT_TRUE(callback.did_run());
EXPECT_EQ("a=b", callback.cookie_line());
}
TEST_F(CookieStoreIOSWithBackend, FlushOnUnSynchronize) {
store_->SetSynchronizedWithSystemStore(true);
EXPECT_FALSE(backend_->flushed());
store_->SetSynchronizedWithSystemStore(false);
EXPECT_TRUE(backend_->flushed());
}
TEST_F(CookieStoreIOSWithBackend, FlushOnCookieChanged) { TEST_F(CookieStoreIOSWithBackend, FlushOnCookieChanged) {
store_->SetSynchronizedWithSystemStore(true); store_->SetSynchronizedWithSystemStore(true);
store_->set_flush_delay_for_testing(base::TimeDelta()); store_->set_flush_delay_for_testing(base::TimeDelta());
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment