diff --git a/DEPS b/DEPS index 4910fb7..ebec986 100644 --- a/DEPS +++ b/DEPS
@@ -78,7 +78,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling Skia # and whatever else without interference from each other. - 'skia_revision': '5c68dce7750a5525a5cfae29943716ce5c0d51b3', + 'skia_revision': '8bc327bffb7ae7d10b93ee75c3d207caeb37f93b', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other. @@ -134,7 +134,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling catapult # and whatever else without interference from each other. - 'catapult_revision': 'efc4a08476292e474385a77e7a5d7c3c8e101a30', + 'catapult_revision': '5626f3590eea9631f90a98549c73334577994819', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling libFuzzer # and whatever else without interference from each other. @@ -643,7 +643,7 @@ Var('chromium_git') + '/external/khronosgroup/webgl.git' + '@' + 'd458ada06171a85af00367251a4ed55db7fe2018', 'src/third_party/webrtc': - Var('webrtc_git') + '/src.git' + '@' + 'fd7df98826e252a68dd27854eb5ac4d0ed8517c6', # commit position 20628 + Var('webrtc_git') + '/src.git' + '@' + '1e53f8ad040ad8e614c0d1205cff070a8a03bf44', # commit position 20628 'src/third_party/xdg-utils': { 'url': Var('chromium_git') + '/chromium/deps/xdg-utils.git' + '@' + 'd80274d5869b17b8c9067a1022e4416ee7ed5e0d',
diff --git a/chrome/browser/extensions/BUILD.gn b/chrome/browser/extensions/BUILD.gn index bd83907..cd4959ea 100644 --- a/chrome/browser/extensions/BUILD.gn +++ b/chrome/browser/extensions/BUILD.gn
@@ -882,6 +882,7 @@ "//services/data_decoder/public/cpp", "//services/device/public/interfaces", "//services/identity/public/interfaces", + "//services/network/public/interfaces", "//services/service_manager/public/cpp", "//services/service_manager/public/interfaces", "//skia",
diff --git a/chrome/browser/extensions/DEPS b/chrome/browser/extensions/DEPS index c668eb2..d87bfeb4 100644 --- a/chrome/browser/extensions/DEPS +++ b/chrome/browser/extensions/DEPS
@@ -9,6 +9,7 @@ "+ui/base", "+components/arc", "+components/vector_icons", + "+services/network/public/interfaces", # For access to testing command line switches. "+ppapi/shared_impl",
diff --git a/chrome/browser/extensions/api/DEPS b/chrome/browser/extensions/api/DEPS index f151a879..fea8d34 100644 --- a/chrome/browser/extensions/api/DEPS +++ b/chrome/browser/extensions/api/DEPS
@@ -8,6 +8,7 @@ # Enable remote assistance on Chrome OS "+remoting/base", "+remoting/host", + "+services/network/public/interfaces", ] specific_include_rules = {
diff --git a/chrome/browser/extensions/api/cookies/cookies_api.cc b/chrome/browser/extensions/api/cookies/cookies_api.cc index 4a705c1..8feb848 100644 --- a/chrome/browser/extensions/api/cookies/cookies_api.cc +++ b/chrome/browser/extensions/api/cookies/cookies_api.cc
@@ -24,18 +24,17 @@ #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_list.h" #include "chrome/common/extensions/api/cookies.h" +#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/notification_service.h" +#include "content/public/browser/storage_partition.h" +#include "content/public/common/network_service.mojom.h" #include "extensions/browser/event_router.h" #include "extensions/common/error_utils.h" #include "extensions/common/extension.h" #include "extensions/common/permissions/permissions_data.h" #include "net/cookies/canonical_cookie.h" #include "net/cookies/cookie_constants.h" -#include "net/cookies/cookie_monster.h" -#include "net/cookies/cookie_store.h" -#include "net/url_request/url_request_context.h" -#include "net/url_request/url_request_context_getter.h" using content::BrowserThread; @@ -71,10 +70,9 @@ return true; } -bool ParseStoreContext(ChromeAsyncExtensionFunction* function, - std::string* store_id, - net::URLRequestContextGetter** context) { - DCHECK((context || store_id->empty())); +network::mojom::CookieManager* ParseStoreCookieManager( + ChromeAsyncExtensionFunction* function, + std::string* store_id) { Profile* store_profile = NULL; if (!store_id->empty()) { store_profile = cookies_helpers::ChooseProfileFromStoreId( @@ -82,26 +80,25 @@ if (!store_profile) { function->SetError(ErrorUtils::FormatErrorMessage( keys::kInvalidStoreIdError, *store_id)); - return false; + return nullptr; } } else { // The store ID was not specified; use the current execution context's // cookie store by default. // GetCurrentBrowser() already takes into account incognito settings. + // TODO(rdevlin.cronin): Relying on the current execution context is + // almost never the right answer; clean this up. Browser* current_browser = function->GetCurrentBrowser(); if (!current_browser) { function->SetError(keys::kNoCookieStoreFoundError); - return false; + return nullptr; } store_profile = current_browser->profile(); *store_id = cookies_helpers::GetStoreIdFromProfile(store_profile); } - if (context) - *context = store_profile->GetRequestContext(); - DCHECK(context); - - return true; + return content::BrowserContext::GetDefaultStoragePartition(store_profile) + ->GetCookieManagerForBrowserProcess(); } } // namespace @@ -212,38 +209,28 @@ std::string store_id = parsed_args_->details.store_id.get() ? *parsed_args_->details.store_id : std::string(); - net::URLRequestContextGetter* store_context = NULL; - if (!ParseStoreContext(this, &store_id, &store_context)) + network::mojom::CookieManager* cookie_manager = + ParseStoreCookieManager(this, &store_id); + if (!cookie_manager) return false; - store_browser_context_ = store_context; + if (!parsed_args_->details.store_id.get()) parsed_args_->details.store_id.reset(new std::string(store_id)); - store_browser_context_ = store_context; - - bool rv = BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::BindOnce(&CookiesGetFunction::GetCookieOnIOThread, this)); - DCHECK(rv); + cookies_helpers::GetCookieListFromManager( + cookie_manager, url_, + base::BindOnce(&CookiesGetFunction::GetCookieCallback, this)); // Will finish asynchronously. return true; } -void CookiesGetFunction::GetCookieOnIOThread() { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - net::CookieStore* cookie_store = - store_browser_context_->GetURLRequestContext()->cookie_store(); - cookies_helpers::GetCookieListFromStore( - cookie_store, url_, - base::BindOnce(&CookiesGetFunction::GetCookieCallback, this)); -} - void CookiesGetFunction::GetCookieCallback(const net::CookieList& cookie_list) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); for (const net::CanonicalCookie& cookie : cookie_list) { // Return the first matching cookie. Relies on the fact that the - // CookieMonster returns them in canonical order (longest path, then - // earliest creation time). + // CookieManager interface returns them in canonical order (longest path, + // then earliest creation time). if (cookie.Name() == parsed_args_->details.name) { cookies::Cookie api_cookie = cookies_helpers::CreateCookie( cookie, *parsed_args_->details.store_id); @@ -256,14 +243,6 @@ if (!results_) SetResult(base::MakeUnique<base::Value>()); - bool rv = BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::BindOnce(&CookiesGetFunction::RespondOnUIThread, this)); - DCHECK(rv); -} - -void CookiesGetFunction::RespondOnUIThread() { - DCHECK_CURRENTLY_ON(BrowserThread::UI); SendResponse(true); } @@ -285,33 +264,26 @@ std::string store_id = parsed_args_->details.store_id.get() ? *parsed_args_->details.store_id : std::string(); - net::URLRequestContextGetter* store_context = NULL; - if (!ParseStoreContext(this, &store_id, &store_context)) + network::mojom::CookieManager* cookie_manager = + ParseStoreCookieManager(this, &store_id); + if (!cookie_manager) return false; - store_browser_context_ = store_context; + if (!parsed_args_->details.store_id.get()) parsed_args_->details.store_id.reset(new std::string(store_id)); - bool rv = BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::BindOnce(&CookiesGetAllFunction::GetAllCookiesOnIOThread, this)); - DCHECK(rv); + DCHECK(url_.is_empty() || url_.is_valid()); + cookies_helpers::GetCookieListFromManager( + cookie_manager, url_, + base::BindOnce(&CookiesGetAllFunction::GetAllCookiesCallback, this)); // Will finish asynchronously. return true; } -void CookiesGetAllFunction::GetAllCookiesOnIOThread() { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - net::CookieStore* cookie_store = - store_browser_context_->GetURLRequestContext()->cookie_store(); - cookies_helpers::GetCookieListFromStore( - cookie_store, url_, - base::BindOnce(&CookiesGetAllFunction::GetAllCookiesCallback, this)); -} - void CookiesGetAllFunction::GetAllCookiesCallback( const net::CookieList& cookie_list) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); if (extension()) { std::vector<cookies::Cookie> match_vector; cookies_helpers::AppendMatchingCookiesToVector( @@ -319,19 +291,11 @@ results_ = GetAll::Results::Create(match_vector); } - bool rv = BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::BindOnce(&CookiesGetAllFunction::RespondOnUIThread, this)); - DCHECK(rv); -} - -void CookiesGetAllFunction::RespondOnUIThread() { - DCHECK_CURRENTLY_ON(BrowserThread::UI); SendResponse(true); } -CookiesSetFunction::CookiesSetFunction() : success_(false) { -} +CookiesSetFunction::CookiesSetFunction() + : state_(NO_RESPONSE), success_(false) {} CookiesSetFunction::~CookiesSetFunction() { } @@ -347,26 +311,14 @@ std::string store_id = parsed_args_->details.store_id.get() ? *parsed_args_->details.store_id : std::string(); - net::URLRequestContextGetter* store_context = NULL; - if (!ParseStoreContext(this, &store_id, &store_context)) + network::mojom::CookieManager* cookie_manager = + ParseStoreCookieManager(this, &store_id); + if (!cookie_manager) return false; - store_browser_context_ = store_context; + if (!parsed_args_->details.store_id.get()) parsed_args_->details.store_id.reset(new std::string(store_id)); - bool rv = BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::BindOnce(&CookiesSetFunction::SetCookieOnIOThread, this)); - DCHECK(rv); - - // Will finish asynchronously. - return true; -} - -void CookiesSetFunction::SetCookieOnIOThread() { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - net::CookieStore* cookie_store = - store_browser_context_->GetURLRequestContext()->cookie_store(); base::Time expiration_time; if (parsed_args_->details.expiration_date.get()) { @@ -414,26 +366,40 @@ net::COOKIE_PRIORITY_DEFAULT)); // clang-format on if (!cc) { - PullCookie(false); - return; + // Return error through callbacks so that the proper error message + // is generated. + success_ = false; + state_ = SET_COMPLETED; + GetCookieListCallback(net::CookieList()); + return true; } - cookie_store->SetCanonicalCookieAsync( - std::move(cc), url_.SchemeIsCryptographic(), true /*modify_http_only*/, - base::BindOnce(&CookiesSetFunction::PullCookie, this)); + + // Dispatch the setter, immediately followed by the getter. This + // plus FIFO ordering on the cookie_manager_ pipe means that no + // other extension function will affect the get result. + cookie_manager->SetCanonicalCookie( + *cc, url_.SchemeIsCryptographic(), true /*modify_http_only*/, + base::BindOnce(&CookiesSetFunction::SetCanonicalCookieCallback, this)); + cookies_helpers::GetCookieListFromManager( + cookie_manager, url_, + base::BindOnce(&CookiesSetFunction::GetCookieListCallback, this)); + + // Will finish asynchronously. + return true; } -void CookiesSetFunction::PullCookie(bool set_cookie_result) { - // Pull the newly set cookie. - net::CookieStore* cookie_store = - store_browser_context_->GetURLRequestContext()->cookie_store(); +void CookiesSetFunction::SetCanonicalCookieCallback(bool set_cookie_result) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + DCHECK_EQ(NO_RESPONSE, state_); + state_ = SET_COMPLETED; success_ = set_cookie_result; - cookies_helpers::GetCookieListFromStore( - cookie_store, url_, - base::BindOnce(&CookiesSetFunction::PullCookieCallback, this)); } -void CookiesSetFunction::PullCookieCallback( +void CookiesSetFunction::GetCookieListCallback( const net::CookieList& cookie_list) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + DCHECK_EQ(SET_COMPLETED, state_); + state_ = GET_COMPLETED; for (const net::CanonicalCookie& cookie : cookie_list) { // Return the first matching cookie. Relies on the fact that the // CookieMonster returns them in canonical order (longest path, then @@ -449,18 +415,12 @@ } } - bool rv = BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::BindOnce(&CookiesSetFunction::RespondOnUIThread, this)); - DCHECK(rv); -} - -void CookiesSetFunction::RespondOnUIThread() { - DCHECK_CURRENTLY_ON(BrowserThread::UI); if (!success_) { std::string name = parsed_args_->details.name.get() ? *parsed_args_->details.name : std::string(); + // TODO(rdevlin.cronin): Avoid setting both error_ and results_ in the + // same call. error_ = ErrorUtils::FormatErrorMessage(keys::kCookieSetFailedError, name); } SendResponse(success_); @@ -483,35 +443,29 @@ std::string store_id = parsed_args_->details.store_id.get() ? *parsed_args_->details.store_id : std::string(); - net::URLRequestContextGetter* store_context = NULL; - if (!ParseStoreContext(this, &store_id, &store_context)) + network::mojom::CookieManager* cookie_manager = + ParseStoreCookieManager(this, &store_id); + if (!cookie_manager) return false; - store_browser_context_ = store_context; + if (!parsed_args_->details.store_id.get()) parsed_args_->details.store_id.reset(new std::string(store_id)); - // Pass the work off to the IO thread. - bool rv = BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::BindOnce(&CookiesRemoveFunction::RemoveCookieOnIOThread, this)); - DCHECK(rv); + network::mojom::CookieDeletionFilterPtr filter( + network::mojom::CookieDeletionFilter::New()); + filter->url = url_; + filter->cookie_name = parsed_args_->details.name; + cookie_manager->DeleteCookies( + std::move(filter), + base::BindOnce(&CookiesRemoveFunction::RemoveCookieCallback, this)); // Will return asynchronously. return true; } -void CookiesRemoveFunction::RemoveCookieOnIOThread() { - DCHECK_CURRENTLY_ON(BrowserThread::IO); +void CookiesRemoveFunction::RemoveCookieCallback(uint32_t /* num_deleted */) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); - // Remove the cookie - net::CookieStore* cookie_store = - store_browser_context_->GetURLRequestContext()->cookie_store(); - cookie_store->DeleteCookieAsync( - url_, parsed_args_->details.name, - base::BindOnce(&CookiesRemoveFunction::RemoveCookieCallback, this)); -} - -void CookiesRemoveFunction::RemoveCookieCallback() { // Build the callback result Remove::Results::Details details; details.name = parsed_args_->details.name; @@ -519,15 +473,6 @@ details.store_id = *parsed_args_->details.store_id; results_ = Remove::Results::Create(details); - // Return to UI thread - bool rv = BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::BindOnce(&CookiesRemoveFunction::RespondOnUIThread, this)); - DCHECK(rv); -} - -void CookiesRemoveFunction::RespondOnUIThread() { - DCHECK_CURRENTLY_ON(BrowserThread::UI); SendResponse(true); }
diff --git a/chrome/browser/extensions/api/cookies/cookies_api.h b/chrome/browser/extensions/api/cookies/cookies_api.h index 4dfa71c..b5a5ad04 100644 --- a/chrome/browser/extensions/api/cookies/cookies_api.h +++ b/chrome/browser/extensions/api/cookies/cookies_api.h
@@ -22,12 +22,9 @@ #include "extensions/browser/browser_context_keyed_api_factory.h" #include "extensions/browser/event_router.h" #include "net/cookies/canonical_cookie.h" +#include "services/network/public/interfaces/cookie_manager.mojom.h" #include "url/gurl.h" -namespace net { -class URLRequestContextGetter; -} - namespace extensions { // Observes CookieMonster notifications and routes them as events to the @@ -76,12 +73,10 @@ bool RunAsync() override; private: - void GetCookieOnIOThread(); - void RespondOnUIThread(); void GetCookieCallback(const net::CookieList& cookie_list); GURL url_; - scoped_refptr<net::URLRequestContextGetter> store_browser_context_; + network::mojom::CookieManagerPtr store_browser_cookie_manager_; std::unique_ptr<api::cookies::Get::Params> parsed_args_; }; @@ -99,12 +94,10 @@ bool RunAsync() override; private: - void GetAllCookiesOnIOThread(); - void RespondOnUIThread(); void GetAllCookiesCallback(const net::CookieList& cookie_list); GURL url_; - scoped_refptr<net::URLRequestContextGetter> store_browser_context_; + network::mojom::CookieManagerPtr store_browser_cookie_manager_; std::unique_ptr<api::cookies::GetAll::Params> parsed_args_; }; @@ -120,14 +113,13 @@ bool RunAsync() override; private: - void SetCookieOnIOThread(); - void RespondOnUIThread(); - void PullCookie(bool set_cookie_); - void PullCookieCallback(const net::CookieList& cookie_list); + void SetCanonicalCookieCallback(bool set_cookie_); + void GetCookieListCallback(const net::CookieList& cookie_list); + enum { NO_RESPONSE, SET_COMPLETED, GET_COMPLETED } state_; GURL url_; bool success_; - scoped_refptr<net::URLRequestContextGetter> store_browser_context_; + network::mojom::CookieManagerPtr store_browser_cookie_manager_; std::unique_ptr<api::cookies::Set::Params> parsed_args_; }; @@ -145,12 +137,10 @@ bool RunAsync() override; private: - void RemoveCookieOnIOThread(); - void RespondOnUIThread(); - void RemoveCookieCallback(); + void RemoveCookieCallback(uint32_t /* num_deleted */); GURL url_; - scoped_refptr<net::URLRequestContextGetter> store_browser_context_; + network::mojom::CookieManagerPtr store_browser_cookie_manager_; std::unique_ptr<api::cookies::Remove::Params> parsed_args_; };
diff --git a/chrome/browser/extensions/api/cookies/cookies_helpers.cc b/chrome/browser/extensions/api/cookies/cookies_helpers.cc index 42ca67b..a9b8840e 100644 --- a/chrome/browser/extensions/api/cookies/cookies_helpers.cc +++ b/chrome/browser/extensions/api/cookies/cookies_helpers.cc
@@ -118,16 +118,20 @@ return cookie_store; } -void GetCookieListFromStore( - net::CookieStore* cookie_store, +void GetCookieListFromManager( + network::mojom::CookieManager* manager, const GURL& url, - net::CookieMonster::GetCookieListCallback callback) { - DCHECK(cookie_store); - if (!url.is_empty()) { - DCHECK(url.is_valid()); - cookie_store->GetAllCookiesForURLAsync(url, std::move(callback)); + network::mojom::CookieManager::GetCookieListCallback callback) { + if (url.is_empty()) { + manager->GetAllCookies(std::move(callback)); } else { - cookie_store->GetAllCookiesAsync(std::move(callback)); + net::CookieOptions options; + options.set_include_httponly(); + options.set_same_site_cookie_mode( + net::CookieOptions::SameSiteCookieMode::INCLUDE_STRICT_AND_LAX); + options.set_do_not_update_access_time(); + + manager->GetCookieList(url, options, std::move(callback)); } }
diff --git a/chrome/browser/extensions/api/cookies/cookies_helpers.h b/chrome/browser/extensions/api/cookies/cookies_helpers.h index 0fa58c69..ec287ff 100644 --- a/chrome/browser/extensions/api/cookies/cookies_helpers.h +++ b/chrome/browser/extensions/api/cookies/cookies_helpers.h
@@ -15,8 +15,10 @@ #include <vector> #include "chrome/common/extensions/api/cookies.h" -#include "net/cookies/cookie_monster.h" #include "net/cookies/canonical_cookie.h" +#include "net/cookies/cookie_monster.h" +#include "net/cookies/cookie_options.h" +#include "services/network/public/interfaces/cookie_manager.mojom.h" class Browser; class Profile; @@ -55,12 +57,12 @@ Profile* profile, std::unique_ptr<base::ListValue> tab_ids); -// 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. -void GetCookieListFromStore(net::CookieStore* cookie_store, - const GURL& url, - net::CookieMonster::GetCookieListCallback callback); +// Dispatch a request to the CookieManager for cookies associated with +// |url|, or all cookies if |url.is_empty()|. +void GetCookieListFromManager( + network::mojom::CookieManager* manager, + const GURL& url, + network::mojom::CookieManager::GetCookieListCallback callback); // Constructs a URL from a cookie's information for use in checking // a cookie against the extension's host permissions. The Secure
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc index 943aae57..b344cfc 100644 --- a/chrome/browser/extensions/extension_service_unittest.cc +++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -35,6 +35,7 @@ #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" +#include "base/time/time.h" #include "base/values.h" #include "base/version.h" #include "build/build_config.h" @@ -99,6 +100,7 @@ #include "content/public/browser/render_process_host.h" #include "content/public/browser/storage_partition.h" #include "content/public/common/content_constants.h" +#include "content/public/common/network_service.mojom.h" #include "content/public/test/test_browser_thread_bundle.h" #include "content/public/test/test_utils.h" #include "extensions/browser/extension_prefs.h" @@ -128,11 +130,13 @@ #include "extensions/common/switches.h" #include "extensions/common/url_pattern.h" #include "extensions/common/value_builder.h" +#include "net/cookies/canonical_cookie.h" #include "net/cookies/cookie_options.h" #include "net/cookies/cookie_store.h" #include "net/url_request/url_request_context.h" #include "net/url_request/url_request_context_getter.h" #include "ppapi/features/features.h" +#include "services/network/public/interfaces/cookie_manager.mojom.h" #include "storage/browser/database/database_tracker.h" #include "storage/browser/quota/quota_manager.h" #include "storage/common/database/database_identifier.h" @@ -4792,6 +4796,20 @@ EXPECT_FALSE(base::DirectoryExists(idb_path)); } +void SetCookieSaveData(bool* result_out, + base::OnceClosure callback, + bool result) { + *result_out = result; + std::move(callback).Run(); +} + +void GetCookiesSaveData(std::vector<net::CanonicalCookie>* result_out, + base::OnceClosure callback, + const std::vector<net::CanonicalCookie>& result) { + *result_out = result; + std::move(callback).Run(); +} + // Verifies app state is removed upon uninstall. TEST_F(ExtensionServiceTest, ClearAppData) { InitializeEmptyExtensionService(); @@ -4828,25 +4846,37 @@ EXPECT_TRUE(profile()->GetExtensionSpecialStoragePolicy()->IsStorageUnlimited( origin2)); - // Set a cookie for the extension. - net::CookieStore* cookie_store = profile()->GetRequestContext() - ->GetURLRequestContext() - ->cookie_store(); - ASSERT_TRUE(cookie_store); - net::CookieOptions options; - cookie_store->SetCookieWithOptionsAsync( - origin1, "dummy=value", options, - base::Bind(&ExtensionCookieCallback::SetCookieCallback, - base::Unretained(&callback))); - content::RunAllTasksUntilIdle(); - EXPECT_TRUE(callback.result_); + content::mojom::NetworkContext* network_context = + content::BrowserContext::GetDefaultStoragePartition(profile()) + ->GetNetworkContext(); + network::mojom::CookieManagerPtr cookie_manager_ptr; + network_context->GetCookieManager(mojo::MakeRequest(&cookie_manager_ptr)); - cookie_store->GetAllCookiesForURLAsync( - origin1, - base::Bind(&ExtensionCookieCallback::GetAllCookiesCallback, - base::Unretained(&callback))); - content::RunAllTasksUntilIdle(); - EXPECT_EQ(1U, callback.list_.size()); + std::unique_ptr<net::CanonicalCookie> cc(net::CanonicalCookie::Create( + origin1, "dummy=value", base::Time::Now(), net::CookieOptions())); + ASSERT_TRUE(cc.get()); + + { + bool set_result = false; + base::RunLoop run_loop; + cookie_manager_ptr->SetCanonicalCookie( + *cc.get(), origin1.SchemeIsCryptographic(), true /* modify_http_only */, + base::BindOnce(&SetCookieSaveData, &set_result, + run_loop.QuitClosure())); + run_loop.Run(); + EXPECT_TRUE(set_result); + } + + { + base::RunLoop run_loop; + std::vector<net::CanonicalCookie> cookies_result; + cookie_manager_ptr->GetCookieList( + origin1, net::CookieOptions(), + base::BindOnce(&GetCookiesSaveData, &cookies_result, + run_loop.QuitClosure())); + run_loop.Run(); + EXPECT_EQ(1U, cookies_result.size()); + } // Open a database. storage::DatabaseTracker* db_tracker = @@ -4885,13 +4915,17 @@ EXPECT_TRUE(profile()->GetExtensionSpecialStoragePolicy()->IsStorageUnlimited( origin1)); - // Check that the cookie is still there. - cookie_store->GetAllCookiesForURLAsync( - origin1, - base::Bind(&ExtensionCookieCallback::GetAllCookiesCallback, - base::Unretained(&callback))); - content::RunAllTasksUntilIdle(); - EXPECT_EQ(1U, callback.list_.size()); + { + // Check that the cookie is still there. + base::RunLoop run_loop; + std::vector<net::CanonicalCookie> cookies_result; + cookie_manager_ptr->GetCookieList( + origin1, net::CookieOptions(), + base::BindOnce(&GetCookiesSaveData, &cookies_result, + run_loop.QuitClosure())); + run_loop.Run(); + EXPECT_EQ(1U, cookies_result.size()); + } // Now uninstall the other. Storage should be cleared for the apps. UninstallExtension(id2); @@ -4900,13 +4934,17 @@ profile()->GetExtensionSpecialStoragePolicy()->IsStorageUnlimited( origin1)); - // Check that the cookie is gone. - cookie_store->GetAllCookiesForURLAsync( - origin1, - base::Bind(&ExtensionCookieCallback::GetAllCookiesCallback, - base::Unretained(&callback))); - content::RunAllTasksUntilIdle(); - EXPECT_EQ(0U, callback.list_.size()); + { + // Check that the cookie is gone. + base::RunLoop run_loop; + std::vector<net::CanonicalCookie> cookies_result; + cookie_manager_ptr->GetCookieList( + origin1, net::CookieOptions(), + base::BindOnce(&GetCookiesSaveData, &cookies_result, + run_loop.QuitClosure())); + run_loop.Run(); + EXPECT_EQ(0U, cookies_result.size()); + } // The database should have vanished as well. db_tracker->task_runner()->PostTask(
diff --git a/components/BUILD.gn b/components/BUILD.gn index 3d8ab7a..1aa0dfe 100644 --- a/components/BUILD.gn +++ b/components/BUILD.gn
@@ -402,6 +402,7 @@ sources = [ "autofill/content/browser/risk/fingerprint_browsertest.cc", + "autofill/content/renderer/form_autofill_util_browsertest.cc", "autofill/content/renderer/password_form_conversion_utils_browsertest.cc", "dom_distiller/content/browser/distillable_page_utils_browsertest.cc", "dom_distiller/content/browser/distiller_page_web_contents_browsertest.cc",
diff --git a/components/autofill/content/renderer/form_autofill_util.cc b/components/autofill/content/renderer/form_autofill_util.cc index 4938fe88..a5b221a06 100644 --- a/components/autofill/content/renderer/form_autofill_util.cc +++ b/components/autofill/content/renderer/form_autofill_util.cc
@@ -1831,5 +1831,17 @@ return FindChildTextWithIgnoreList(node, std::set<WebNode>()); } +base::string16 FindChildTextWithIgnoreListForTesting( + const WebNode& node, + const std::set<WebNode>& divs_to_skip) { + return FindChildTextWithIgnoreList(node, divs_to_skip); +} + +base::string16 InferLabelForElementForTesting( + const WebFormControlElement& element, + const std::vector<base::char16>& stop_words) { + return InferLabelForElement(element, stop_words); +} + } // namespace form_util } // namespace autofill
diff --git a/components/autofill/content/renderer/form_autofill_util.h b/components/autofill/content/renderer/form_autofill_util.h index 38378c3..ed772f5 100644 --- a/components/autofill/content/renderer/form_autofill_util.h +++ b/components/autofill/content/renderer/form_autofill_util.h
@@ -7,6 +7,7 @@ #include <stddef.h> +#include <set> #include <vector> #include "base/macros.h" @@ -278,6 +279,14 @@ // Whitespace is trimmed from text accumulated at descendant nodes. base::string16 FindChildText(const blink::WebNode& node); +// Exposed for testing purpose +base::string16 FindChildTextWithIgnoreListForTesting( + const blink::WebNode& node, + const std::set<blink::WebNode>& divs_to_skip); +base::string16 InferLabelForElementForTesting( + const blink::WebFormControlElement& element, + const std::vector<base::char16>& stop_words); + } // namespace form_util } // namespace autofill
diff --git a/components/autofill/content/renderer/form_autofill_util_browsertest.cc b/components/autofill/content/renderer/form_autofill_util_browsertest.cc new file mode 100644 index 0000000..44dffc4 --- /dev/null +++ b/components/autofill/content/renderer/form_autofill_util_browsertest.cc
@@ -0,0 +1,206 @@ +// Copyright (c) 2017 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 "components/autofill/content/renderer/form_autofill_util.h" + +#include "base/strings/utf_string_conversions.h" +#include "content/public/test/render_view_test.h" +#include "third_party/WebKit/public/platform/WebString.h" +#include "third_party/WebKit/public/platform/WebVector.h" +#include "third_party/WebKit/public/web/WebDocument.h" +#include "third_party/WebKit/public/web/WebElement.h" +#include "third_party/WebKit/public/web/WebElementCollection.h" +#include "third_party/WebKit/public/web/WebFormControlElement.h" +#include "third_party/WebKit/public/web/WebFormElement.h" +#include "third_party/WebKit/public/web/WebInputElement.h" +#include "third_party/WebKit/public/web/WebLocalFrame.h" +#include "third_party/WebKit/public/web/WebSelectElement.h" + +using blink::WebDocument; +using blink::WebElement; +using blink::WebFormControlElement; +using blink::WebFormElement; +using blink::WebInputElement; +using blink::WebLocalFrame; +using blink::WebSelectElement; +using blink::WebString; +using blink::WebVector; + +struct AutofillFieldUtilCase { + const char* description; + const char* html; + const char* expected_label; +}; + +const char kElevenChildren[] = + "<div id='target'>" + "<div>child0</div>" + "<div>child1</div>" + "<div>child2</div>" + "<div>child3</div>" + "<div>child4</div>" + "<div>child5</div>" + "<div>child6</div>" + "<div>child7</div>" + "<div>child8</div>" + "<div>child9</div>" + "<div>child10</div>" + "</div>"; +const char kElevenChildrenExpected[] = + "child0child1child2child3child4child5child6child7child8"; + +const char kElevenChildrenNested[] = + "<div id='target'>" + "<div>child0" + "<div>child1" + "<div>child2" + "<div>child3" + "<div>child4" + "<div>child5" + "<div>child6" + "<div>child7" + "<div>child8" + "<div>child9" + "<div>child10" + "</div></div></div></div></div></div></div></div></div></div></div></div>"; +// Take 10 elements -1 for target element, -1 as text is a leaf element. +const char kElevenChildrenNestedExpected[] = "child0child1child2child3child4"; + +const char kSkipElement[] = + "<div id='target'>" + "<div>child0</div>" + "<div class='skip'>child1</div>" + "<div>child2</div>" + "</div>"; +// TODO(crbug.com/796918): Should be child0child2 +const char kSkipElementExpected[] = "child0"; + +const char kDivTableExample1[] = + "<div>" + "<div>label</div><div><input id='target'/></div>" + "</div>"; +const char kDivTableExample1Expected[] = "label"; + +const char kDivTableExample2[] = + "<div>" + "<div>label</div>" + "<div>should be skipped<input/></div>" + "<div><input id='target'/></div>" + "</div>"; +const char kDivTableExample2Expected[] = "label"; + +const char kDivTableExample3[] = + "<div>" + "<div>should be skipped<input/></div>" + "<div>label</div>" + "<div><input id='target'/></div>" + "</div>"; +const char kDivTableExample3Expected[] = "label"; + +const char kDivTableExample4[] = + "<div>" + "<div>should be skipped<input/></div>" + "label" + "<div><input id='target'/></div>" + "</div>"; +// TODO(crbug.com/796918): Should be label +const char kDivTableExample4Expected[] = ""; + +const char kDivTableExample5[] = + "<div>" + "<div>label<div><input id='target'/></div>behind</div>" + "</div>"; +// TODO(crbug.com/796918): Should be label +const char kDivTableExample5Expected[] = "labelbehind"; + +const char kDivTableExample6[] = + "<div>" + "<div>label<div><div>-<div><input id='target'/></div></div>" + "</div>"; +// TODO(crbug.com/796918): Should be "label" or "label-" +const char kDivTableExample6Expected[] = ""; + +class FormAutofillUtilsTest : public content::RenderViewTest { + public: + FormAutofillUtilsTest() : content::RenderViewTest() {} + ~FormAutofillUtilsTest() override {} +}; + +TEST_F(FormAutofillUtilsTest, FindChildTextTest) { + static const AutofillFieldUtilCase test_cases[] = { + {"simple test", "<div id='target'>test</div>", "test"}, + {"Concatenate test", "<div id='target'><span>one</span>two</div>", + "onetwo"}, + // TODO(crbug.com/796918): should be "onetwo" + {"Ignore input", "<div id='target'>one<input value='test'/>two</div>", + "one"}, + {"Trim", "<div id='target'> one<span>two </span></div>", "onetwo"}, + {"eleven children", kElevenChildren, kElevenChildrenExpected}, + // TODO(crbug.com/796918): Depth is only 5 elements + {"eleven children nested", kElevenChildrenNested, + kElevenChildrenNestedExpected}, + }; + for (auto test_case : test_cases) { + SCOPED_TRACE(test_case.description); + LoadHTML(test_case.html); + WebLocalFrame* web_frame = GetMainFrame(); + ASSERT_NE(nullptr, web_frame); + WebElement target = web_frame->GetDocument().GetElementById("target"); + ASSERT_FALSE(target.IsNull()); + EXPECT_EQ(base::UTF8ToUTF16(test_case.expected_label), + autofill::form_util::FindChildText(target)); + } +} + +TEST_F(FormAutofillUtilsTest, FindChildTextSkipElementTest) { + static const AutofillFieldUtilCase test_cases[] = { + {"Skip div element", kSkipElement, kSkipElementExpected}, + }; + for (auto test_case : test_cases) { + SCOPED_TRACE(test_case.description); + LoadHTML(test_case.html); + WebLocalFrame* web_frame = GetMainFrame(); + ASSERT_NE(nullptr, web_frame); + WebElement target = web_frame->GetDocument().GetElementById("target"); + ASSERT_FALSE(target.IsNull()); + WebVector<WebElement> web_to_skip = + web_frame->GetDocument().QuerySelectorAll("div[class='skip']"); + std::set<blink::WebNode> to_skip; + for (size_t i = 0; i < web_to_skip.size(); ++i) { + to_skip.insert(web_to_skip[i]); + } + + EXPECT_EQ(base::UTF8ToUTF16(test_case.expected_label), + autofill::form_util::FindChildTextWithIgnoreListForTesting( + target, to_skip)); + } +} + +TEST_F(FormAutofillUtilsTest, InferLabelForElementTest) { + std::vector<base::char16> stop_words; + stop_words.push_back(static_cast<base::char16>('-')); + static const AutofillFieldUtilCase test_cases[] = { + {"DIV table test 1", kDivTableExample1, kDivTableExample1Expected}, + {"DIV table test 2", kDivTableExample2, kDivTableExample2Expected}, + {"DIV table test 3", kDivTableExample3, kDivTableExample3Expected}, + {"DIV table test 4", kDivTableExample4, kDivTableExample4Expected}, + {"DIV table test 5", kDivTableExample5, kDivTableExample5Expected}, + {"DIV table test 6", kDivTableExample6, kDivTableExample6Expected}, + }; + for (auto test_case : test_cases) { + SCOPED_TRACE(test_case.description); + LoadHTML(test_case.html); + WebLocalFrame* web_frame = GetMainFrame(); + ASSERT_NE(nullptr, web_frame); + WebElement target = web_frame->GetDocument().GetElementById("target"); + ASSERT_FALSE(target.IsNull()); + const WebFormControlElement form_target = + target.ToConst<WebFormControlElement>(); + ASSERT_FALSE(form_target.IsNull()); + + EXPECT_EQ(base::UTF8ToUTF16(test_case.expected_label), + autofill::form_util::InferLabelForElementForTesting(form_target, + stop_words)); + } +}
diff --git a/content/network/cookie_manager.cc b/content/network/cookie_manager.cc index f74be95..00c4f20 100644 --- a/content/network/cookie_manager.cc +++ b/content/network/cookie_manager.cc
@@ -31,23 +31,46 @@ filter->including_domains.value().begin(), filter->including_domains.value().end()) : std::set<std::string>()), - session_control_(filter->session_control) {} + use_cookie_name_(filter->cookie_name.has_value()), + cookie_name_(filter->cookie_name.has_value() + ? filter->cookie_name.value() + : std::string()), + use_url_(filter->url.has_value()), + url_(filter->url.has_value() ? filter->url.value() : GURL()), + session_control_(filter->session_control) { + // Options to use for deletion of cookies associated with + // a particular URL. These options will make sure that all + // cookies associated with the URL are deleted. + const_cast<net::CookieOptions&>(options_).set_include_httponly(); + const_cast<net::CookieOptions&>(options_).set_same_site_cookie_mode( + net::CookieOptions::SameSiteCookieMode::INCLUDE_STRICT_AND_LAX); + } // Return true if the given cookie should be deleted. bool Predicate(const net::CanonicalCookie& cookie) { // Ignore begin/end times; they're handled by method args. bool result = true; + // Delete if the cookie is not in excluding_domains_. - if (use_excluding_domains_) + if (use_excluding_domains_ && result) result &= !DomainMatches(cookie, excluding_domains_); // Delete if the cookie is in including_domains_. - if (use_including_domains_) + if (use_including_domains_ && result) result &= DomainMatches(cookie, including_domains_); + // Delete if the cookie has a specified name. + if (use_cookie_name_ && result) + result &= (cookie_name_ == cookie.Name()); + + // Delete if the cookie matches the URL. + if (use_url_ && result) + result &= cookie.IncludeForRequestURL(url_, options_); + // Delete if the cookie is not the correct persistent or session type. if (session_control_ != - network::mojom::CookieDeletionSessionControl::IGNORE_CONTROL) { + network::mojom::CookieDeletionSessionControl::IGNORE_CONTROL && + result) { // Relies on the assumption that there are only three values possible for // session_control. result &= @@ -78,13 +101,22 @@ return match_domains.count(effective_domain) != 0; } - bool use_excluding_domains_; - std::set<std::string> excluding_domains_; + const bool use_excluding_domains_; + const std::set<std::string> excluding_domains_; - bool use_including_domains_; - std::set<std::string> including_domains_; + const bool use_including_domains_; + const std::set<std::string> including_domains_; - network::mojom::CookieDeletionSessionControl session_control_; + const bool use_cookie_name_; + const std::string cookie_name_; + + const bool use_url_; + const GURL url_; + + const network::mojom::CookieDeletionSessionControl session_control_; + + // Set at construction; used for IncludeForRequestURL(). + const net::CookieOptions options_; DISALLOW_COPY_AND_ASSIGN(PredicateWrapper); };
diff --git a/content/network/cookie_manager_unittest.cc b/content/network/cookie_manager_unittest.cc index 4ab0e56..b20d180 100644 --- a/content/network/cookie_manager_unittest.cc +++ b/content/network/cookie_manager_unittest.cc
@@ -33,6 +33,7 @@ // # Functions // * CompareCanonicalCookies: Comparison function to make it easy to // sort cookie list responses from the network::mojom::CookieManager. +// * CompareCookiesByValue: As above, but only by value. namespace content { @@ -165,6 +166,18 @@ return callback.result(); } + std::string DumpAllCookies() { + std::string result = "Cookies in store:\n"; + std::vector<net::CanonicalCookie> cookies = + service_wrapper()->GetAllCookies(); + for (int i = 0; i < static_cast<int>(cookies.size()); ++i) { + result += "\t"; + result += cookies[i].DebugString(); + result += "\n"; + } + return result; + } + net::CookieStore* cookie_store() { return &cookie_monster_; } CookieManager* service() const { return cookie_service_.get(); } @@ -1149,6 +1162,154 @@ } } +TEST_F(CookieManagerTest, DeleteByName) { + // Create cookies with varying (name, host) + EXPECT_TRUE(SetCanonicalCookie( + net::CanonicalCookie( + "A1", "val", "foo_host", "/", base::Time(), base::Time(), + base::Time(), /*secure=*/false, /*httponly=*/false, + net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), + true /*secure_source*/, true /*modify_httponly*/)); + + EXPECT_TRUE(SetCanonicalCookie( + net::CanonicalCookie( + "A1", "val", "bar_host", "/", base::Time(), base::Time(), + base::Time(), /*secure=*/false, /*httponly=*/false, + net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), + true /*secure_source*/, true /*modify_httponly*/)); + + EXPECT_TRUE(SetCanonicalCookie( + net::CanonicalCookie( + "A2", "val", "foo_host", "/", base::Time(), base::Time(), + base::Time(), /*secure=*/false, /*httponly=*/false, + net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), + true /*secure_source*/, true /*modify_httponly*/)); + + EXPECT_TRUE(SetCanonicalCookie( + net::CanonicalCookie( + "A3", "val", "bar_host", "/", base::Time(), base::Time(), + base::Time(), /*secure=*/false, /*httponly=*/false, + net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), + true /*secure_source*/, true /*modify_httponly*/)); + + network::mojom::CookieDeletionFilter filter; + filter.cookie_name = std::string("A1"); + EXPECT_EQ(2u, service_wrapper()->DeleteCookies(filter)); + + std::vector<net::CanonicalCookie> cookies = + service_wrapper()->GetAllCookies(); + ASSERT_EQ(2u, cookies.size()); + std::sort(cookies.begin(), cookies.end(), &CompareCanonicalCookies); + EXPECT_EQ("A2", cookies[0].Name()); + EXPECT_EQ("A3", cookies[1].Name()); +} + +TEST_F(CookieManagerTest, DeleteByURL) { + GURL filter_url("http://www.example.com/path"); + + // Cookie that shouldn't be deleted because it's secure. + EXPECT_TRUE(SetCanonicalCookie( + net::CanonicalCookie( + "A01", "val", "www.example.com", "/path", base::Time(), base::Time(), + base::Time(), /*secure=*/true, /*httponly=*/false, + net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), + true /*secure_source*/, true /*modify_httponly*/)); + + // Cookie that should not be deleted because it's a host cookie in a + // subdomain that doesn't exactly match the passed URL. + EXPECT_TRUE(SetCanonicalCookie( + net::CanonicalCookie( + "A02", "val", "sub.www.example.com", "/path", base::Time(), + base::Time(), base::Time(), /*secure=*/false, /*httponly=*/false, + net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), + true /*secure_source*/, true /*modify_httponly*/)); + + // Cookie that shouldn't be deleted because the path doesn't match. + EXPECT_TRUE(SetCanonicalCookie( + net::CanonicalCookie( + "A03", "val", "www.example.com", "/otherpath", base::Time(), + base::Time(), base::Time(), /*secure=*/false, /*httponly=*/false, + net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), + true /*secure_source*/, true /*modify_httponly*/)); + + // Cookie that shouldn't be deleted because the path is more specific + // than the URL. + EXPECT_TRUE(SetCanonicalCookie( + net::CanonicalCookie( + "A04", "val", "www.example.com", "/path/path2", base::Time(), + base::Time(), base::Time(), /*secure=*/false, /*httponly=*/false, + net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), + true /*secure_source*/, true /*modify_httponly*/)); + + // Cookie that shouldn't be deleted because it's at a host cookie domain that + // doesn't exactly match the url's host. + EXPECT_TRUE(SetCanonicalCookie( + net::CanonicalCookie( + "A05", "val", "example.com", "/path", base::Time(), base::Time(), + base::Time(), /*secure=*/false, /*httponly=*/false, + net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), + true /*secure_source*/, true /*modify_httponly*/)); + + // Cookie that should not be deleted because it's not a host cookie and + // has a domain that's more specific than the URL + EXPECT_TRUE(SetCanonicalCookie( + net::CanonicalCookie( + "A06", "val", ".sub.www.example.com", "/path", base::Time(), + base::Time(), base::Time(), /*secure=*/false, /*httponly=*/false, + net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), + true /*secure_source*/, true /*modify_httponly*/)); + + // Cookie that should be deleted because it's not a host cookie and has a + // domain that matches the URL + EXPECT_TRUE(SetCanonicalCookie( + net::CanonicalCookie( + "A07", "val", ".www.example.com", "/path", base::Time(), base::Time(), + base::Time(), /*secure=*/false, /*httponly=*/false, + net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), + true /*secure_source*/, true /*modify_httponly*/)); + + // Cookie that should be deleted because it's not a host cookie and has a + // domain that domain matches the URL. + EXPECT_TRUE(SetCanonicalCookie( + net::CanonicalCookie( + "A08", "val", ".example.com", "/path", base::Time(), base::Time(), + base::Time(), /*secure=*/false, /*httponly=*/false, + net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), + true /*secure_source*/, true /*modify_httponly*/)); + + // Cookie that should be deleted because it matches exactly. + EXPECT_TRUE(SetCanonicalCookie( + net::CanonicalCookie( + "A09", "val", "www.example.com", "/path", base::Time(), base::Time(), + base::Time(), /*secure=*/false, /*httponly=*/false, + net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), + true /*secure_source*/, true /*modify_httponly*/)); + + // Cookie that should be deleted because it applies to a larger set + // of paths than the URL path. + EXPECT_TRUE(SetCanonicalCookie( + net::CanonicalCookie( + "A10", "val", "www.example.com", "/", base::Time(), base::Time(), + base::Time(), /*secure=*/false, /*httponly=*/false, + net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), + true /*secure_source*/, true /*modify_httponly*/)); + + network::mojom::CookieDeletionFilter filter; + filter.url = filter_url; + EXPECT_EQ(4u, service_wrapper()->DeleteCookies(filter)); + + std::vector<net::CanonicalCookie> cookies = + service_wrapper()->GetAllCookies(); + ASSERT_EQ(6u, cookies.size()) << DumpAllCookies(); + std::sort(cookies.begin(), cookies.end(), &CompareCanonicalCookies); + EXPECT_EQ("A01", cookies[0].Name()); + EXPECT_EQ("A02", cookies[1].Name()); + EXPECT_EQ("A03", cookies[2].Name()); + EXPECT_EQ("A04", cookies[3].Name()); + EXPECT_EQ("A05", cookies[4].Name()); + EXPECT_EQ("A06", cookies[5].Name()); +} + TEST_F(CookieManagerTest, DeleteBySessionStatus) { base::Time now(base::Time::Now()); @@ -1196,12 +1357,16 @@ // * Including domains: no.com and nope.com // * Excluding domains: no.com and yes.com (excluding wins on no.com // because of ANDing) + // * Matching a specific URL. // * Persistent cookies. // The archetypal cookie (which will be deleted) will satisfy all of // these filters (2 days old, nope.com, persistent). // Each of the other four cookies will vary in a way that will take it out // of being deleted by one of the filter. + // Cookie name is not included as a filter because the cookies are made + // unique by name. + network::mojom::CookieDeletionFilter filter; filter.created_after_time = now - base::TimeDelta::FromDays(4); filter.created_before_time = now - base::TimeDelta::FromDays(2); @@ -1211,13 +1376,14 @@ filter.excluding_domains = std::vector<std::string>(); filter.excluding_domains->push_back("no.com"); filter.excluding_domains->push_back("yes.com"); + filter.url = GURL("http://nope.com/path"); filter.session_control = network::mojom::CookieDeletionSessionControl::PERSISTENT_COOKIES; // Architectypal cookie: EXPECT_TRUE(SetCanonicalCookie( net::CanonicalCookie( - "A1", "val", "nope.com", "/", now - base::TimeDelta::FromDays(3), + "A1", "val0", "nope.com", "/path", now - base::TimeDelta::FromDays(3), now + base::TimeDelta::FromDays(3), base::Time(), /*secure=*/false, /*httponly=*/false, net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), @@ -1226,7 +1392,7 @@ // Too old cookie. EXPECT_TRUE(SetCanonicalCookie( net::CanonicalCookie( - "A2", "val", "nope.com", "/", now - base::TimeDelta::FromDays(5), + "A2", "val1", "nope.com", "/path", now - base::TimeDelta::FromDays(5), now + base::TimeDelta::FromDays(3), base::Time(), /*secure=*/false, /*httponly=*/false, net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), @@ -1235,7 +1401,7 @@ // Too young cookie. EXPECT_TRUE(SetCanonicalCookie( net::CanonicalCookie( - "A3", "val", "nope.com", "/", now - base::TimeDelta::FromDays(1), + "A3", "val2", "nope.com", "/path", now - base::TimeDelta::FromDays(1), now + base::TimeDelta::FromDays(3), base::Time(), /*secure=*/false, /*httponly=*/false, net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), @@ -1244,7 +1410,8 @@ // Not in including_domains. EXPECT_TRUE(SetCanonicalCookie( net::CanonicalCookie( - "A4", "val", "other.com", "/", now - base::TimeDelta::FromDays(3), + "A4", "val3", "other.com", "/path", + now - base::TimeDelta::FromDays(3), now + base::TimeDelta::FromDays(3), base::Time(), /*secure=*/false, /*httponly=*/false, net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), @@ -1253,7 +1420,17 @@ // In excluding_domains. EXPECT_TRUE(SetCanonicalCookie( net::CanonicalCookie( - "A5", "val", "no.com", "/", now - base::TimeDelta::FromDays(3), + "A5", "val4", "no.com", "/path", now - base::TimeDelta::FromDays(3), + now + base::TimeDelta::FromDays(3), base::Time(), /*secure=*/false, + /*httponly=*/false, net::CookieSameSite::NO_RESTRICTION, + net::COOKIE_PRIORITY_MEDIUM), + true, true)); + + // Doesn't match URL (by path). + EXPECT_TRUE(SetCanonicalCookie( + net::CanonicalCookie( + "A6", "val6", "nope.com", "/otherpath", + now - base::TimeDelta::FromDays(3), now + base::TimeDelta::FromDays(3), base::Time(), /*secure=*/false, /*httponly=*/false, net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), @@ -1262,7 +1439,7 @@ // Session EXPECT_TRUE(SetCanonicalCookie( net::CanonicalCookie( - "A6", "val", "nope.com", "/", now - base::TimeDelta::FromDays(3), + "A7", "val7", "nope.com", "/path", now - base::TimeDelta::FromDays(3), base::Time(), base::Time(), /*secure=*/false, /*httponly=*/false, net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM), true, true)); @@ -1270,13 +1447,14 @@ EXPECT_EQ(1u, service_wrapper()->DeleteCookies(filter)); std::vector<net::CanonicalCookie> cookies = service_wrapper()->GetAllCookies(); - ASSERT_EQ(5u, cookies.size()); + ASSERT_EQ(6u, cookies.size()); std::sort(cookies.begin(), cookies.end(), &CompareCanonicalCookies); EXPECT_EQ("A2", cookies[0].Name()); EXPECT_EQ("A3", cookies[1].Name()); EXPECT_EQ("A4", cookies[2].Name()); EXPECT_EQ("A5", cookies[3].Name()); EXPECT_EQ("A6", cookies[4].Name()); + EXPECT_EQ("A7", cookies[5].Name()); } // Receives and records notifications from the network::mojom::CookieManager.
diff --git a/services/network/public/interfaces/cookie_manager.mojom b/services/network/public/interfaces/cookie_manager.mojom index 03bb615..37dc116a 100644 --- a/services/network/public/interfaces/cookie_manager.mojom +++ b/services/network/public/interfaces/cookie_manager.mojom
@@ -117,6 +117,22 @@ // Deletes cookies whose domains are listed. array<string>? including_domains; + // Delete cookies with a particular name. + string? cookie_name; + + // Delete cookies which match the given URL. + // See https://tools.ietf.org/html/rfc6265, sections 5.1.{3,4} & 5.2.{5,6} + // for matching rules. In general terms, secure cookies only match + // https URLs, the domain must match (the cookie domain must be a suffix + // of the URL domain), and the path must match (the cookie path must + // be a prefix of the URL path). So + // a cookie with {domain: ".sub.example.com", path: "/path", secure} + // would be deleted if the URL passed was + // "https://www.sub.example.com/path/path2" but not if it was + // "http://www.example.com/x"--in fact, that cookie wouldn't be deleted + // if any of the secure/domain/path attributes in the URL were changed. + url.mojom.Url? url; + // Delete session/persistent cookies. CookieDeletionSessionControl session_control = IGNORE_CONTROL; }; @@ -130,9 +146,16 @@ // TODO(rdsmith): Worthwhile specifying a sort order for the getters? // Get all the cookies known to the service. + // Returned cookie list is sorted first by path length (longest first) + // and second by creation time. + // TODO(rdsmith): There are consumers that rely on this behavior, but + // for this function it doesn't make a lot of sense not to also sort + // on origin. Should the returned cookies also be sorted by origin? GetAllCookies() => (array<CanonicalCookie> cookies); // Get all cookies for the specified URL and cookie options. + // Returned cookie list is sorted first by path length (longest first) + // and second by creation time. GetCookieList(url.mojom.Url url, CookieOptions cookie_options) => (array<CanonicalCookie> cookies);
diff --git a/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter b/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter index 63c3253..37bb5fa 100644 --- a/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter +++ b/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
@@ -844,11 +844,6 @@ -PPAPINaClPNaClNonSfiTest.FileRef2 -PPAPINaClPNaClNonSfiTest.URLLoader1 -# Disabled for first phase of extension system shift; will be re-enabled in -# https://chromium-review.googlesource.com/c/chromium/src/+/776017. --ExtensionApiTest.CookiesEventsSpanning --ExtensionApiTest.CookiesEvents - # Prevent tests from making real network requests. # https://crbug.com/793899 -ExtensionApiTest.DoNotOpenUninstallUrlForBlacklistedExtensions
diff --git a/third_party/WebKit/Source/platform/heap/HeapTest.cpp b/third_party/WebKit/Source/platform/heap/HeapTest.cpp index 7da1b93..1a43500 100644 --- a/third_party/WebKit/Source/platform/heap/HeapTest.cpp +++ b/third_party/WebKit/Source/platform/heap/HeapTest.cpp
@@ -213,6 +213,12 @@ visitor->Trace(first); return false; } + + // Incremental marking requires that these objects have a regular tracing + // method that is used for eagerly tracing through them in case they are + // in-place constructed in a container. In this case, we only care about + // strong fields. + void Trace(blink::Visitor* visitor) { visitor->Trace(first); } }; template <typename T>
diff --git a/third_party/WebKit/Source/platform/heap/IncrementalMarkingTest.cpp b/third_party/WebKit/Source/platform/heap/IncrementalMarkingTest.cpp index ea617f5..48a82d5 100644 --- a/third_party/WebKit/Source/platform/heap/IncrementalMarkingTest.cpp +++ b/third_party/WebKit/Source/platform/heap/IncrementalMarkingTest.cpp
@@ -582,6 +582,233 @@ } } +// ============================================================================= +// HeapDeque support. ========================================================== +// ============================================================================= + +TEST(IncrementalMarkingTest, HeapDequePushBackMember) { + Object* obj = Object::Create(); + HeapDeque<Member<Object>> deq; + { + ExpectWriteBarrierFires<Object> scope(ThreadState::Current(), {obj}); + deq.push_back(obj); + } +} + +TEST(IncrementalMarkingTest, HeapDequePushFrontMember) { + Object* obj = Object::Create(); + HeapDeque<Member<Object>> deq; + { + ExpectWriteBarrierFires<Object> scope(ThreadState::Current(), {obj}); + deq.push_front(obj); + } +} + +TEST(IncrementalMarkingTest, HeapDequeEmplaceBackMember) { + Object* obj = Object::Create(); + HeapDeque<Member<Object>> deq; + { + ExpectWriteBarrierFires<Object> scope(ThreadState::Current(), {obj}); + deq.emplace_back(obj); + } +} + +TEST(IncrementalMarkingTest, HeapDequeEmplaceFrontMember) { + Object* obj = Object::Create(); + HeapDeque<Member<Object>> deq; + { + ExpectWriteBarrierFires<Object> scope(ThreadState::Current(), {obj}); + deq.emplace_front(obj); + } +} + +TEST(IncrementalMarkingTest, HeapDequeCopyMember) { + Object* object = Object::Create(); + HeapDeque<Member<Object>> deq1; + deq1.push_back(object); + { + ExpectWriteBarrierFires<Object> scope(ThreadState::Current(), {object}); + HeapDeque<Member<Object>> deq2(deq1); + } +} + +TEST(IncrementalMarkingTest, HeapDequeMoveMember) { + Object* object = Object::Create(); + HeapDeque<Member<Object>> deq1; + deq1.push_back(object); + { + ExpectWriteBarrierFires<Object> scope(ThreadState::Current(), {object}); + HeapDeque<Member<Object>> deq2(std::move(deq1)); + } +} + +TEST(IncrementalMarkingTest, HeapDequeSwapMember) { + Object* obj1 = Object::Create(); + Object* obj2 = Object::Create(); + HeapDeque<Member<Object>> deq1; + deq1.push_back(obj1); + HeapDeque<Member<Object>> deq2; + deq2.push_back(obj2); + { + ExpectWriteBarrierFires<Object> scope(ThreadState::Current(), {obj1, obj2}); + std::swap(deq1, deq2); + } +} + +// ============================================================================= +// HeapHashSet support. ======================================================== +// ============================================================================= + +TEST(IncrementalMarkingTest, HeapHashSetSetMember) { + Object* obj = Object::Create(); + HeapHashSet<Member<Object>> set; + { + ExpectWriteBarrierFires<Object> scope(ThreadState::Current(), {obj}); + set.insert(obj); + } +} + +TEST(IncrementalMarkingTest, HeapHashSetCopyMember) { + Object* obj = Object::Create(); + HeapHashSet<Member<Object>> set1; + set1.insert(obj); + { + ExpectWriteBarrierFires<Object> scope(ThreadState::Current(), {obj}); + HeapHashSet<Member<Object>> set2(set1); + EXPECT_TRUE(set1.Contains(obj)); + EXPECT_TRUE(set2.Contains(obj)); + } +} + +TEST(IncrementalMarkingTest, HeapHashSetMoveMember) { + Object* obj = Object::Create(); + HeapHashSet<Member<Object>> set1; + set1.insert(obj); + { + ExpectWriteBarrierFires<Object> scope(ThreadState::Current(), {obj}); + HeapHashSet<Member<Object>> set2(std::move(set1)); + } +} + +TEST(IncrementalMarkingTest, HeapHashSetSwapMember) { + Object* obj1 = Object::Create(); + Object* obj2 = Object::Create(); + HeapHashSet<Member<Object>> set1; + set1.insert(obj1); + HeapHashSet<Member<Object>> set2; + set2.insert(obj2); + { + ExpectWriteBarrierFires<Object> scope(ThreadState::Current(), {obj1, obj2}); + std::swap(set1, set2); + } +} + +class StrongWeakPair : public std::pair<Member<Object>, WeakMember<Object>> { + DISALLOW_NEW_EXCEPT_PLACEMENT_NEW(); + + typedef std::pair<Member<Object>, WeakMember<Object>> Base; + + public: + StrongWeakPair(Object* obj1, Object* obj2) : Base(obj1, obj2) {} + + StrongWeakPair(WTF::HashTableDeletedValueType) + : Base(WTF::kHashTableDeletedValue, nullptr) {} + + bool IsHashTableDeletedValue() const { + return first.IsHashTableDeletedValue(); + } + + // Trace will be called for write barrier invocations. Only strong members + // are interesting. + void Trace(blink::Visitor* visitor) { visitor->Trace(first); } + + // TraceInCollection will be called for weak processing. + template <typename VisitorDispatcher> + bool TraceInCollection(VisitorDispatcher visitor, + WTF::ShouldWeakPointersBeMarkedStrongly strongify) { + visitor->Trace(first); + if (strongify == WTF::kWeakPointersActStrong) { + visitor->Trace(second); + } + return false; + } +}; + +} // namespace incremental_marking_test +} // namespace blink + +namespace WTF { + +template <> +struct HashTraits<blink::incremental_marking_test::StrongWeakPair> + : SimpleClassHashTraits<blink::incremental_marking_test::StrongWeakPair> { + static const WTF::WeakHandlingFlag kWeakHandlingFlag = + WTF::kWeakHandlingInCollections; + + template <typename U = void> + struct IsTraceableInCollection { + static const bool value = true; + }; + + static const bool kHasIsEmptyValueFunction = true; + static bool IsEmptyValue( + const blink::incremental_marking_test::StrongWeakPair& value) { + return !value.first; + } + + static void ConstructDeletedValue( + blink::incremental_marking_test::StrongWeakPair& slot, + bool) { + new (NotNull, &slot) + blink::incremental_marking_test::StrongWeakPair(kHashTableDeletedValue); + } + + static bool IsDeletedValue( + const blink::incremental_marking_test::StrongWeakPair& value) { + return value.IsHashTableDeletedValue(); + } + + template <typename VisitorDispatcher> + static bool TraceInCollection( + VisitorDispatcher visitor, + blink::incremental_marking_test::StrongWeakPair& t, + WTF::ShouldWeakPointersBeMarkedStrongly strongify) { + return t.TraceInCollection(visitor, strongify); + } +}; + +template <> +struct DefaultHash<blink::incremental_marking_test::StrongWeakPair> { + typedef PairHash<blink::Member<blink::incremental_marking_test::Object>, + blink::WeakMember<blink::incremental_marking_test::Object>> + Hash; +}; + +template <> +struct IsTraceable<blink::incremental_marking_test::StrongWeakPair> { + static const bool value = IsTraceable<std::pair< + blink::Member<blink::incremental_marking_test::Object>, + blink::WeakMember<blink::incremental_marking_test::Object>>>::value; +}; + +} // namespace WTF + +namespace blink { +namespace incremental_marking_test { + +TEST(IncrementalMarkingTest, HeapHashSetStrongWeakPair) { + Object* obj1 = Object::Create(); + Object* obj2 = Object::Create(); + HeapHashSet<StrongWeakPair> set; + { + // Only the strong field in the StrongWeakPair should be hit by the + // write barrier. + ExpectWriteBarrierFires<Object> scope(ThreadState::Current(), {obj1}); + set.insert(StrongWeakPair(obj1, obj2)); + EXPECT_FALSE(obj2->IsMarked()); + } +} + } // namespace incremental_marking_test } // namespace blink
diff --git a/third_party/WebKit/Source/platform/wtf/Deque.h b/third_party/WebKit/Source/platform/wtf/Deque.h index 80ecdeaf..cad6e69 100644 --- a/third_party/WebKit/Source/platform/wtf/Deque.h +++ b/third_party/WebKit/Source/platform/wtf/Deque.h
@@ -37,6 +37,7 @@ #include "base/macros.h" #include "platform/wtf/Allocator.h" +#include "platform/wtf/ConstructTraits.h" #include "platform/wtf/Vector.h" namespace WTF { @@ -498,7 +499,8 @@ end_ = 0; else ++end_; - new (NotNull, new_element) T(std::forward<U>(value)); + ConstructTraits<T, Allocator>::ConstructAndNotifyElement( + new_element, std::forward<U>(value)); } template <typename T, size_t inlineCapacity, typename Allocator> @@ -509,7 +511,8 @@ start_ = buffer_.capacity() - 1; else --start_; - new (NotNull, &buffer_.Buffer()[start_]) T(std::forward<U>(value)); + ConstructTraits<T, Allocator>::ConstructAndNotifyElement( + &buffer_.Buffer()[start_], std::forward<U>(value)); } template <typename T, size_t inlineCapacity, typename Allocator> @@ -521,7 +524,8 @@ end_ = 0; else ++end_; - new (NotNull, new_element) T(std::forward<Args>(args)...); + ConstructTraits<T, Allocator>::ConstructAndNotifyElement( + new_element, std::forward<Args>(args)...); } template <typename T, size_t inlineCapacity, typename Allocator> @@ -532,7 +536,8 @@ start_ = buffer_.capacity() - 1; else --start_; - new (NotNull, &buffer_.Buffer()[start_]) T(std::forward<Args>(args)...); + ConstructTraits<T, Allocator>::ConstructAndNotifyElement( + &buffer_.Buffer()[start_], std::forward<Args>(args)...); } template <typename T, size_t inlineCapacity, typename Allocator>
diff --git a/third_party/WebKit/Source/platform/wtf/HashTable.h b/third_party/WebKit/Source/platform/wtf/HashTable.h index 61bf08da..855ea16 100644 --- a/third_party/WebKit/Source/platform/wtf/HashTable.h +++ b/third_party/WebKit/Source/platform/wtf/HashTable.h
@@ -23,14 +23,16 @@ #ifndef WTF_HashTable_h #define WTF_HashTable_h +#include <memory> + #include "platform/wtf/Alignment.h" #include "platform/wtf/Allocator.h" #include "platform/wtf/Assertions.h" #include "platform/wtf/ConditionalDestructor.h" +#include "platform/wtf/ConstructTraits.h" #include "platform/wtf/HashTraits.h" #include "platform/wtf/PtrUtil.h" #include "platform/wtf/allocator/PartitionAllocator.h" -#include <memory> #if !defined(DUMP_HASHTABLE_STATS) #define DUMP_HASHTABLE_STATS 0 @@ -526,7 +528,8 @@ STATIC_ONLY(Mover); static void Move(T&& from, T& to) { to.~T(); - new (NotNull, &to) T(std::move(from)); + ConstructTraits<T, Allocator>::ConstructAndNotifyElement(&to, + std::move(from)); } }; @@ -536,12 +539,13 @@ static void Move(T&& from, T& to) { to.~T(); Allocator::EnterGCForbiddenScope(); - new (NotNull, &to) T(std::move(from)); + ConstructTraits<T, Allocator>::ConstructAndNotifyElement(&to, + std::move(from)); Allocator::LeaveGCForbiddenScope(); } }; -template <typename HashFunctions> +template <typename HashFunctions, typename Allocator> class IdentityHashTranslator { STATIC_ONLY(IdentityHashTranslator); @@ -557,6 +561,7 @@ template <typename T, typename U, typename V> static void Translate(T& location, U&&, V&& value) { location = std::forward<V>(value); + ConstructTraits<T, Allocator>::NotifyNewElements(&location, 1); } }; @@ -680,7 +685,8 @@ typedef Value ValueType; typedef Extractor ExtractorType; typedef KeyTraits KeyTraitsType; - typedef IdentityHashTranslator<HashFunctions> IdentityTranslatorType; + typedef IdentityHashTranslator<HashFunctions, Allocator> + IdentityTranslatorType; typedef HashTableAddResult<HashTable, ValueType> AddResult; HashTable(); @@ -1191,16 +1197,17 @@ template <> struct HashTableBucketInitializer<false> { STATIC_ONLY(HashTableBucketInitializer); - template <typename Traits, typename Value> + template <typename Traits, typename Allocator, typename Value> static void Initialize(Value& bucket) { - new (NotNull, &bucket) Value(Traits::EmptyValue()); + ConstructTraits<Value, Allocator>::ConstructAndNotifyElement( + &bucket, Traits::EmptyValue()); } }; template <> struct HashTableBucketInitializer<true> { STATIC_ONLY(HashTableBucketInitializer); - template <typename Traits, typename Value> + template <typename Traits, typename Allocator, typename Value> static void Initialize(Value& bucket) { // This initializes the bucket without copying the empty value. That // makes it possible to use this with types that don't support copying. @@ -1221,7 +1228,7 @@ HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits, Allocator>:: InitializeBucket(ValueType& bucket) { HashTableBucketInitializer<Traits::kEmptyValueIsZero>::template Initialize< - Traits>(bucket); + Traits, Allocator>(bucket); } template <typename Key, @@ -1908,6 +1915,15 @@ #if DUMP_HASHTABLE_STATS_PER_TABLE HashTableStatsPtr<Allocator>::swap(stats_, other.stats_); #endif + + if (table_) { + ConstructTraits<ValueType, Allocator>::NotifyNewElements(table_, + table_size_); + } + if (other.table_) { + ConstructTraits<ValueType, Allocator>::NotifyNewElements(other.table_, + other.table_size_); + } } template <typename Key,