From 869ae21c9feeb3d9d61bb0f2f8d3e65a2ddd26d0 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Fri, 27 Mar 2020 16:00:36 -0400 Subject: [PATCH] Convert CefCookieManagerImpl to use CefBrowserContext::Getter. With this change CefCookieManagerImpl no longer keeps a reference to the originating CefRequestContextImpl. This means that the CefRequestContextImpl can be destroyed if all other references are released while the CefCookieManagerImpl exists. If CefRequestContextImpl destruction results in the underlying CefBrowserContext being destroyed then the CefCookieManagerImpl's reference to that CefBrowserContext will be invalidated. This is the same ownership model introduced with CefMediaRouterImpl in the previous commit. --- .../net_service/cookie_manager_impl.cc | 102 ++++++++++++------ .../browser/net_service/cookie_manager_impl.h | 16 +-- libcef/browser/request_context_impl.cc | 18 +++- libcef/browser/request_context_impl.h | 4 + 4 files changed, 99 insertions(+), 41 deletions(-) diff --git a/libcef/browser/net_service/cookie_manager_impl.cc b/libcef/browser/net_service/cookie_manager_impl.cc index bdd71b915..60dcd3f81 100644 --- a/libcef/browser/net_service/cookie_manager_impl.cc +++ b/libcef/browser/net_service/cookie_manager_impl.cc @@ -18,11 +18,19 @@ using network::mojom::CookieManager; namespace { -// Do not keep a reference to the CookieManager returned by this method. -CookieManager* GetCookieManager(CefRequestContextImpl* request_context) { +// Do not keep a reference to the object returned by this method. +CefBrowserContext* GetBrowserContext(const CefBrowserContext::Getter& getter) { CEF_REQUIRE_UIT(); - return content::BrowserContext::GetDefaultStoragePartition( - request_context->GetBrowserContext()) + DCHECK(!getter.is_null()); + + // Will return nullptr if the BrowserContext has been destroyed. + return getter.Run(); +} + +// Do not keep a reference to the object returned by this method. +CookieManager* GetCookieManager(CefBrowserContext* browser_context) { + CEF_REQUIRE_UIT(); + return content::BrowserContext::GetDefaultStoragePartition(browser_context) ->GetCookieManagerForBrowserProcess(); } @@ -53,11 +61,15 @@ void DeleteCookiesCallbackImpl(CefRefPtr callback, } void ExecuteVisitor(CefRefPtr visitor, - CefRefPtr request_context, + const CefBrowserContext::Getter& browser_context_getter, const std::vector& cookies) { CEF_REQUIRE_UIT(); - CookieManager* cookie_manager = GetCookieManager(request_context.get()); + auto browser_context = GetBrowserContext(browser_context_getter); + if (!browser_context) + return; + + auto cookie_manager = GetCookieManager(browser_context); int total = cookies.size(), count = 0; for (const auto& cc : cookies) { @@ -77,22 +89,24 @@ void ExecuteVisitor(CefRefPtr visitor, } // Always execute the callback asynchronously. -void GetAllCookiesCallbackImpl(CefRefPtr visitor, - CefRefPtr request_context, - const net::CookieList& cookies) { - CEF_POST_TASK(CEF_UIT, - base::Bind(&ExecuteVisitor, visitor, request_context, cookies)); +void GetAllCookiesCallbackImpl( + CefRefPtr visitor, + const CefBrowserContext::Getter& browser_context_getter, + const net::CookieList& cookies) { + CEF_POST_TASK(CEF_UIT, base::Bind(&ExecuteVisitor, visitor, + browser_context_getter, cookies)); } -void GetCookiesCallbackImpl(CefRefPtr visitor, - CefRefPtr request_context, - const net::CookieStatusList& include_cookies, - const net::CookieStatusList&) { +void GetCookiesCallbackImpl( + CefRefPtr visitor, + const CefBrowserContext::Getter& browser_context_getter, + const net::CookieStatusList& include_cookies, + const net::CookieStatusList&) { net::CookieList cookies; for (const auto& status : include_cookies) { cookies.push_back(status.cookie); } - GetAllCookiesCallbackImpl(visitor, request_context, cookies); + GetAllCookiesCallbackImpl(visitor, browser_context_getter, cookies); } } // namespace @@ -100,11 +114,12 @@ void GetCookiesCallbackImpl(CefRefPtr visitor, CefCookieManagerImpl::CefCookieManagerImpl() {} void CefCookieManagerImpl::Initialize( - CefRefPtr request_context, + CefBrowserContext::Getter browser_context_getter, CefRefPtr callback) { - DCHECK(request_context); - DCHECK(!request_context_); - request_context_ = request_context; + CEF_REQUIRE_UIT(); + DCHECK(!browser_context_getter.is_null()); + DCHECK(browser_context_getter_.is_null()); + browser_context_getter_ = browser_context_getter; RunAsyncCompletionOnUIThread(callback); } @@ -132,11 +147,14 @@ void CefCookieManagerImpl::SetSupportedSchemes( all_schemes.push_back("wss"); } + auto browser_context = GetBrowserContext(browser_context_getter_); + if (!browser_context) + return; + // This will be forwarded to the CookieMonster that lives in the // NetworkService process when the NetworkContext is created via // CefContentBrowserClient::CreateNetworkContext. - request_context_->GetBrowserContext()->set_cookieable_schemes( - base::make_optional(all_schemes)); + browser_context->set_cookieable_schemes(base::make_optional(all_schemes)); RunAsyncCompletionOnUIThread(callback); } @@ -153,9 +171,13 @@ bool CefCookieManagerImpl::VisitAllCookies( return true; } - GetCookieManager(request_context_.get()) - ->GetAllCookies( - base::Bind(&GetAllCookiesCallbackImpl, visitor, request_context_)); + auto browser_context = GetBrowserContext(browser_context_getter_); + if (!browser_context) + return false; + + GetCookieManager(browser_context) + ->GetAllCookies(base::Bind(&GetAllCookiesCallbackImpl, visitor, + browser_context_getter_)); return true; } @@ -182,10 +204,14 @@ bool CefCookieManagerImpl::VisitUrlCookies( if (includeHttpOnly) options.set_include_httponly(); - GetCookieManager(request_context_.get()) - ->GetCookieList( - gurl, options, - base::Bind(&GetCookiesCallbackImpl, visitor, request_context_)); + auto browser_context = GetBrowserContext(browser_context_getter_); + if (!browser_context) + return false; + + GetCookieManager(browser_context) + ->GetCookieList(gurl, options, + base::Bind(&GetCookiesCallbackImpl, visitor, + browser_context_getter_)); return true; } @@ -233,7 +259,11 @@ bool CefCookieManagerImpl::SetCookie(const CefString& url, if (cookie.httponly) options.set_include_httponly(); - GetCookieManager(request_context_.get()) + auto browser_context = GetBrowserContext(browser_context_getter_); + if (!browser_context) + return false; + + GetCookieManager(browser_context) ->SetCanonicalCookie(*canonical_cookie, gurl.scheme(), options, base::Bind(SetCookieCallbackImpl, callback)); return true; @@ -270,7 +300,11 @@ bool CefCookieManagerImpl::DeleteCookies( deletion_filter->cookie_name = cookie_name; } - GetCookieManager(request_context_.get()) + auto browser_context = GetBrowserContext(browser_context_getter_); + if (!browser_context) + return false; + + GetCookieManager(browser_context) ->DeleteCookies(std::move(deletion_filter), base::Bind(DeleteCookiesCallbackImpl, callback)); return true; @@ -286,7 +320,11 @@ bool CefCookieManagerImpl::FlushStore( return true; } - GetCookieManager(request_context_.get()) + auto browser_context = GetBrowserContext(browser_context_getter_); + if (!browser_context) + return false; + + GetCookieManager(browser_context) ->FlushCookieStore(base::Bind(RunAsyncCompletionOnUIThread, callback)); return true; } diff --git a/libcef/browser/net_service/cookie_manager_impl.h b/libcef/browser/net_service/cookie_manager_impl.h index baa4ccbcc..19eff86d8 100644 --- a/libcef/browser/net_service/cookie_manager_impl.h +++ b/libcef/browser/net_service/cookie_manager_impl.h @@ -6,19 +6,20 @@ #define CEF_LIBCEF_BROWSER_NET_SERVICE_COOKIE_MANAGER_IMPL_H_ #include "include/cef_cookie.h" -#include "libcef/browser/request_context_impl.h" +#include "libcef/browser/browser_context.h" #include "libcef/browser/thread_util.h" #include "base/files/file_path.h" -// Implementation of the CefCookieManager interface. +// Implementation of the CefCookieManager interface. May be created on any +// thread. class CefCookieManagerImpl : public CefCookieManager { public: CefCookieManagerImpl(); - // Must be called immediately after this object is created when |is_blocking| - // is false. - void Initialize(CefRefPtr request_context, + // Called on the UI thread after object creation and before any other object + // methods are executed on the UI thread. + void Initialize(CefBrowserContext::Getter browser_context_getter, CefRefPtr callback); // CefCookieManager methods. @@ -38,10 +39,11 @@ class CefCookieManagerImpl : public CefCookieManager { bool FlushStore(CefRefPtr callback) override; private: - // Context that owns the cookie manager. - CefRefPtr request_context_; + // Only accessed on the UI thread. Will be non-null after Initialize(). + CefBrowserContext::Getter browser_context_getter_; IMPLEMENT_REFCOUNTING(CefCookieManagerImpl); + DISALLOW_COPY_AND_ASSIGN(CefCookieManagerImpl); }; #endif // CEF_LIBCEF_BROWSER_NET_SERVICE_COOKIE_MANAGER_IMPL_H_ diff --git a/libcef/browser/request_context_impl.cc b/libcef/browser/request_context_impl.cc index cc62313a9..52eb37fbf 100644 --- a/libcef/browser/request_context_impl.cc +++ b/libcef/browser/request_context_impl.cc @@ -7,7 +7,6 @@ #include "libcef/browser/content_browser_client.h" #include "libcef/browser/context.h" #include "libcef/browser/extensions/extension_system.h" -#include "libcef/browser/net_service/cookie_manager_impl.h" #include "libcef/browser/thread_util.h" #include "libcef/common/extensions/extensions_util.h" #include "libcef/common/task_runner_impl.h" @@ -315,7 +314,7 @@ CefString CefRequestContextImpl::GetCachePath() { CefRefPtr CefRequestContextImpl::GetCookieManager( CefRefPtr callback) { CefRefPtr cookie_manager = new CefCookieManagerImpl(); - cookie_manager->Initialize(this, callback); + InitializeCookieManagerOnUIThread(cookie_manager, callback); return cookie_manager.get(); } @@ -767,6 +766,21 @@ void CefRequestContextImpl::ResolveHostInternal( helper->Start(browser_context, origin); } +void CefRequestContextImpl::InitializeCookieManagerOnUIThread( + CefRefPtr cookie_manager, + CefRefPtr callback) { + if (!CEF_CURRENTLY_ON_UIT()) { + CEF_POST_TASK( + CEF_UIT, + base::Bind(&CefRequestContextImpl::InitializeCookieManagerOnUIThread, + this, cookie_manager, callback)); + return; + } + + auto browser_context = GetBrowserContext(); + cookie_manager->Initialize(browser_context->getter(), callback); +} + void CefRequestContextImpl::InitializeMediaRouterOnUIThread( CefRefPtr media_router) { if (!CEF_CURRENTLY_ON_UIT()) { diff --git a/libcef/browser/request_context_impl.h b/libcef/browser/request_context_impl.h index f99ffe613..a802b26b7 100644 --- a/libcef/browser/request_context_impl.h +++ b/libcef/browser/request_context_impl.h @@ -9,6 +9,7 @@ #include "include/cef_request_context.h" #include "libcef/browser/browser_context.h" #include "libcef/browser/media_router/media_router_impl.h" +#include "libcef/browser/net_service/cookie_manager_impl.h" #include "libcef/browser/thread_util.h" class CefBrowserContext; @@ -149,6 +150,9 @@ class CefRequestContextImpl : public CefRequestContext { CefRefPtr callback, CefBrowserContext* browser_context); + void InitializeCookieManagerOnUIThread( + CefRefPtr cookie_manager, + CefRefPtr callback); void InitializeMediaRouterOnUIThread( CefRefPtr media_router);