diff options
author | willchan@chromium.org <willchan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-06-29 20:51:01 +0000 |
---|---|---|
committer | willchan@chromium.org <willchan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-06-29 20:51:01 +0000 |
commit | 9eaa18ed61a3837e1046045476a3c7f4e8eb6a5f (patch) | |
tree | 814e98bb2f18d5017b047c241cdf58e47ea0d050 /chrome | |
parent | 564e170373c0bb521d735547dc7f443d19b2fac5 (diff) | |
download | chromium_src-9eaa18ed61a3837e1046045476a3c7f4e8eb6a5f.zip chromium_src-9eaa18ed61a3837e1046045476a3c7f4e8eb6a5f.tar.gz chromium_src-9eaa18ed61a3837e1046045476a3c7f4e8eb6a5f.tar.bz2 |
Reland r50296 which removes some uses of CookieMonster on the UI thread.
Removes the addition of NonThreadSafe and DCHECKs to CookieMonster.
BUG=44083
Review URL: http://codereview.chromium.org/2845031
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@51164 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
-rw-r--r-- | chrome/browser/automation/automation_provider.cc | 125 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_cookies_api.cc | 252 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_cookies_api.h | 79 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_cookies_helpers.cc | 5 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_cookies_helpers.h | 4 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_data_deleter.cc | 26 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_data_deleter.h | 4 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_function.h | 7 | ||||
-rw-r--r-- | chrome/browser/net/cookie_policy_browsertest.cc | 62 |
9 files changed, 436 insertions, 128 deletions
diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc index 07c37d3..25091a2 100644 --- a/chrome/browser/automation/automation_provider.cc +++ b/chrome/browser/automation/automation_provider.cc @@ -20,9 +20,11 @@ #include "base/process_util.h" #include "base/stl_util-inl.h" #include "base/string_util.h" +#include "base/task.h" #include "base/thread.h" #include "base/utf_string_conversions.h" #include "base/values.h" +#include "base/waitable_event.h" #include "chrome/app/chrome_dll_resource.h" #include "chrome/app/chrome_version_info.h" #include "chrome/browser/app_modal_dialog.h" @@ -1119,6 +1121,112 @@ Browser* AutomationProvider::FindAndActivateTab( return browser; } +namespace { + +class GetCookiesTask : public Task { + public: + GetCookiesTask(const GURL& url, + URLRequestContextGetter* context_getter, + base::WaitableEvent* event, + std::string* cookies) + : url_(url), + context_getter_(context_getter), + event_(event), + cookies_(cookies) {} + + virtual void Run() { + *cookies_ = context_getter_->GetCookieStore()->GetCookies(url_); + event_->Signal(); + } + + private: + const GURL& url_; + URLRequestContextGetter* const context_getter_; + base::WaitableEvent* const event_; + std::string* const cookies_; + + DISALLOW_COPY_AND_ASSIGN(GetCookiesTask); +}; + +std::string GetCookiesForURL( + const GURL& url, + URLRequestContextGetter* context_getter) { + std::string cookies; + base::WaitableEvent event(true /* manual reset */, + false /* not initially signaled */); + CHECK(ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + new GetCookiesTask(url, context_getter, &event, &cookies))); + event.Wait(); + return cookies; +} + +class SetCookieTask : public Task { + public: + SetCookieTask(const GURL& url, + const std::string& value, + URLRequestContextGetter* context_getter, + base::WaitableEvent* event, + bool* rv) + : url_(url), + value_(value), + context_getter_(context_getter), + event_(event), + rv_(rv) {} + + virtual void Run() { + *rv_ = context_getter_->GetCookieStore()->SetCookie(url_, value_); + event_->Signal(); + } + + private: + const GURL& url_; + const std::string& value_; + URLRequestContextGetter* const context_getter_; + base::WaitableEvent* const event_; + bool* const rv_; + + DISALLOW_COPY_AND_ASSIGN(SetCookieTask); +}; + +bool SetCookieForURL( + const GURL& url, + const std::string& value, + URLRequestContextGetter* context_getter) { + base::WaitableEvent event(true /* manual reset */, + false /* not initially signaled */); + bool rv = false; + CHECK(ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + new SetCookieTask(url, value, context_getter, &event, &rv))); + event.Wait(); + return rv; +} + +class DeleteCookieTask : public Task { + public: + DeleteCookieTask(const GURL& url, + const std::string& name, + const scoped_refptr<URLRequestContextGetter>& context_getter) + : url_(url), + name_(name), + context_getter_(context_getter) {} + + virtual void Run() { + net::CookieStore* cookie_store = context_getter_->GetCookieStore(); + cookie_store->DeleteCookie(url_, name_); + } + + private: + const GURL url_; + const std::string name_; + const scoped_refptr<URLRequestContextGetter> context_getter_; + + DISALLOW_COPY_AND_ASSIGN(DeleteCookieTask); +}; + +} // namespace + void AutomationProvider::GetCookies(const GURL& url, int handle, int* value_size, std::string* value) { @@ -1132,9 +1240,7 @@ void AutomationProvider::GetCookies(const GURL& url, int handle, if (!request_context.get()) request_context = tab->profile()->GetRequestContext(); - net::CookieStore* cookie_store = request_context->GetCookieStore(); - - *value = cookie_store->GetCookies(url); + *value = GetCookiesForURL(url, request_context.get()); *value_size = static_cast<int>(value->size()); } } @@ -1153,11 +1259,7 @@ void AutomationProvider::SetCookie(const GURL& url, if (!request_context.get()) request_context = tab->profile()->GetRequestContext(); - // Since we are running on the UI thread don't call GetURLRequestContext(). - scoped_refptr<net::CookieStore> cookie_store = - request_context->GetCookieStore(); - - if (cookie_store->SetCookie(url, value)) + if (SetCookieForURL(url, value, request_context.get())) *response_value = 1; } } @@ -1168,9 +1270,10 @@ void AutomationProvider::DeleteCookie(const GURL& url, *success = false; if (url.is_valid() && tab_tracker_->ContainsHandle(handle)) { NavigationController* tab = tab_tracker_->GetResource(handle); - net::CookieStore* cookie_store = - tab->profile()->GetRequestContext()->GetCookieStore(); - cookie_store->DeleteCookie(url, cookie_name); + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + new DeleteCookieTask(url, cookie_name, + tab->profile()->GetRequestContext())); *success = true; } } diff --git a/chrome/browser/extensions/extension_cookies_api.cc b/chrome/browser/extensions/extension_cookies_api.cc index 86cc0d8..c20d1eb 100644 --- a/chrome/browser/extensions/extension_cookies_api.cc +++ b/chrome/browser/extensions/extension_cookies_api.cc @@ -7,7 +7,9 @@ #include "chrome/browser/extensions/extension_cookies_api.h" #include "base/json/json_writer.h" +#include "base/task.h" #include "chrome/browser/browser_list.h" +#include "chrome/browser/chrome_thread.h" #include "chrome/browser/extensions/extension_cookies_api_constants.h" #include "chrome/browser/extensions/extension_cookies_helpers.h" #include "chrome/browser/extensions/extension_message_service.h" @@ -99,10 +101,10 @@ bool CookiesFunction::ParseUrl(const DictionaryValue* details, GURL* url, return true; } -bool CookiesFunction::ParseCookieStore(const DictionaryValue* details, - net::CookieStore** store, - std::string* store_id) { - DCHECK(details && (store || store_id)); +bool CookiesFunction::ParseStoreContext(const DictionaryValue* details, + URLRequestContextGetter** context, + std::string* store_id) { + DCHECK(details && (context || store_id)); Profile* store_profile = NULL; if (details->HasKey(keys::kStoreIdKey)) { // The store ID was explicitly specified in the details dictionary. @@ -130,14 +132,17 @@ bool CookiesFunction::ParseCookieStore(const DictionaryValue* details, store_profile = current_browser->profile(); } DCHECK(store_profile); - if (store) - *store = store_profile->GetRequestContext()->GetCookieStore(); + + if (context) + *context = store_profile->GetRequestContext(); if (store_id) - *store_id = - extension_cookies_helpers::GetStoreIdFromProfile(store_profile); + *store_id = extension_cookies_helpers::GetStoreIdFromProfile(store_profile); + return true; } +GetCookieFunction::GetCookieFunction() {} + bool GetCookieFunction::RunImpl() { // Return false if the arguments are malformed. DictionaryValue* details; @@ -145,62 +150,115 @@ bool GetCookieFunction::RunImpl() { DCHECK(details); // Read/validate input parameters. - GURL url; - if (!ParseUrl(details, &url, true)) + if (!ParseUrl(details, &url_, true)) return false; - std::string name; // Get the cookie name string or return false. - EXTENSION_FUNCTION_VALIDATE(details->GetString(keys::kNameKey, &name)); + EXTENSION_FUNCTION_VALIDATE(details->GetString(keys::kNameKey, &name_)); - net::CookieStore* cookie_store; - std::string store_id; - if (!ParseCookieStore(details, &cookie_store, &store_id)) + URLRequestContextGetter* store_context = NULL; + if (!ParseStoreContext(details, &store_context, &store_id_)) return false; - DCHECK(cookie_store && !store_id.empty()); - net::CookieMonster::CookieList cookie_list = - extension_cookies_helpers::GetCookieListFromStore(cookie_store, url); + DCHECK(store_context && !store_id_.empty()); + store_context_ = store_context; + + bool rv = ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod(this, &GetCookieFunction::GetCookieOnIOThread)); + DCHECK(rv); + + // Will finish asynchronously. + return true; +} + +void GetCookieFunction::GetCookieOnIOThread() { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); + net::CookieStore* cookie_store = store_context_->GetCookieStore(); + cookie_list_ = + extension_cookies_helpers::GetCookieListFromStore(cookie_store, url_); + + bool rv = ChromeThread::PostTask( + ChromeThread::UI, FROM_HERE, + NewRunnableMethod(this, &GetCookieFunction::RespondOnUIThread)); + DCHECK(rv); +} + +void GetCookieFunction::RespondOnUIThread() { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI)); + net::CookieMonster::CookieList::iterator it; - for (it = cookie_list.begin(); it != cookie_list.end(); ++it) { + for (it = cookie_list_.begin(); it != cookie_list_.end(); ++it) { // Return the first matching cookie. Relies on the fact that the // CookieMonster retrieves them in reverse domain-length order. const net::CookieMonster::CanonicalCookie& cookie = it->second; - if (cookie.Name() == name) { + if (cookie.Name() == name_) { result_.reset( - extension_cookies_helpers::CreateCookieValue(*it, store_id)); - return true; + extension_cookies_helpers::CreateCookieValue(*it, store_id_)); + break; } } + // The cookie doesn't exist; return null. - result_.reset(Value::CreateNullValue()); - return true; + if (it == cookie_list_.end()) + result_.reset(Value::CreateNullValue()); + + SendResponse(true); } +GetAllCookiesFunction::GetAllCookiesFunction() {} + bool GetAllCookiesFunction::RunImpl() { // Return false if the arguments are malformed. - DictionaryValue* details; - EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(0, &details)); - DCHECK(details); + EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(0, &details_)); + DCHECK(details_); // Read/validate input parameters. - GURL url; - if (details->HasKey(keys::kUrlKey) && !ParseUrl(details, &url, false)) + if (details_->HasKey(keys::kUrlKey) && !ParseUrl(details_, &url_, false)) return false; - net::CookieStore* cookie_store; - std::string store_id; - if (!ParseCookieStore(details, &cookie_store, &store_id)) + URLRequestContextGetter* store_context = NULL; + if (!ParseStoreContext(details_, &store_context, &store_id_)) return false; - DCHECK(cookie_store); + DCHECK(store_context); + store_context_ = store_context; - ListValue* matching_list = new ListValue(); - extension_cookies_helpers::AppendMatchingCookiesToList( - cookie_store, store_id, url, details, GetExtension(), matching_list); - result_.reset(matching_list); + bool rv = ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod(this, &GetAllCookiesFunction::GetAllCookiesOnIOThread)); + DCHECK(rv); + + // Will finish asynchronously. return true; } +void GetAllCookiesFunction::GetAllCookiesOnIOThread() { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); + net::CookieStore* cookie_store = store_context_->GetCookieStore(); + cookie_list_ = + extension_cookies_helpers::GetCookieListFromStore(cookie_store, url_); + + bool rv = ChromeThread::PostTask( + ChromeThread::UI, FROM_HERE, + NewRunnableMethod(this, &GetAllCookiesFunction::RespondOnUIThread)); + DCHECK(rv); +} + +void GetAllCookiesFunction::RespondOnUIThread() { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI)); + + const Extension* extension = GetExtension(); + if (extension) { + ListValue* matching_list = new ListValue(); + extension_cookies_helpers::AppendMatchingCookiesToList( + cookie_list_, store_id_, url_, details_, GetExtension(), matching_list); + result_.reset(matching_list); + } + SendResponse(true); +} + +SetCookieFunction::SetCookieFunction() : secure_(false), http_only_(false) {} + bool SetCookieFunction::RunImpl() { // Return false if the arguments are malformed. DictionaryValue* details; @@ -208,36 +266,26 @@ bool SetCookieFunction::RunImpl() { DCHECK(details); // Read/validate input parameters. - GURL url; - if (!ParseUrl(details, &url, true)) + if (!ParseUrl(details, &url_, true)) return false; // The macros below return false if argument types are not as expected. - std::string name; - if (details->HasKey(keys::kNameKey)) { - EXTENSION_FUNCTION_VALIDATE(details->GetString(keys::kNameKey, &name)); - } - std::string value; - if (details->HasKey(keys::kValueKey)) { - EXTENSION_FUNCTION_VALIDATE(details->GetString(keys::kValueKey, &value)); - } - std::string domain; - if (details->HasKey(keys::kDomainKey)) { - EXTENSION_FUNCTION_VALIDATE(details->GetString(keys::kDomainKey, &domain)); - } - std::string path; - if (details->HasKey(keys::kPathKey)) { - EXTENSION_FUNCTION_VALIDATE(details->GetString(keys::kPathKey, &path)); - } - bool secure = false; + if (details->HasKey(keys::kNameKey)) + EXTENSION_FUNCTION_VALIDATE(details->GetString(keys::kNameKey, &name_)); + if (details->HasKey(keys::kValueKey)) + EXTENSION_FUNCTION_VALIDATE(details->GetString(keys::kValueKey, &value_)); + if (details->HasKey(keys::kDomainKey)) + EXTENSION_FUNCTION_VALIDATE(details->GetString(keys::kDomainKey, &domain_)); + if (details->HasKey(keys::kPathKey)) + EXTENSION_FUNCTION_VALIDATE(details->GetString(keys::kPathKey, &path_)); + if (details->HasKey(keys::kSecureKey)) { - EXTENSION_FUNCTION_VALIDATE(details->GetBoolean(keys::kSecureKey, &secure)); + EXTENSION_FUNCTION_VALIDATE( + details->GetBoolean(keys::kSecureKey, &secure_)); } - bool http_only = false; if (details->HasKey(keys::kHttpOnlyKey)) { EXTENSION_FUNCTION_VALIDATE( - details->GetBoolean(keys::kHttpOnlyKey, &http_only)); + details->GetBoolean(keys::kHttpOnlyKey, &http_only_)); } - base::Time expiration_time; if (details->HasKey(keys::kExpirationDateKey)) { Value* expiration_date_value; EXTENSION_FUNCTION_VALIDATE(details->Get(keys::kExpirationDateKey, @@ -252,24 +300,73 @@ bool SetCookieFunction::RunImpl() { EXTENSION_FUNCTION_VALIDATE( expiration_date_value->GetAsReal(&expiration_date)); } - expiration_time = base::Time::FromDoubleT(expiration_date); + expiration_time_ = base::Time::FromDoubleT(expiration_date); } - net::CookieStore* cookie_store; - if (!ParseCookieStore(details, &cookie_store, NULL)) + URLRequestContextGetter* store_context = NULL; + if (!ParseStoreContext(details, &store_context, NULL)) return false; - DCHECK(cookie_store); + DCHECK(store_context); + store_context_ = store_context; + + bool rv = ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod(this, &SetCookieFunction::SetCookieOnIOThread)); + DCHECK(rv); - if (!cookie_store->GetCookieMonster()->SetCookieWithDetails( - url, name, value, domain, path, expiration_time, secure, - http_only)) { + // Will finish asynchronously. + return true; +} + +void SetCookieFunction::SetCookieOnIOThread() { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); + net::CookieMonster* cookie_monster = + store_context_->GetCookieStore()->GetCookieMonster(); + success_ = cookie_monster->SetCookieWithDetails( + url_, name_, value_, domain_, path_, expiration_time_, + secure_, http_only_); + + bool rv = ChromeThread::PostTask( + ChromeThread::UI, FROM_HERE, + NewRunnableMethod(this, &SetCookieFunction::RespondOnUIThread)); + DCHECK(rv); +} + +void SetCookieFunction::RespondOnUIThread() { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI)); + if (!success_) { error_ = ExtensionErrorUtils::FormatErrorMessage( - keys::kCookieSetFailedError, name); - return false; + keys::kCookieSetFailedError, name_); } - return true; + SendResponse(success_); } +namespace { + +class RemoveCookieTask : public Task { + public: + RemoveCookieTask(const GURL& url, + const std::string& name, + const scoped_refptr<URLRequestContextGetter>& context_getter) + : url_(url), + name_(name), + context_getter_(context_getter) {} + + virtual void Run() { + net::CookieStore* cookie_store = context_getter_->GetCookieStore(); + cookie_store->DeleteCookie(url_, name_); + } + + private: + const GURL url_; + const std::string name_; + const scoped_refptr<URLRequestContextGetter> context_getter_; + + DISALLOW_COPY_AND_ASSIGN(RemoveCookieTask); +}; + +} // namespace + bool RemoveCookieFunction::RunImpl() { // Return false if the arguments are malformed. DictionaryValue* details; @@ -285,12 +382,19 @@ bool RemoveCookieFunction::RunImpl() { // Get the cookie name string or return false. EXTENSION_FUNCTION_VALIDATE(details->GetString(keys::kNameKey, &name)); - net::CookieStore* cookie_store; - if (!ParseCookieStore(details, &cookie_store, NULL)) + URLRequestContextGetter* store_context = NULL; + if (!ParseStoreContext(details, &store_context, NULL)) return false; - DCHECK(cookie_store); + DCHECK(store_context); + + // We don't bother to synchronously wait for the result here, because + // CookieMonster is only ever accessed on the IO thread, so any other accesses + // should happen after this. + bool rv = ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + new RemoveCookieTask(url, name, store_context)); + DCHECK(rv); - cookie_store->DeleteCookie(url, name); return true; } diff --git a/chrome/browser/extensions/extension_cookies_api.h b/chrome/browser/extensions/extension_cookies_api.h index c230b01..a250524 100644 --- a/chrome/browser/extensions/extension_cookies_api.h +++ b/chrome/browser/extensions/extension_cookies_api.h @@ -10,14 +10,16 @@ #include <string> +#include "base/ref_counted.h" #include "base/singleton.h" +#include "base/time.h" #include "chrome/browser/extensions/extension_function.h" #include "chrome/browser/net/chrome_cookie_notification_details.h" #include "chrome/common/notification_registrar.h" +#include "googleurl/src/gurl.h" +#include "net/base/cookie_monster.h" -namespace net { -class CookieStore; -} // namespace net +class URLRequestContextGetter; // Observes CookieMonster notifications and routes them as events to the // extension system. @@ -58,10 +60,12 @@ class ExtensionCookiesEventRouter : public NotificationObserver { // Serves as a base class for all cookies API functions, and defines some // common functionality for parsing cookies API function arguments. -// Note that all of the functions in this file derive from ExtensionFunction, -// and are not threadsafe. They modify the result_ member variable directly; -// see chrome/browser/extensions/extension_function.h for more information. -class CookiesFunction : public SyncExtensionFunction { +// Note that all of the functions in this file derive from +// AsyncExtensionFunction, and are not threadsafe, so they should not be +// concurrently accessed from multiple threads. They modify |result_| and other +// member variables directly. +// See chrome/browser/extensions/extension_function.h for more information. +class CookiesFunction : public AsyncExtensionFunction { protected: // Looks for a 'url' value in the given details dictionary and constructs a // GURL from it. Returns false and assigns the internal error_ value if the @@ -72,41 +76,84 @@ class CookiesFunction : public SyncExtensionFunction { bool check_host_permissions); // Checks the given details dictionary for a 'storeId' value, and retrieves - // the cookie store and the store ID associated with it. If the 'storeId' - // value isn't found in the dictionary, the current execution context's - // cookie store is retrieved. Returns false on error and assigns the - // internal error_ value if that occurs. + // the cookie store context and the store ID associated with it. If the + // 'storeId' value isn't found in the dictionary, the current execution + // context's cookie store context is retrieved. Returns false on error and + // assigns the internal error_ value if that occurs. // At least one of the output parameters store and store_id should be - // non-null. - bool ParseCookieStore(const DictionaryValue* details, - net::CookieStore** store, std::string* store_id); + // non-NULL. + bool ParseStoreContext(const DictionaryValue* details, + URLRequestContextGetter** context, + std::string* store_id); }; // Implements the experimental.cookies.get() extension function. class GetCookieFunction : public CookiesFunction { public: + GetCookieFunction(); virtual bool RunImpl(); DECLARE_EXTENSION_FUNCTION_NAME("experimental.cookies.get") + + private: + void GetCookieOnIOThread(); + void RespondOnUIThread(); + + std::string name_; + GURL url_; + std::string store_id_; + scoped_refptr<URLRequestContextGetter> store_context_; + net::CookieMonster::CookieList cookie_list_; }; // Implements the experimental.cookies.getAll() extension function. class GetAllCookiesFunction : public CookiesFunction { public: + GetAllCookiesFunction(); virtual bool RunImpl(); DECLARE_EXTENSION_FUNCTION_NAME("experimental.cookies.getAll") + + private: + void GetAllCookiesOnIOThread(); + void RespondOnUIThread(); + + DictionaryValue* details_; + GURL url_; + std::string store_id_; + scoped_refptr<URLRequestContextGetter> store_context_; + net::CookieMonster::CookieList cookie_list_; }; // Implements the experimental.cookies.set() extension function. class SetCookieFunction : public CookiesFunction { public: + SetCookieFunction(); virtual bool RunImpl(); DECLARE_EXTENSION_FUNCTION_NAME("experimental.cookies.set") + + private: + void SetCookieOnIOThread(); + void RespondOnUIThread(); + + GURL url_; + std::string name_; + std::string value_; + std::string domain_; + std::string path_; + bool secure_; + bool http_only_; + base::Time expiration_time_; + bool success_; + scoped_refptr<URLRequestContextGetter> store_context_; }; // Implements the experimental.cookies.remove() extension function. class RemoveCookieFunction : public CookiesFunction { public: virtual bool RunImpl(); + // RemoveCookieFunction is sync. + virtual void Run() { + SendResponse(RunImpl()); + } DECLARE_EXTENSION_FUNCTION_NAME("experimental.cookies.remove") }; @@ -114,6 +161,10 @@ class RemoveCookieFunction : public CookiesFunction { class GetAllCookieStoresFunction : public CookiesFunction { public: virtual bool RunImpl(); + // GetAllCookieStoresFunction is sync. + virtual void Run() { + SendResponse(RunImpl()); + } DECLARE_EXTENSION_FUNCTION_NAME("experimental.cookies.getAllCookieStores") }; diff --git a/chrome/browser/extensions/extension_cookies_helpers.cc b/chrome/browser/extensions/extension_cookies_helpers.cc index ed881e2..903e89e 100644 --- a/chrome/browser/extensions/extension_cookies_helpers.cc +++ b/chrome/browser/extensions/extension_cookies_helpers.cc @@ -94,12 +94,11 @@ GURL GetURLFromCookiePair( } void AppendMatchingCookiesToList( - net::CookieStore* cookie_store, const std::string& store_id, + const net::CookieMonster::CookieList& all_cookies, + const std::string& store_id, const GURL& url, const DictionaryValue* details, const Extension* extension, ListValue* match_list) { - net::CookieMonster::CookieList all_cookies = GetCookieListFromStore( - cookie_store, url); net::CookieMonster::CookieList::const_iterator it; for (it = all_cookies.begin(); it != all_cookies.end(); ++it) { // Ignore any cookie whose domain doesn't match the extension's diff --git a/chrome/browser/extensions/extension_cookies_helpers.h b/chrome/browser/extensions/extension_cookies_helpers.h index 889a8c08..77284c6 100644 --- a/chrome/browser/extensions/extension_cookies_helpers.h +++ b/chrome/browser/extensions/extension_cookies_helpers.h @@ -46,6 +46,7 @@ DictionaryValue* CreateCookieStoreValue(Profile* profile, // Retrieves all cookies from the given cookie store corresponding to the given // URL. If the URL is empty, all cookies in the cookie store are retrieved. +// This can only be called on the IO thread. net::CookieMonster::CookieList GetCookieListFromStore( net::CookieStore* cookie_store, const GURL& url); @@ -60,7 +61,8 @@ GURL GetURLFromCookiePair( // match list all the cookies that both match the given URL and cookie details // and are allowed by extension host permissions. void AppendMatchingCookiesToList( - net::CookieStore* cookie_store, const std::string& store_id, + const net::CookieMonster::CookieList& all_cookies, + const std::string& store_id, const GURL& url, const DictionaryValue* details, const Extension* extension, ListValue* match_list); diff --git a/chrome/browser/extensions/extension_data_deleter.cc b/chrome/browser/extensions/extension_data_deleter.cc index 98cf433..1a9f84e 100644 --- a/chrome/browser/extensions/extension_data_deleter.cc +++ b/chrome/browser/extensions/extension_data_deleter.cc @@ -23,18 +23,28 @@ ExtensionDataDeleter::ExtensionDataDeleter(Profile* profile, void ExtensionDataDeleter::StartDeleting() { DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI)); + + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod(this, &ExtensionDataDeleter::DeleteCookiesOnIOThread)); + + ChromeThread::PostTask( + ChromeThread::WEBKIT, FROM_HERE, + NewRunnableMethod( + this, &ExtensionDataDeleter::DeleteLocalStorageOnWebkitThread)); + + ChromeThread::PostTask( + ChromeThread::FILE, FROM_HERE, + NewRunnableMethod( + this, &ExtensionDataDeleter::DeleteDatabaseOnFileThread)); +} + +void ExtensionDataDeleter::DeleteCookiesOnIOThread() { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); net::CookieMonster* cookie_monster = extension_request_context_->GetCookieStore()->GetCookieMonster(); if (cookie_monster) cookie_monster->DeleteAllForURL(extension_url_, true); - - ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, - NewRunnableMethod(this, - &ExtensionDataDeleter::DeleteLocalStorageOnWebkitThread)); - - ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE, - NewRunnableMethod(this, - &ExtensionDataDeleter::DeleteDatabaseOnFileThread)); } void ExtensionDataDeleter::DeleteDatabaseOnFileThread() { diff --git a/chrome/browser/extensions/extension_data_deleter.h b/chrome/browser/extensions/extension_data_deleter.h index d7f13327..4b90509 100644 --- a/chrome/browser/extensions/extension_data_deleter.h +++ b/chrome/browser/extensions/extension_data_deleter.h @@ -33,6 +33,10 @@ class ExtensionDataDeleter void StartDeleting(); private: + // Deletes the cookies for the extension. May only be called on the io + // thread. + void DeleteCookiesOnIOThread(); + // Deletes the database for the extension. May only be called on the file // thread. void DeleteDatabaseOnFileThread(); diff --git a/chrome/browser/extensions/extension_function.h b/chrome/browser/extensions/extension_function.h index d0e1d74..27bd37c 100644 --- a/chrome/browser/extensions/extension_function.h +++ b/chrome/browser/extensions/extension_function.h @@ -35,10 +35,7 @@ class QuotaLimitHeuristic; // Abstract base class for extension functions the ExtensionFunctionDispatcher // knows how to dispatch to. -// -// TODO(aa): This will have to become reference counted when we introduce -// APIs that live beyond a single stack frame. -class ExtensionFunction : public base::RefCounted<ExtensionFunction> { +class ExtensionFunction : public base::RefCountedThreadSafe<ExtensionFunction> { public: ExtensionFunction() : request_id_(-1), name_(""), has_callback_(false) {} @@ -98,7 +95,7 @@ class ExtensionFunction : public base::RefCounted<ExtensionFunction> { virtual void Run() = 0; protected: - friend class base::RefCounted<ExtensionFunction>; + friend class base::RefCountedThreadSafe<ExtensionFunction>; virtual ~ExtensionFunction() {} diff --git a/chrome/browser/net/cookie_policy_browsertest.cc b/chrome/browser/net/cookie_policy_browsertest.cc index 7a33ce9..e43cd14 100644 --- a/chrome/browser/net/cookie_policy_browsertest.cc +++ b/chrome/browser/net/cookie_policy_browsertest.cc @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/task.h" +#include "base/waitable_event.h" #include "chrome/browser/browser.h" #include "chrome/browser/host_content_settings_map.h" #include "chrome/browser/profile.h" @@ -10,10 +12,51 @@ #include "chrome/test/ui_test_utils.h" #include "net/base/mock_host_resolver.h" -class CookiePolicyBrowserTest : public InProcessBrowserTest { +namespace { + +class GetCookiesTask : public Task { public: + GetCookiesTask(const GURL& url, + URLRequestContextGetter* context_getter, + base::WaitableEvent* event, + std::string* cookies) + : url_(url), + context_getter_(context_getter), + event_(event), + cookies_(cookies) {} + + virtual void Run() { + *cookies_ = context_getter_->GetCookieStore()->GetCookies(url_); + event_->Signal(); + } + + private: + const GURL& url_; + URLRequestContextGetter* const context_getter_; + base::WaitableEvent* const event_; + std::string* const cookies_; + + DISALLOW_COPY_AND_ASSIGN(GetCookiesTask); +}; + +class CookiePolicyBrowserTest : public InProcessBrowserTest { + protected: CookiePolicyBrowserTest() {} + std::string GetCookies(const GURL& url) { + std::string cookies; + base::WaitableEvent event(true /* manual reset */, + false /* not initially signaled */); + URLRequestContextGetter* context_getter = + browser()->profile()->GetRequestContext(); + EXPECT_TRUE( + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + new GetCookiesTask(url, context_getter, &event, &cookies))); + EXPECT_TRUE(event.Wait()); + return cookies; + } + private: DISALLOW_COPY_AND_ASSIGN(CookiePolicyBrowserTest); }; @@ -26,21 +69,17 @@ IN_PROC_BROWSER_TEST_F(CookiePolicyBrowserTest, AllowFirstPartyCookies) { browser()->profile()->GetHostContentSettingsMap()-> SetBlockThirdPartyCookies(true); - net::CookieStore* cookie_store = - browser()->profile()->GetRequestContext()->GetCookieStore(); - GURL url = server->TestServerPage("set-cookie?cookie1"); - std::string cookie = cookie_store->GetCookies(url); + std::string cookie = GetCookies(url); ASSERT_EQ("", cookie); ui_test_utils::NavigateToURL(browser(), url); - cookie = cookie_store->GetCookies(url); + cookie = GetCookies(url); EXPECT_EQ("cookie1", cookie); } - // Visits a page that is a redirect across domain boundary to a page that sets // a first-party cookie. IN_PROC_BROWSER_TEST_F(CookiePolicyBrowserTest, @@ -51,9 +90,6 @@ IN_PROC_BROWSER_TEST_F(CookiePolicyBrowserTest, browser()->profile()->GetHostContentSettingsMap()-> SetBlockThirdPartyCookies(true); - net::CookieStore* cookie_store = - browser()->profile()->GetRequestContext()->GetCookieStore(); - GURL url = server->TestServerPage("server-redirect?"); GURL redirected_url = server->TestServerPage("set-cookie?cookie2"); @@ -66,7 +102,7 @@ IN_PROC_BROWSER_TEST_F(CookiePolicyBrowserTest, replacements.SetHostStr(new_host); redirected_url = redirected_url.ReplaceComponents(replacements); - std::string cookie = cookie_store->GetCookies(redirected_url); + std::string cookie = GetCookies(redirected_url); ASSERT_EQ("", cookie); host_resolver()->AddRule("www.example.com", "127.0.0.1"); @@ -74,6 +110,8 @@ IN_PROC_BROWSER_TEST_F(CookiePolicyBrowserTest, ui_test_utils::NavigateToURL(browser(), GURL(url.spec() + redirected_url.spec())); - cookie = cookie_store->GetCookies(redirected_url); + cookie = GetCookies(redirected_url); EXPECT_EQ("cookie2", cookie); } + +} // namespace |