From 5eb0a89579bc6903b23f816c4268bd44c5ea0650 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Tue, 28 Nov 2017 12:53:42 +0530 Subject: [PATCH] REVIEW: let browser context manage cookie change sub list --- atom/browser/api/atom_api_cookies.cc | 11 ++++----- atom/browser/api/atom_api_cookies.h | 14 ++++++----- atom/browser/atom_browser_context.cc | 15 ++++++++++++ atom/browser/atom_browser_context.h | 11 +++++++++ .../browser/net/cookie_details.h | 10 ++++---- .../browser/url_request_context_getter.cc | 24 +++++-------------- .../browser/url_request_context_getter.h | 13 +++------- 7 files changed, 53 insertions(+), 45 deletions(-) rename {brightray => atom}/browser/net/cookie_details.h (73%) diff --git a/atom/browser/api/atom_api_cookies.cc b/atom/browser/api/atom_api_cookies.cc index 0ba537147336..b0ecbea4fef1 100644 --- a/atom/browser/api/atom_api_cookies.cc +++ b/atom/browser/api/atom_api_cookies.cc @@ -238,12 +238,11 @@ void SetCookieOnIO(scoped_refptr getter, } // namespace Cookies::Cookies(v8::Isolate* isolate, AtomBrowserContext* browser_context) - : request_context_getter_(browser_context->url_request_context_getter()) { + : browser_context_(browser_context), + request_context_getter_(browser_context->url_request_context_getter()) { Init(isolate); - cookie_change_subscription_ = - browser_context->url_request_context_getter() - ->RegisterCookieChangeCallback( - base::Bind(&Cookies::OnCookieChanged, base::Unretained(this))); + cookie_change_subscription_ = browser_context->RegisterCookieChangeCallback( + base::Bind(&Cookies::OnCookieChanged, base::Unretained(this))); } Cookies::~Cookies() {} @@ -281,7 +280,7 @@ void Cookies::FlushStore(const base::Closure& callback) { base::Bind(FlushCookieStoreOnIOThread, getter, callback)); } -void Cookies::OnCookieChanged(const brightray::CookieDetails* details) { +void Cookies::OnCookieChanged(const CookieDetails* details) { Emit("changed", *(details->cookie), details->cause, details->removed); } diff --git a/atom/browser/api/atom_api_cookies.h b/atom/browser/api/atom_api_cookies.h index 47922f343e42..b1fb9e0aaf74 100644 --- a/atom/browser/api/atom_api_cookies.h +++ b/atom/browser/api/atom_api_cookies.h @@ -8,8 +8,8 @@ #include #include "atom/browser/api/trackable_object.h" +#include "atom/browser/net/cookie_details.h" #include "base/callback.h" -#include "brightray/browser/net/cookie_details.h" #include "native_mate/handle.h" #include "net/cookies/canonical_cookie.h" @@ -54,15 +54,17 @@ class Cookies : public mate::TrackableObject { void Set(const base::DictionaryValue& details, const SetCallback& callback); void FlushStore(const base::Closure& callback); - // brightray::URLRequestContextGetter subscription: - void OnCookieChanged(const brightray::CookieDetails*); + // AtomBrowserContext::RegisterCookieChangeCallback subscription: + void OnCookieChanged(const CookieDetails*); private: - net::URLRequestContextGetter* request_context_getter_; - std::unique_ptr< - base::CallbackList::Subscription> + // Store a reference to ensure this class gets destroyed before the context. + scoped_refptr browser_context_; + std::unique_ptr::Subscription> cookie_change_subscription_; + net::URLRequestContextGetter* request_context_getter_; + DISALLOW_COPY_AND_ASSIGN(Cookies); }; diff --git a/atom/browser/atom_browser_context.cc b/atom/browser/atom_browser_context.cc index 33ff92d2f135..bd69d8bc78c2 100644 --- a/atom/browser/atom_browser_context.cc +++ b/atom/browser/atom_browser_context.cc @@ -15,6 +15,7 @@ #include "atom/browser/net/atom_cert_verifier.h" #include "atom/browser/net/atom_network_delegate.h" #include "atom/browser/net/atom_url_request_job_factory.h" +#include "atom/browser/net/cookie_details.h" #include "atom/browser/net/http_protocol_handler.h" #include "atom/browser/web_view_manager.h" #include "atom/common/atom_version.h" @@ -102,6 +103,12 @@ void AtomBrowserContext::SetUserAgent(const std::string& user_agent) { user_agent_ = user_agent; } +std::unique_ptr::Subscription> +AtomBrowserContext::RegisterCookieChangeCallback( + const base::Callback& cb) { + return cookie_change_sub_list_.Add(cb); +} + std::unique_ptr AtomBrowserContext::CreateNetworkDelegate() { return base::MakeUnique(); @@ -198,6 +205,14 @@ std::vector AtomBrowserContext::GetCookieableSchemes() { return default_schemes; } +void AtomBrowserContext::NotifyCookieChange( + const net::CanonicalCookie& cookie, + bool removed, + net::CookieStore::ChangeCause cause) { + CookieDetails cookie_details(&cookie, removed, cause); + cookie_change_sub_list_.Notify(&cookie_details); +} + void AtomBrowserContext::RegisterPrefs(PrefRegistrySimple* pref_registry) { pref_registry->RegisterFilePathPref(prefs::kSelectFileLastDirectory, base::FilePath()); diff --git a/atom/browser/atom_browser_context.h b/atom/browser/atom_browser_context.h index 8f99a5e74107..c892f2b6a22a 100644 --- a/atom/browser/atom_browser_context.h +++ b/atom/browser/atom_browser_context.h @@ -8,6 +8,7 @@ #include #include +#include "base/callback_list.h" #include "brightray/browser/browser_context.h" namespace atom { @@ -17,6 +18,7 @@ class AtomDownloadManagerDelegate; class AtomNetworkDelegate; class AtomPermissionManager; class WebViewManager; +struct CookieDetails; class AtomBrowserContext : public brightray::BrowserContext { public: @@ -28,6 +30,10 @@ class AtomBrowserContext : public brightray::BrowserContext { const base::DictionaryValue& options = base::DictionaryValue()); void SetUserAgent(const std::string& user_agent); + // Register callbacks that needs to notified on any cookie store changes. + std::unique_ptr::Subscription> + RegisterCookieChangeCallback( + const base::Callback& cb); // brightray::URLRequestContextGetter::Delegate: std::unique_ptr CreateNetworkDelegate() override; @@ -39,6 +45,9 @@ class AtomBrowserContext : public brightray::BrowserContext { std::unique_ptr CreateCertVerifier( brightray::RequireCTDelegate* ct_delegate) override; std::vector GetCookieableSchemes() override; + void NotifyCookieChange(const net::CanonicalCookie& cookie, + bool removed, + net::CookieStore::ChangeCause cause) override; // content::BrowserContext: content::DownloadManagerDelegate* GetDownloadManagerDelegate() override; @@ -63,6 +72,8 @@ class AtomBrowserContext : public brightray::BrowserContext { std::string user_agent_; bool use_cache_; + base::CallbackList cookie_change_sub_list_; + DISALLOW_COPY_AND_ASSIGN(AtomBrowserContext); }; diff --git a/brightray/browser/net/cookie_details.h b/atom/browser/net/cookie_details.h similarity index 73% rename from brightray/browser/net/cookie_details.h rename to atom/browser/net/cookie_details.h index 42a5b7eb6b6d..5103836dcb6c 100644 --- a/brightray/browser/net/cookie_details.h +++ b/atom/browser/net/cookie_details.h @@ -2,13 +2,13 @@ // Use of this source code is governed by the MIT license that can be // found in the LICENSE file. -#ifndef BRIGHTRAY_BROWSER_NET_COOKIE_DETAILS_H_ -#define BRIGHTRAY_BROWSER_NET_COOKIE_DETAILS_H_ +#ifndef ATOM_BROWSER_NET_COOKIE_DETAILS_H_ +#define ATOM_BROWSER_NET_COOKIE_DETAILS_H_ #include "base/macros.h" #include "net/cookies/cookie_store.h" -namespace brightray { +namespace atom { struct CookieDetails { public: @@ -22,6 +22,6 @@ struct CookieDetails { net::CookieStore::ChangeCause cause; }; -} // namespace brightray +} // namespace atom -#endif // BRIGHTRAY_BROWSER_NET_COOKIE_DETAILS_H_ +#endif // ATOM_BROWSER_NET_COOKIE_DETAILS_H_ diff --git a/brightray/browser/url_request_context_getter.cc b/brightray/browser/url_request_context_getter.cc index 70d19afce62f..6d9f8a2dab6e 100644 --- a/brightray/browser/url_request_context_getter.cc +++ b/brightray/browser/url_request_context_getter.cc @@ -13,7 +13,6 @@ #include "base/threading/sequenced_worker_pool.h" #include "base/threading/worker_pool.h" #include "brightray/browser/browser_client.h" -#include "brightray/browser/net/cookie_details.h" #include "brightray/browser/net/devtools_network_controller_handle.h" #include "brightray/browser/net/devtools_network_transaction_factory.h" #include "brightray/browser/net/require_ct_delegate.h" @@ -155,30 +154,19 @@ URLRequestContextGetter::URLRequestContextGetter( URLRequestContextGetter::~URLRequestContextGetter() { } -std::unique_ptr::Subscription> -URLRequestContextGetter::RegisterCookieChangeCallback( - const base::Callback& cb) { - return cookie_change_sub_list_.Add(cb); -} - -void URLRequestContextGetter::NotifyCookieChange( - const net::CanonicalCookie& cookie, - bool removed, - net::CookieStore::ChangeCause cause) { - CookieDetails cookie_details(&cookie, removed, cause); - cookie_change_sub_list_.Notify(&cookie_details); -} - void URLRequestContextGetter::OnCookieChanged( const net::CanonicalCookie& cookie, net::CookieStore::ChangeCause cause) { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + if (!delegate_) + return; + content::BrowserThread::PostTask( content::BrowserThread::UI, FROM_HERE, - base::BindOnce(&URLRequestContextGetter::NotifyCookieChange, this, cookie, - !(cause == net::CookieStore::ChangeCause::INSERTED), - cause)); + base::BindOnce( + &Delegate::NotifyCookieChange, base::Unretained(delegate_), cookie, + !(cause == net::CookieStore::ChangeCause::INSERTED), cause)); } net::HostResolver* URLRequestContextGetter::host_resolver() { diff --git a/brightray/browser/url_request_context_getter.h b/brightray/browser/url_request_context_getter.h index 880473ee7e5d..b0a2549280e8 100644 --- a/brightray/browser/url_request_context_getter.h +++ b/brightray/browser/url_request_context_getter.h @@ -8,7 +8,6 @@ #include #include -#include "base/callback_list.h" #include "base/files/file_path.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/content_browser_client.h" @@ -38,7 +37,6 @@ namespace brightray { class RequireCTDelegate; class DevToolsNetworkControllerHandle; class NetLog; -struct CookieDetails; class URLRequestContextGetter : public net::URLRequestContextGetter { public: @@ -59,6 +57,9 @@ class URLRequestContextGetter : public net::URLRequestContextGetter { RequireCTDelegate* ct_delegate); virtual net::SSLConfigService* CreateSSLConfigService(); virtual std::vector GetCookieableSchemes(); + virtual void NotifyCookieChange(const net::CanonicalCookie& cookie, + bool removed, + net::CookieStore::ChangeCause cause) {} }; URLRequestContextGetter( @@ -72,13 +73,6 @@ class URLRequestContextGetter : public net::URLRequestContextGetter { content::URLRequestInterceptorScopedVector protocol_interceptors); virtual ~URLRequestContextGetter(); - // Register callbacks that needs to notified on any cookie store changes. - std::unique_ptr::Subscription> - RegisterCookieChangeCallback( - const base::Callback& cb); - void NotifyCookieChange(const net::CanonicalCookie& cookie, - bool removed, - net::CookieStore::ChangeCause cause); // net::CookieStore::CookieChangedCallback implementation. void OnCookieChanged(const net::CanonicalCookie& cookie, net::CookieStore::ChangeCause cause); @@ -119,7 +113,6 @@ class URLRequestContextGetter : public net::URLRequestContextGetter { content::ProtocolHandlerMap protocol_handlers_; content::URLRequestInterceptorScopedVector protocol_interceptors_; - base::CallbackList cookie_change_sub_list_; net::URLRequestJobFactory* job_factory_; // weak ref DISALLOW_COPY_AND_ASSIGN(URLRequestContextGetter);