Skip to content

Commit

Permalink
Cookie Store: Rewrite CookieMonster's notification dispatch.
Browse files Browse the repository at this point in the history
The old implementation was designed to support a small, static number of
observers. The new implementation supports the churn (adding/removing
observers) that will come from exposing cookie change observation to Web
APIs.

The new implementation also adds AddCallbackForUrl(), which is in
between AddCallbackForAllChanges (no filter) and AddCallbackForCookie
(very specific filter). The new method will be used by the Async Cookies
API implementation.

Bug: 729800
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Idc58252ede82be82b34c530607e811d5392801cd
Reviewed-on: https://chromium-review.googlesource.com/934304
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540460}
  • Loading branch information
pwnall authored and Commit Bot committed Mar 2, 2018
1 parent bbbfd60 commit 2a18916
Show file tree
Hide file tree
Showing 17 changed files with 1,342 additions and 142 deletions.
15 changes: 12 additions & 3 deletions android_webview/browser/net/aw_cookie_change_dispatcher_wrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class AwCookieChangeSubscription : public net::CookieChangeSubscription {

// Wraps a subscription to cookie change notifications for the global
// CookieStore for a consumer that lives on another thread. Handles passing
// messages between thread, and destroys itself when the consumer unsubscribes.
// messages between threads, and destroys itself when the consumer unsubscribes.
// Must be created on the consumer's thread. Each instance only supports a
// single subscription.
class SubscriptionWrapper {
Expand Down Expand Up @@ -145,11 +145,20 @@ AwCookieChangeDispatcherWrapper::AddCallbackForCookie(
return subscription->Subscribe(url, name, std::move(callback));
}

std::unique_ptr<net::CookieChangeSubscription>
AwCookieChangeDispatcherWrapper::AddCallbackForUrl(
const GURL& url,
net::CookieChangeCallback callback) {
// Implement when needed by Android Webview consumers.
NOTIMPLEMENTED();
return nullptr;
}

std::unique_ptr<net::CookieChangeSubscription>
AwCookieChangeDispatcherWrapper::AddCallbackForAllChanges(
net::CookieChangeCallback callback) {
// TODO(rdsmith): Implement when needed by Android Webview consumer.
CHECK(false);
// Implement when needed by Android Webview consumers.
NOTIMPLEMENTED();
return nullptr;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ class AwCookieChangeDispatcherWrapper : public net::CookieChangeDispatcher {
const GURL& url,
const std::string& name,
net::CookieChangeCallback callback) override WARN_UNUSED_RESULT;
std::unique_ptr<net::CookieChangeSubscription> AddCallbackForUrl(
const GURL& url,
net::CookieChangeCallback callback) override WARN_UNUSED_RESULT;
std::unique_ptr<net::CookieChangeSubscription> AddCallbackForAllChanges(
net::CookieChangeCallback callback) override WARN_UNUSED_RESULT;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ struct AwCookieStoreWrapperTestTraits {
static const bool has_path_prefix_bug = false;
static const bool forbids_setting_empty_name = false;
static const bool supports_global_cookie_tracking = false;
static const bool supports_url_cookie_tracking = false;
static const bool supports_named_cookie_tracking = true;
static const bool supports_multiple_tracking_callbacks = false;
static const int creation_time_granularity_in_ms = 0;
Expand All @@ -68,6 +69,9 @@ INSTANTIATE_TYPED_TEST_CASE_P(AwCookieStoreWrapper,
INSTANTIATE_TYPED_TEST_CASE_P(AwCookieStoreWrapper,
CookieStoreChangeGlobalTest,
android_webview::AwCookieStoreWrapperTestTraits);
INSTANTIATE_TYPED_TEST_CASE_P(AwCookieStoreWrapper,
CookieStoreChangeUrlTest,
android_webview::AwCookieStoreWrapperTestTraits);
INSTANTIATE_TYPED_TEST_CASE_P(AwCookieStoreWrapper,
CookieStoreChangeNamedTest,
android_webview::AwCookieStoreWrapperTestTraits);
Expand Down
7 changes: 7 additions & 0 deletions headless/public/util/testing/generic_url_request_mocks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ MockCookieChangeDispatcher::AddCallbackForCookie(
return nullptr;
}
std::unique_ptr<net::CookieChangeSubscription>
MockCookieChangeDispatcher::AddCallbackForUrl(
const GURL& url,
net::CookieChangeCallback callback) {
CHECK(false);
return nullptr;
}
std::unique_ptr<net::CookieChangeSubscription>
MockCookieChangeDispatcher::AddCallbackForAllChanges(
net::CookieChangeCallback callback) {
CHECK(false);
Expand Down
3 changes: 3 additions & 0 deletions headless/public/util/testing/generic_url_request_mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ class MockCookieChangeDispatcher : public net::CookieChangeDispatcher {
const GURL& url,
const std::string& name,
net::CookieChangeCallback callback) override WARN_UNUSED_RESULT;
std::unique_ptr<net::CookieChangeSubscription> AddCallbackForUrl(
const GURL& url,
net::CookieChangeCallback callback) override WARN_UNUSED_RESULT;
std::unique_ptr<net::CookieChangeSubscription> AddCallbackForAllChanges(
net::CookieChangeCallback callback) override WARN_UNUSED_RESULT;

Expand Down
3 changes: 3 additions & 0 deletions ios/net/cookies/cookie_store_ios.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@ class CookieStoreIOS : public net::CookieStore,
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;

Expand Down
9 changes: 9 additions & 0 deletions ios/net/cookies/cookie_store_ios.mm
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,15 @@ bool HasExplicitDomain(const std::string& cookie_line) {
return cookie_store_->AddCallbackForCookie(gurl, name, std::move(callback));
}

std::unique_ptr<CookieChangeSubscription>
CookieStoreIOS::CookieChangeDispatcherIOS::AddCallbackForUrl(
const GURL& gurl,
CookieChangeCallback callback) {
// Implement when needed by iOS consumers.
NOTIMPLEMENTED();
return nullptr;
}

std::unique_ptr<CookieChangeSubscription>
CookieStoreIOS::CookieChangeDispatcherIOS::AddCallbackForAllChanges(
CookieChangeCallback callback) {
Expand Down
4 changes: 4 additions & 0 deletions ios/net/cookies/cookie_store_ios_persistent_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
static const bool supports_global_cookie_tracking = false;
// TODO(crbug.com/813931): Fix the bugs uncovered by these tests.
static const bool supports_named_cookie_tracking = false;
static const bool supports_url_cookie_tracking = false;
static const bool supports_multiple_tracking_callbacks = false;
static const int creation_time_granularity_in_ms = 0;
static const int enforces_prefixes = true;
Expand All @@ -57,6 +58,9 @@
INSTANTIATE_TYPED_TEST_CASE_P(InactiveCookieStoreIOS,
CookieStoreChangeGlobalTest,
InactiveCookieStoreIOSTestTraits);
INSTANTIATE_TYPED_TEST_CASE_P(InactiveCookieStoreIOS,
CookieStoreChangeUrlTest,
InactiveCookieStoreIOSTestTraits);
INSTANTIATE_TYPED_TEST_CASE_P(InactiveCookieStoreIOS,
CookieStoreChangeNamedTest,
InactiveCookieStoreIOSTestTraits);
Expand Down
4 changes: 4 additions & 0 deletions ios/net/cookies/cookie_store_ios_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
static const bool forbids_setting_empty_name = true;
static const bool supports_global_cookie_tracking = false;
// TODO(crbug.com/813931): Fix the bugs uncovered by these tests.
static const bool supports_url_cookie_tracking = false;
static const bool supports_named_cookie_tracking = false;
static const bool supports_multiple_tracking_callbacks = false;
static const int creation_time_granularity_in_ms = 1000;
Expand All @@ -70,6 +71,9 @@
INSTANTIATE_TYPED_TEST_CASE_P(CookieStoreIOS,
CookieStoreChangeGlobalTest,
CookieStoreIOSTestTraits);
INSTANTIATE_TYPED_TEST_CASE_P(CookieStoreIOS,
CookieStoreChangeUrlTest,
CookieStoreIOSTestTraits);
INSTANTIATE_TYPED_TEST_CASE_P(CookieStoreIOS,
CookieStoreChangeNamedTest,
CookieStoreIOSTestTraits);
Expand Down
5 changes: 5 additions & 0 deletions net/cookies/cookie_change_dispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,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.
Expand Down
Loading

0 comments on commit 2a18916

Please sign in to comment.