From 5b9fb7e4ec0740fc1ccea663a202e3b43d064bda Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Thu, 4 May 2017 17:53:27 -0400 Subject: [PATCH] Popups must share the parent context to avoid crashes on parent browser destruction (issue #2162) --- libcef/browser/browser_context.h | 7 + libcef/browser/browser_context_impl.cc | 35 +- libcef/browser/browser_context_impl.h | 16 +- libcef/browser/browser_context_proxy.cc | 8 +- libcef/browser/browser_context_proxy.h | 7 +- libcef/browser/browser_host_impl.cc | 8 +- libcef/browser/browser_main.cc | 14 +- libcef/browser/browser_main.h | 8 +- libcef/browser/chrome_profile_manager_stub.cc | 3 +- libcef/browser/content_browser_client.cc | 5 +- libcef/browser/content_browser_client.h | 4 +- libcef/browser/prefs/browser_prefs.cc | 2 + libcef/browser/request_context_impl.cc | 124 +++---- libcef/browser/request_context_impl.h | 16 +- tests/ceftests/navigation_unittest.cc | 223 ------------- tests/ceftests/request_context_unittest.cc | 313 +++++++++++++++--- tests/ceftests/test_handler.cc | 45 ++- 17 files changed, 448 insertions(+), 390 deletions(-) diff --git a/libcef/browser/browser_context.h b/libcef/browser/browser_context.h index 87d3ca3bb..4302fc734 100644 --- a/libcef/browser/browser_context.h +++ b/libcef/browser/browser_context.h @@ -118,6 +118,7 @@ // CefURLRequestContextGetter* destruction. */ +class CefRequestContextImpl; class HostContentSettingsMap; class PrefService; @@ -146,6 +147,12 @@ class CefBrowserContext : public ChromeProfileStub { // Profile methods. ChromeZoomLevelPrefs* GetZoomLevelPrefs() override; + // Returns a RequestContext associated with this object. If this object is a + // *Proxy then it will return the single associated proxy RequestContext. If + // this object is an *Impl then it will return the first non-proxy + // RequestContext, if one exists, otherwise the first proxy RequestContext. + virtual CefRequestContextImpl* GetCefRequestContext() const = 0; + // Returns the settings associated with this object. Safe to call from any // thread. virtual const CefRequestContextSettings& GetSettings() const = 0; diff --git a/libcef/browser/browser_context_impl.cc b/libcef/browser/browser_context_impl.cc index 04095b6c2..56b0855a2 100644 --- a/libcef/browser/browser_context_impl.cc +++ b/libcef/browser/browser_context_impl.cc @@ -14,6 +14,7 @@ #include "libcef/browser/extensions/extension_system.h" #include "libcef/browser/permissions/permission_manager.h" #include "libcef/browser/prefs/browser_prefs.h" +#include "libcef/browser/request_context_impl.h" #include "libcef/browser/ssl_host_state_delegate.h" #include "libcef/browser/thread_util.h" #include "libcef/common/cef_switches.h" @@ -220,7 +221,7 @@ CefBrowserContextImpl::~CefBrowserContextImpl() { CEF_REQUIRE_UIT(); // No CefRequestContextImpl should be referencing this object any longer. - DCHECK_EQ(request_context_count_, 0); + DCHECK(request_context_set_.empty()); // Unregister the context first to avoid re-entrancy during shutdown. g_manager.Get().RemoveImpl(this, cache_path_); @@ -324,21 +325,33 @@ void CefBrowserContextImpl::RemoveProxy(const CefBrowserContextProxy* proxy) { visitedlink_listener_->RemoveListenerForContext(proxy); } -void CefBrowserContextImpl::AddRequestContext() { +void CefBrowserContextImpl::AddCefRequestContext( + CefRequestContextImpl* context) { CEF_REQUIRE_UIT(); - request_context_count_++; + request_context_set_.insert(context); } -void CefBrowserContextImpl::RemoveRequestContext() { +void CefBrowserContextImpl::RemoveCefRequestContext( + CefRequestContextImpl* context) { CEF_REQUIRE_UIT(); - request_context_count_--; - DCHECK_GE(request_context_count_, 0); + request_context_set_.erase(context); - // Delete non-global contexts when the reference count reaches zero. - if (request_context_count_ == 0 && - this != CefContentBrowserClient::Get()->browser_context()) { + // Delete ourselves when the reference count reaches zero. + if (request_context_set_.empty()) delete this; +} + +CefRequestContextImpl* CefBrowserContextImpl::GetCefRequestContext( + bool impl_only) const { + CEF_REQUIRE_UIT(); + // First try to find a non-proxy RequestContext. + for (CefRequestContextImpl* impl : request_context_set_) { + if (!impl->GetHandler()) + return impl; } + if (impl_only) + return nullptr; + return *request_context_set_.begin(); } // static @@ -479,6 +492,10 @@ const PrefService* CefBrowserContextImpl::GetPrefs() const { return pref_service_.get(); } +CefRequestContextImpl* CefBrowserContextImpl::GetCefRequestContext() const { + return GetCefRequestContext(false); +} + const CefRequestContextSettings& CefBrowserContextImpl::GetSettings() const { return settings_; } diff --git a/libcef/browser/browser_context_impl.h b/libcef/browser/browser_context_impl.h index 5539364ca..cd438264d 100644 --- a/libcef/browser/browser_context_impl.h +++ b/libcef/browser/browser_context_impl.h @@ -52,10 +52,11 @@ class CefBrowserContextImpl : public CefBrowserContext, void AddProxy(const CefBrowserContextProxy* proxy); void RemoveProxy(const CefBrowserContextProxy* proxy); - // Track associated CefRequestContextImpl objects. If this object is a non- - // global context then it will delete itself when the count reaches zero. - void AddRequestContext(); - void RemoveRequestContext(); + // Track associated CefRequestContextImpl objects. This object will delete + // itself when the count reaches zero. + void AddCefRequestContext(CefRequestContextImpl* context); + void RemoveCefRequestContext(CefRequestContextImpl* context); + CefRequestContextImpl* GetCefRequestContext(bool impl_only) const; // BrowserContext methods. base::FilePath GetPath() const override; @@ -88,6 +89,7 @@ class CefBrowserContextImpl : public CefBrowserContext, const PrefService* GetPrefs() const override; // CefBrowserContext methods. + CefRequestContextImpl* GetCefRequestContext() const override; const CefRequestContextSettings& GetSettings() const override; CefRefPtr GetHandler() const override; HostContentSettingsMap* GetHostContentSettingsMap() override; @@ -97,7 +99,7 @@ class CefBrowserContextImpl : public CefBrowserContext, void RebuildTable(const scoped_refptr& enumerator) override; // Guaranteed to exist once this object has been initialized. - scoped_refptr request_context() const { + scoped_refptr request_context_getter() const { return url_request_getter_; } @@ -111,8 +113,8 @@ class CefBrowserContextImpl : public CefBrowserContext, CefRequestContextSettings settings_; base::FilePath cache_path_; - // Number of CefRequestContextImpl objects referencing this object. - int request_context_count_ = 0; + // CefRequestContextImpl objects referencing this object. + std::set request_context_set_; std::unique_ptr pref_service_; std::unique_ptr pref_proxy_config_tracker_; diff --git a/libcef/browser/browser_context_proxy.cc b/libcef/browser/browser_context_proxy.cc index bc085144c..c930a169b 100644 --- a/libcef/browser/browser_context_proxy.cc +++ b/libcef/browser/browser_context_proxy.cc @@ -58,9 +58,11 @@ bool ShouldProxyUserData(const void* key) { } // namespace CefBrowserContextProxy::CefBrowserContextProxy( + CefRequestContextImpl* const request_context, CefRefPtr handler, CefBrowserContextImpl* parent) : CefBrowserContext(true), + request_context_(request_context), handler_(handler), parent_(parent) { DCHECK(handler_.get()); @@ -197,6 +199,10 @@ const PrefService* CefBrowserContextProxy::GetPrefs() const { return parent_->GetPrefs(); } +CefRequestContextImpl* CefBrowserContextProxy::GetCefRequestContext() const { + return request_context_; +} + const CefRequestContextSettings& CefBrowserContextProxy::GetSettings() const { return parent_->GetSettings(); } @@ -221,7 +227,7 @@ CefBrowserContextProxy::GetOrCreateStoragePartitionProxy( if (!storage_partition_proxy_) { scoped_refptr url_request_getter = new CefURLRequestContextGetterProxy(handler_, - parent_->request_context()); + parent_->request_context_getter()); resource_context()->set_url_request_context_getter( url_request_getter.get()); storage_partition_proxy_.reset( diff --git a/libcef/browser/browser_context_proxy.h b/libcef/browser/browser_context_proxy.h index 05d9f1519..ad8b1af2e 100644 --- a/libcef/browser/browser_context_proxy.h +++ b/libcef/browser/browser_context_proxy.h @@ -20,7 +20,8 @@ class CefStoragePartitionProxy; // browser_context.h for an object relationship diagram. class CefBrowserContextProxy : public CefBrowserContext { public: - CefBrowserContextProxy(CefRefPtr handler, + CefBrowserContextProxy(CefRequestContextImpl* const request_context, + CefRefPtr handler, CefBrowserContextImpl* parent); // Must be called immediately after this object is created. @@ -61,6 +62,7 @@ class CefBrowserContextProxy : public CefBrowserContext { const PrefService* GetPrefs() const override; // CefBrowserContext methods. + CefRequestContextImpl* GetCefRequestContext() const override; const CefRequestContextSettings& GetSettings() const override; CefRefPtr GetHandler() const override; HostContentSettingsMap* GetHostContentSettingsMap() override; @@ -79,6 +81,9 @@ class CefBrowserContextProxy : public CefBrowserContext { ~CefBrowserContextProxy() override; + // Guaranteed to outlive this object. + CefRequestContextImpl* const request_context_; + // Members initialized during construction are safe to access from any thread. CefRefPtr handler_; CefBrowserContextImpl* parent_; // Guaranteed to outlive this object. diff --git a/libcef/browser/browser_host_impl.cc b/libcef/browser/browser_host_impl.cc index 43104d6f8..14df2b671 100644 --- a/libcef/browser/browser_host_impl.cc +++ b/libcef/browser/browser_host_impl.cc @@ -2286,16 +2286,14 @@ void CefBrowserHostImpl::WebContentsCreated( CefRefPtr opener = GetBrowserForContents(source_contents); DCHECK(opener.get()); + // Popups must share the same BrowserContext as the parent. CefBrowserContext* browser_context = static_cast(new_contents->GetBrowserContext()); DCHECK(browser_context); - CefRefPtr request_context = - CefRequestContextImpl::CreateForBrowserContext(browser_context).get(); - DCHECK(request_context.get()); CefRefPtr browser = CefBrowserHostImpl::CreateInternal( - settings, client, new_contents, info, opener, false, request_context, - std::move(platform_delegate)); + settings, client, new_contents, info, opener, false, + browser_context->GetCefRequestContext(), std::move(platform_delegate)); } void CefBrowserHostImpl::DidNavigateMainFramePostCommit( diff --git a/libcef/browser/browser_main.cc b/libcef/browser/browser_main.cc index 05d12bf2a..cbab8b753 100644 --- a/libcef/browser/browser_main.cc +++ b/libcef/browser/browser_main.cc @@ -190,18 +190,20 @@ void CefBrowserMainParts::PreMainMessageLoopRun() { CefRequestContextSettings settings; CefContext::Get()->PopulateRequestContextSettings(&settings); - // Create the global BrowserContext. - global_browser_context_.reset(new CefBrowserContextImpl(settings)); - global_browser_context_->Initialize(); + // Create the global RequestContext. + global_request_context_ = + CefRequestContextImpl::CreateGlobalRequestContext(settings); + CefBrowserContextImpl* browser_context = static_cast( + global_request_context_->GetBrowserContext()); - CefDevToolsManagerDelegate::StartHttpHandler(global_browser_context_.get()); + CefDevToolsManagerDelegate::StartHttpHandler(browser_context); // Triggers initialization of the singleton instance on UI thread. PluginFinder::GetInstance()->Init(); device::GeolocationProvider::SetGeolocationDelegate( new CefGeolocationDelegate( - global_browser_context_->request_context().get())); + browser_context->request_context_getter().get())); scheme::RegisterWebUIControllerFactory(); } @@ -210,7 +212,7 @@ void CefBrowserMainParts::PostMainMessageLoopRun() { // NOTE: Destroy objects in reverse order of creation. CefDevToolsManagerDelegate::StopHttpHandler(); - global_browser_context_.reset(nullptr); + global_request_context_ = NULL; if (extensions::ExtensionsEnabled()) { extensions::ExtensionsBrowserClient::Set(NULL); diff --git a/libcef/browser/browser_main.h b/libcef/browser/browser_main.h index 0a7b58552..e55bdba8c 100644 --- a/libcef/browser/browser_main.h +++ b/libcef/browser/browser_main.h @@ -7,6 +7,7 @@ #pragma once #include "libcef/browser/net/url_request_context_getter_impl.h" +#include "libcef/browser/request_context_impl.h" #include "base/macros.h" #include "base/memory/scoped_vector.h" @@ -36,7 +37,6 @@ class WMState; } #endif -class CefBrowserContextImpl; class CefDevToolsDelegate; class CefBrowserMainParts : public content::BrowserMainParts { @@ -53,8 +53,8 @@ class CefBrowserMainParts : public content::BrowserMainParts { void PostMainMessageLoopRun() override; void PostDestroyThreads() override; - CefBrowserContextImpl* browser_context() const { - return global_browser_context_.get(); + CefRefPtr request_context() const { + return global_request_context_; } CefDevToolsDelegate* devtools_delegate() const { return devtools_delegate_; @@ -65,7 +65,7 @@ class CefBrowserMainParts : public content::BrowserMainParts { void PlatformInitialize(); #endif // defined(OS_WIN) - std::unique_ptr global_browser_context_; + CefRefPtr global_request_context_; CefDevToolsDelegate* devtools_delegate_; // Deletes itself. std::unique_ptr message_loop_; diff --git a/libcef/browser/chrome_profile_manager_stub.cc b/libcef/browser/chrome_profile_manager_stub.cc index 9c7edd066..1c3cde34b 100644 --- a/libcef/browser/chrome_profile_manager_stub.cc +++ b/libcef/browser/chrome_profile_manager_stub.cc @@ -21,7 +21,8 @@ namespace { // Return the main context for now since we don't currently have a good way to // determine that. CefBrowserContextImpl* GetActiveBrowserContext() { - return CefContentBrowserClient::Get()->browser_context(); + return static_cast( + CefContentBrowserClient::Get()->request_context()->GetBrowserContext()); } } // namespace diff --git a/libcef/browser/content_browser_client.cc b/libcef/browser/content_browser_client.cc index 3c151cb79..fdf6363f6 100644 --- a/libcef/browser/content_browser_client.cc +++ b/libcef/browser/content_browser_client.cc @@ -996,8 +996,9 @@ void CefContentBrowserClient::RegisterCustomScheme(const std::string& scheme) { policy->RegisterWebSafeScheme(scheme); } -CefBrowserContextImpl* CefContentBrowserClient::browser_context() const { - return browser_main_parts_->browser_context(); +CefRefPtr +CefContentBrowserClient::request_context() const { + return browser_main_parts_->request_context(); } CefDevToolsDelegate* CefContentBrowserClient::devtools_delegate() const { diff --git a/libcef/browser/content_browser_client.h b/libcef/browser/content_browser_client.h index 82d782bf8..57a44c576 100644 --- a/libcef/browser/content_browser_client.h +++ b/libcef/browser/content_browser_client.h @@ -11,6 +11,7 @@ #include "include/cef_request_context_handler.h" #include "libcef/browser/net/url_request_context_getter_impl.h" +#include "libcef/browser/request_context_impl.h" #include "base/macros.h" #include "base/memory/ref_counted.h" @@ -19,7 +20,6 @@ #include "third_party/skia/include/core/SkColor.h" class CefBrowserMainParts; -class CefBrowserContextImpl; class CefDevToolsDelegate; class CefResourceDispatcherHostDelegate; @@ -121,7 +121,7 @@ class CefContentBrowserClient : public content::ContentBrowserClient { // Perform browser process registration for the custom scheme. void RegisterCustomScheme(const std::string& scheme); - CefBrowserContextImpl* browser_context() const; + CefRefPtr request_context() const; CefDevToolsDelegate* devtools_delegate() const; private: diff --git a/libcef/browser/prefs/browser_prefs.cc b/libcef/browser/prefs/browser_prefs.cc index 36742634e..f3a0b7ae3 100644 --- a/libcef/browser/prefs/browser_prefs.cc +++ b/libcef/browser/prefs/browser_prefs.cc @@ -24,6 +24,7 @@ #include "chrome/grit/locale_settings.h" #include "components/content_settings/core/browser/cookie_settings.h" #include "components/content_settings/core/browser/host_content_settings_map.h" +#include "components/google/core/browser/google_url_tracker.h" #include "components/prefs/json_pref_store.h" #include "components/prefs/pref_filter.h" #include "components/prefs/pref_registry_simple.h" @@ -181,6 +182,7 @@ std::unique_ptr CreatePrefService( update_client::RegisterPrefs(registry.get()); content_settings::CookieSettings::RegisterProfilePrefs(registry.get()); chrome_browser_net::RegisterPredictionOptionsProfilePrefs(registry.get()); + GoogleURLTracker::RegisterProfilePrefs(registry.get()); // Print preferences. registry->RegisterBooleanPref(prefs::kPrintingEnabled, true); diff --git a/libcef/browser/request_context_impl.cc b/libcef/browser/request_context_impl.cc index 4de0e410a..f7a59b43e 100644 --- a/libcef/browser/request_context_impl.cc +++ b/libcef/browser/request_context_impl.cc @@ -90,7 +90,7 @@ CefRefPtr CefRequestContext::GetGlobalContext() { CefRequestContextImpl::Config config; config.is_global = true; - return new CefRequestContextImpl(config); + return CefRequestContextImpl::GetOrCreateRequestContext(config); } // static @@ -107,7 +107,7 @@ CefRefPtr CefRequestContext::CreateContext( config.settings = settings; config.handler = handler; config.unique_id = g_next_id.GetNext(); - return new CefRequestContextImpl(config); + return CefRequestContextImpl::GetOrCreateRequestContext(config); } // static @@ -127,7 +127,7 @@ CefRefPtr CefRequestContext::CreateContext( config.other = static_cast(other.get()); config.handler = handler; config.unique_id = g_next_id.GetNext(); - return new CefRequestContextImpl(config); + return CefRequestContextImpl::GetOrCreateRequestContext(config); } @@ -139,12 +139,25 @@ CefRequestContextImpl::~CefRequestContextImpl() { browser_context_proxy_.reset(nullptr); if (browser_context_impl_) { - // May result in |browser_context_impl_| being deleted if it's not the - // global context and no other CefRequestContextImpl are referencing it. - browser_context_impl_->RemoveRequestContext(); + // May result in |browser_context_impl_| being deleted if no other + // CefRequestContextImpl are referencing it. + browser_context_impl_->RemoveCefRequestContext(this); } } +// static +CefRefPtr +CefRequestContextImpl::CreateGlobalRequestContext( + const CefRequestContextSettings& settings) { + // Create and initialize the global context immediately. + Config config; + config.is_global = true; + config.settings = settings; + CefRefPtr impl = new CefRequestContextImpl(config); + impl->Initialize(); + return impl; +} + // static CefRefPtr CefRequestContextImpl::GetOrCreateForRequestContext( @@ -155,22 +168,9 @@ CefRequestContextImpl::GetOrCreateForRequestContext( } // Use the global context. - CefRequestContextImpl::Config config; + Config config; config.is_global = true; - return new CefRequestContextImpl(config); -} - -// static -CefRefPtr CefRequestContextImpl::CreateForBrowserContext( - CefBrowserContext* browser_context) { - DCHECK(browser_context); - CefRequestContextImpl::Config config; - config.handler = browser_context->GetHandler(); - CefRefPtr impl = new CefRequestContextImpl(config); - // Force immediate initialization because it's not safe to keep a raw pointer - // to |browser_context|. - impl->Initialize(browser_context); - return impl; + return CefRequestContextImpl::GetOrCreateRequestContext(config); } CefBrowserContext* CefRequestContextImpl::GetBrowserContext() { @@ -191,7 +191,7 @@ void CefRequestContextImpl::GetRequestContextImpl( const RequestContextCallback& callback) { if (!task_runner.get()) task_runner = base::MessageLoop::current()->task_runner(); - if (request_context_impl_) { + if (request_context_getter_impl_) { // The browser context already exists. DCHECK(browser_context()); GetRequestContextImplOnIOThread(task_runner, callback, browser_context()); @@ -257,9 +257,10 @@ bool CefRequestContextImpl::IsSharingWith(CefRefPtr other) { return pending_other->IsSharingWith(this); } - if (request_context_impl_ && other_impl->request_context_impl_) { + if (request_context_getter_impl_ && other_impl->request_context_getter_impl_) { // Both objects are initialized. Compare the request context objects. - return (request_context_impl_ == other_impl->request_context_impl_); + return (request_context_getter_impl_ == + other_impl->request_context_getter_impl_); } // This or the other object is not initialized. Compare the cache path values. @@ -479,12 +480,13 @@ cef_errorcode_t CefRequestContextImpl::ResolveHostCached( return ERR_FAILED; } - if (!request_context_impl_) + if (!request_context_getter_impl_) return ERR_FAILED; int retval = ERR_FAILED; - net::HostResolver* host_resolver = request_context_impl_->GetHostResolver(); + net::HostResolver* host_resolver = + request_context_getter_impl_->GetHostResolver(); if (host_resolver) { net::HostResolver::RequestInfo request_info( net::HostPortPair::FromURL(GURL(origin.ToString()))); @@ -501,38 +503,34 @@ cef_errorcode_t CefRequestContextImpl::ResolveHostCached( return static_cast(retval); } +// static +CefRefPtr +CefRequestContextImpl::GetOrCreateRequestContext(const Config& config) { + if (config.is_global || + (config.other && config.other->IsGlobal() && !config.handler)) { + // Return the singleton global context. + return CefContentBrowserClient::Get()->request_context(); + } + + // The new context will be initialized later by EnsureBrowserContext(). + return new CefRequestContextImpl(config); +} + CefRequestContextImpl::CefRequestContextImpl( const CefRequestContextImpl::Config& config) : config_(config) { } void CefRequestContextImpl::Initialize() { - CefBrowserContext* other_browser_context = nullptr; - if (config_.is_global) - other_browser_context = CefContentBrowserClient::Get()->browser_context(); - else if (config_.other.get()) - other_browser_context = config_.other->GetBrowserContext(); - - Initialize(other_browser_context); - - if (config_.other.get()) { - // Clear the reference to |other_| after setting |request_context_impl_|. - // This is the reverse order of checks in IsSharedWith(). - config_.other = NULL; - } -} - -void CefRequestContextImpl::Initialize( - CefBrowserContext* other_browser_context) { CEF_REQUIRE_UIT(); DCHECK(!browser_context_impl_); - DCHECK(!request_context_impl_); + DCHECK(!request_context_getter_impl_); - if (other_browser_context) { - // Share storage with |other_browser_context|. + if (config_.other) { + // Share storage with |config_.other|. browser_context_impl_ = CefBrowserContextImpl::GetForContext( - other_browser_context); + config_.other->GetBrowserContext()); } if (!browser_context_impl_) { @@ -556,7 +554,7 @@ void CefRequestContextImpl::Initialize( } // We'll disassociate from |browser_context_impl_| on destruction. - browser_context_impl_->AddRequestContext(); + browser_context_impl_->AddCefRequestContext(this); // Force our settings to match |browser_context_impl_|. config_.settings = browser_context_impl_->GetSettings(); @@ -565,21 +563,27 @@ void CefRequestContextImpl::Initialize( // Use a proxy that will execute handler callbacks where appropriate and // otherwise forward all requests to |browser_context_impl_|. browser_context_proxy_.reset( - new CefBrowserContextProxy(config_.handler, browser_context_impl_)); + new CefBrowserContextProxy(this, config_.handler, + browser_context_impl_)); browser_context_proxy_->Initialize(); DCHECK(!config_.is_global); - } else { - config_.is_global = (browser_context_impl_ == - CefContentBrowserClient::Get()->browser_context()); } - request_context_impl_ = browser_context_impl_->request_context().get(); - DCHECK(request_context_impl_); + request_context_getter_impl_ = + browser_context_impl_->request_context_getter().get(); + DCHECK(request_context_getter_impl_); if (config_.handler.get()) { // Keep the handler alive until the associated request context is // destroyed. - request_context_impl_->AddHandler(config_.handler); + request_context_getter_impl_->AddHandler(config_.handler); + } + + if (config_.other) { + // Clear the reference to |config_.other| after setting + // |request_context_getter_impl_|. This is the reverse order of checks in + // IsSharedWith(). + config_.other = NULL; } } @@ -587,7 +591,7 @@ void CefRequestContextImpl::EnsureBrowserContext() { if (!browser_context()) Initialize(); DCHECK(browser_context()); - DCHECK(request_context_impl_); + DCHECK(request_context_getter_impl_); } void CefRequestContextImpl::GetBrowserContextOnUIThread( @@ -623,18 +627,18 @@ void CefRequestContextImpl::GetRequestContextImplOnIOThread( return; } - DCHECK(request_context_impl_); + DCHECK(request_context_getter_impl_); // Make sure the request context exists. - request_context_impl_->GetURLRequestContext(); + request_context_getter_impl_->GetURLRequestContext(); if (task_runner->BelongsToCurrentThread()) { // Execute the callback immediately. - callback.Run(request_context_impl_); + callback.Run(request_context_getter_impl_); } else { // Execute the callback on the target thread. task_runner->PostTask(FROM_HERE, - base::Bind(callback, make_scoped_refptr(request_context_impl_))); + base::Bind(callback, make_scoped_refptr(request_context_getter_impl_))); } } diff --git a/libcef/browser/request_context_impl.h b/libcef/browser/request_context_impl.h index 7f2c4a510..3cfd069a3 100644 --- a/libcef/browser/request_context_impl.h +++ b/libcef/browser/request_context_impl.h @@ -19,16 +19,16 @@ class CefRequestContextImpl : public CefRequestContext { public: ~CefRequestContextImpl() override; + // Creates the singleton global RequestContext. Called from + // CefBrowserMainParts::PreMainMessageLoopRun. + static CefRefPtr CreateGlobalRequestContext( + const CefRequestContextSettings& settings); + // Returns a CefRequestContextImpl for the specified |request_context|. // Will return the global context if |request_context| is NULL. static CefRefPtr GetOrCreateForRequestContext( CefRefPtr request_context); - // Returns a CefRequestContextImpl for the specified |browser_context|. - // |browser_context| must be non-NULL. - static CefRefPtr CreateForBrowserContext( - CefBrowserContext* browser_context); - // Returns the browser context object. Can only be called on the UI thread. CefBrowserContext* GetBrowserContext(); @@ -108,10 +108,12 @@ class CefRequestContextImpl : public CefRequestContext { int unique_id = -1; }; + static CefRefPtr GetOrCreateRequestContext( + const Config& config); + explicit CefRequestContextImpl(const Config& config); void Initialize(); - void Initialize(CefBrowserContext* other_browser_context); // Make sure the browser context exists. Only called on the UI thread. void EnsureBrowserContext(); @@ -155,7 +157,7 @@ class CefRequestContextImpl : public CefRequestContext { Config config_; // Owned by the CefBrowserContext. - CefURLRequestContextGetterImpl* request_context_impl_ = nullptr; + CefURLRequestContextGetterImpl* request_context_getter_impl_ = nullptr; IMPLEMENT_REFCOUNTING_DELETE_ON_UIT(CefRequestContextImpl); DISALLOW_COPY_AND_ASSIGN(CefRequestContextImpl); diff --git a/tests/ceftests/navigation_unittest.cc b/tests/ceftests/navigation_unittest.cc index c3c4fc101..b0aaa0371 100644 --- a/tests/ceftests/navigation_unittest.cc +++ b/tests/ceftests/navigation_unittest.cc @@ -2237,229 +2237,6 @@ TEST(NavigationTest, CrossOriginCtrlLeftClickCancel) { } -namespace { - -const char kPopupNavPageUrl[] = "http://tests-popup.com/page.html"; -const char kPopupNavPopupUrl[] = "http://tests-popup.com/popup.html"; -const char kPopupNavPopupUrl2[] = "http://tests-popup2.com/popup.html"; -const char kPopupNavPopupName[] = "my_popup"; - -// Browser side. -class PopupNavTestHandler : public TestHandler { - public: - enum Mode { - ALLOW, - DENY, - NAVIGATE_AFTER_CREATION, - }; - - PopupNavTestHandler(Mode mode) - : mode_(mode) {} - - void RunTest() override { - // Add the resources that we will navigate to/from. - std::string page = "Page"; - AddResource(kPopupNavPageUrl, page, "text/html"); - AddResource(kPopupNavPopupUrl, "Popup", "text/html"); - if (mode_ == NAVIGATE_AFTER_CREATION) - AddResource(kPopupNavPopupUrl2, "Popup2", "text/html"); - - // Create the browser. - CreateBrowser(kPopupNavPageUrl); - - // Time out the test after a reasonable period of time. - SetTestTimeout(); - } - - bool OnBeforePopup(CefRefPtr browser, - CefRefPtr frame, - const CefString& target_url, - const CefString& target_frame_name, - cef_window_open_disposition_t target_disposition, - bool user_gesture, - const CefPopupFeatures& popupFeatures, - CefWindowInfo& windowInfo, - CefRefPtr& client, - CefBrowserSettings& settings, - bool* no_javascript_access) override { - EXPECT_FALSE(got_on_before_popup_); - got_on_before_popup_.yes(); - - EXPECT_TRUE(CefCurrentlyOn(TID_IO)); - EXPECT_EQ(GetBrowserId(), browser->GetIdentifier()); - EXPECT_STREQ(kPopupNavPageUrl, frame->GetURL().ToString().c_str()); - EXPECT_STREQ(kPopupNavPopupUrl, target_url.ToString().c_str()); - EXPECT_STREQ(kPopupNavPopupName, target_frame_name.ToString().c_str()); - EXPECT_EQ(WOD_NEW_FOREGROUND_TAB, target_disposition); - EXPECT_FALSE(user_gesture); - EXPECT_FALSE(*no_javascript_access); - - return (mode_ == DENY); // Return true to cancel the popup. - } - - void OnAfterCreated(CefRefPtr browser) override { - TestHandler::OnAfterCreated(browser); - - if (mode_ == NAVIGATE_AFTER_CREATION && browser->IsPopup()) { - // Navigate to the 2nd popup URL instead of the 1st popup URL. - browser->GetMainFrame()->LoadURL(kPopupNavPopupUrl2); - } - } - - void OnLoadStart(CefRefPtr browser, - CefRefPtr frame, - TransitionType transition_type) override { - const std::string& url = frame->GetURL(); - if (url == kPopupNavPageUrl) { - EXPECT_FALSE(got_load_start_); - got_load_start_.yes(); - } else if (url == kPopupNavPopupUrl) { - EXPECT_FALSE(got_popup_load_start_); - got_popup_load_start_.yes(); - } else if (url == kPopupNavPopupUrl2) { - EXPECT_FALSE(got_popup_load_start2_); - got_popup_load_start2_.yes(); - } - } - - void OnLoadError(CefRefPtr browser, - CefRefPtr frame, - ErrorCode errorCode, - const CefString& errorText, - const CefString& failedUrl) override { - if (failedUrl == kPopupNavPageUrl) { - EXPECT_FALSE(got_load_error_); - got_load_error_.yes(); - } else if (failedUrl == kPopupNavPopupUrl) { - EXPECT_FALSE(got_popup_load_error_); - got_popup_load_error_.yes(); - } else if (failedUrl == kPopupNavPopupUrl2) { - EXPECT_FALSE(got_popup_load_error2_); - got_popup_load_error2_.yes(); - } - } - - void OnLoadEnd(CefRefPtr browser, - CefRefPtr frame, - int httpStatusCode) override { - const std::string& url = frame->GetURL(); - if (url == kPopupNavPageUrl) { - EXPECT_FALSE(got_load_end_); - got_load_end_.yes(); - - frame->ExecuteJavaScript("doPopup()", kPopupNavPageUrl, 0); - - if (mode_ == DENY) { - // Wait a bit to make sure the popup window isn't created. - CefPostDelayedTask(TID_UI, - base::Bind(&PopupNavTestHandler::DestroyTest, this), 200); - } - } else if (url == kPopupNavPopupUrl) { - EXPECT_FALSE(got_popup_load_end_); - got_popup_load_end_.yes(); - - if (mode_ != NAVIGATE_AFTER_CREATION) { - if (mode_ != DENY) { - CloseBrowser(browser, false); - DestroyTest(); - } else { - EXPECT_FALSE(true); // Not reached. - } - } - } else if (url == kPopupNavPopupUrl2) { - EXPECT_FALSE(got_popup_load_end2_); - got_popup_load_end2_.yes(); - - if (mode_ == NAVIGATE_AFTER_CREATION) { - CloseBrowser(browser, false); - DestroyTest(); - } else { - EXPECT_FALSE(true); // Not reached. - } - } else { - EXPECT_FALSE(true); // Not reached. - } - } - - private: - void DestroyTest() override { - EXPECT_TRUE(got_load_start_); - EXPECT_FALSE(got_load_error_); - EXPECT_TRUE(got_load_end_); - EXPECT_TRUE(got_on_before_popup_); - if (mode_ == ALLOW) { - EXPECT_TRUE(got_popup_load_start_); - EXPECT_FALSE(got_popup_load_error_); - EXPECT_TRUE(got_popup_load_end_); - EXPECT_FALSE(got_popup_load_start2_); - EXPECT_FALSE(got_popup_load_error2_); - EXPECT_FALSE(got_popup_load_end2_); - } else if (mode_ == DENY) { - EXPECT_FALSE(got_popup_load_start_); - EXPECT_FALSE(got_popup_load_error_); - EXPECT_FALSE(got_popup_load_end_); - EXPECT_FALSE(got_popup_load_start2_); - EXPECT_FALSE(got_popup_load_error2_); - EXPECT_FALSE(got_popup_load_end2_); - } else if (mode_ == NAVIGATE_AFTER_CREATION) { - EXPECT_FALSE(got_popup_load_start_); - EXPECT_TRUE(got_popup_load_error_); - EXPECT_FALSE(got_popup_load_end_); - EXPECT_TRUE(got_popup_load_start2_); - EXPECT_FALSE(got_popup_load_error2_); - EXPECT_TRUE(got_popup_load_end2_); - } - - TestHandler::DestroyTest(); - } - - const Mode mode_; - - TrackCallback got_on_before_popup_; - TrackCallback got_load_start_; - TrackCallback got_load_error_; - TrackCallback got_load_end_; - TrackCallback got_popup_load_start_; - TrackCallback got_popup_load_error_; - TrackCallback got_popup_load_end_; - TrackCallback got_popup_load_start2_; - TrackCallback got_popup_load_error2_; - TrackCallback got_popup_load_end2_; - - IMPLEMENT_REFCOUNTING(PopupNavTestHandler); -}; - -} // namespace - -// Test allowing popups. -TEST(NavigationTest, PopupAllow) { - CefRefPtr handler = - new PopupNavTestHandler(PopupNavTestHandler::ALLOW); - handler->ExecuteTest(); - ReleaseAndWaitForDestructor(handler); -} - -// Test denying popups. -TEST(NavigationTest, PopupDeny) { - CefRefPtr handler = - new PopupNavTestHandler(PopupNavTestHandler::DENY); - handler->ExecuteTest(); - ReleaseAndWaitForDestructor(handler); -} - -// Test navigation to a different origin after popup creation to verify that -// internal objects are tracked correctly (see issue #1392). -TEST(NavigationTest, PopupNavigateAfterCreation) { - CefRefPtr handler = - new PopupNavTestHandler(PopupNavTestHandler::NAVIGATE_AFTER_CREATION); - handler->ExecuteTest(); - ReleaseAndWaitForDestructor(handler); -} - - namespace { const char kSimultPopupMainUrl[] = "http://www.tests-sp.com/main.html"; diff --git a/tests/ceftests/request_context_unittest.cc b/tests/ceftests/request_context_unittest.cc index ea5574282..e842a28b9 100644 --- a/tests/ceftests/request_context_unittest.cc +++ b/tests/ceftests/request_context_unittest.cc @@ -125,50 +125,17 @@ TEST(RequestContextTest, CreateContextSharedGlobal) { EXPECT_TRUE(context1->IsSame(context1)); EXPECT_TRUE(context1->IsSharingWith(context1)); + // Returns the same global context. CefRefPtr context2 = CefRequestContext::CreateContext(context1, NULL); EXPECT_TRUE(context2.get()); - EXPECT_FALSE(context2->IsGlobal()); + EXPECT_TRUE(context2->IsGlobal()); EXPECT_TRUE(context2->IsSame(context2)); - EXPECT_FALSE(context2->IsSame(context1)); - EXPECT_FALSE(context1->IsSame(context2)); + EXPECT_TRUE(context2->IsSame(context1)); + EXPECT_TRUE(context1->IsSame(context2)); EXPECT_TRUE(context2->IsSharingWith(context2)); EXPECT_TRUE(context2->IsSharingWith(context1)); EXPECT_TRUE(context1->IsSharingWith(context2)); - - CefRefPtr context3 = - CefRequestContext::CreateContext(context2, NULL); - EXPECT_TRUE(context3.get()); - EXPECT_FALSE(context3->IsGlobal()); - EXPECT_TRUE(context3->IsSame(context3)); - EXPECT_FALSE(context3->IsSame(context2)); - EXPECT_FALSE(context3->IsSame(context1)); - EXPECT_FALSE(context1->IsSame(context3)); - EXPECT_FALSE(context2->IsSame(context3)); - EXPECT_TRUE(context3->IsSharingWith(context3)); - EXPECT_TRUE(context3->IsSharingWith(context2)); - EXPECT_TRUE(context3->IsSharingWith(context1)); - EXPECT_TRUE(context1->IsSharingWith(context3)); - EXPECT_TRUE(context2->IsSharingWith(context3)); - - CefRefPtr context4 = - CefRequestContext::CreateContext(context1, NULL); - EXPECT_TRUE(context4.get()); - EXPECT_FALSE(context4->IsGlobal()); - EXPECT_TRUE(context4->IsSame(context4)); - EXPECT_FALSE(context4->IsSame(context3)); - EXPECT_FALSE(context4->IsSame(context2)); - EXPECT_FALSE(context4->IsSame(context1)); - EXPECT_FALSE(context1->IsSame(context4)); - EXPECT_FALSE(context2->IsSame(context4)); - EXPECT_FALSE(context3->IsSame(context4)); - EXPECT_TRUE(context4->IsSharingWith(context4)); - EXPECT_TRUE(context4->IsSharingWith(context3)); - EXPECT_TRUE(context4->IsSharingWith(context2)); - EXPECT_TRUE(context4->IsSharingWith(context1)); - EXPECT_TRUE(context1->IsSharingWith(context4)); - EXPECT_TRUE(context2->IsSharingWith(context4)); - EXPECT_TRUE(context3->IsSharingWith(context4)); } TEST(RequestContextTest, CreateContextSharedOnDisk) { @@ -649,6 +616,278 @@ TEST(RequestContextTest, NoReferrerLinkDifferentOrigin) { } +namespace { + +const char kPopupNavPageUrl[] = "http://tests-popup.com/page.html"; +const char kPopupNavPopupUrl[] = "http://tests-popup.com/popup.html"; +const char kPopupNavPopupUrl2[] = "http://tests-popup2.com/popup.html"; +const char kPopupNavPopupName[] = "my_popup"; + +// Browser side. +class PopupNavTestHandler : public TestHandler { + public: + enum Mode { + ALLOW_CLOSE_POPUP_FIRST, + ALLOW_CLOSE_POPUP_LAST, + DENY, + NAVIGATE_AFTER_CREATION, + }; + enum RCMode { + RC_MODE_NONE, + RC_MODE_IMPL, + RC_MODE_PROXY, + }; + + PopupNavTestHandler(Mode mode, RCMode rc_mode) + : mode_(mode), + rc_mode_(rc_mode) {} + + void RunTest() override { + // Add the resources that we will navigate to/from. + std::string page = "Page"; + AddResource(kPopupNavPageUrl, page, "text/html"); + AddResource(kPopupNavPopupUrl, "Popup", "text/html"); + if (mode_ == NAVIGATE_AFTER_CREATION) + AddResource(kPopupNavPopupUrl2, "Popup2", "text/html"); + + CefRefPtr request_context; + CefRefPtr rc_handler; + if (rc_mode_ == RC_MODE_PROXY) { + class Handler : public CefRequestContextHandler { + public: + Handler() {} + private: + IMPLEMENT_REFCOUNTING(Handler); + }; + rc_handler = new Handler(); + } + + if (rc_mode_ != RC_MODE_NONE) { + CefRequestContextSettings settings; + request_context = CefRequestContext::CreateContext(settings, rc_handler); + } + + // Create the browser. + CreateBrowser(kPopupNavPageUrl, request_context); + + // Time out the test after a reasonable period of time. + SetTestTimeout(); + } + + bool OnBeforePopup(CefRefPtr browser, + CefRefPtr frame, + const CefString& target_url, + const CefString& target_frame_name, + cef_window_open_disposition_t target_disposition, + bool user_gesture, + const CefPopupFeatures& popupFeatures, + CefWindowInfo& windowInfo, + CefRefPtr& client, + CefBrowserSettings& settings, + bool* no_javascript_access) override { + EXPECT_FALSE(got_on_before_popup_); + got_on_before_popup_.yes(); + + EXPECT_TRUE(CefCurrentlyOn(TID_IO)); + EXPECT_EQ(GetBrowserId(), browser->GetIdentifier()); + EXPECT_STREQ(kPopupNavPageUrl, frame->GetURL().ToString().c_str()); + EXPECT_STREQ(kPopupNavPopupUrl, target_url.ToString().c_str()); + EXPECT_STREQ(kPopupNavPopupName, target_frame_name.ToString().c_str()); + EXPECT_EQ(WOD_NEW_FOREGROUND_TAB, target_disposition); + EXPECT_FALSE(user_gesture); + EXPECT_FALSE(*no_javascript_access); + + return (mode_ == DENY); // Return true to cancel the popup. + } + + void OnAfterCreated(CefRefPtr browser) override { + TestHandler::OnAfterCreated(browser); + + if (mode_ == NAVIGATE_AFTER_CREATION && browser->IsPopup()) { + // Navigate to the 2nd popup URL instead of the 1st popup URL. + browser->GetMainFrame()->LoadURL(kPopupNavPopupUrl2); + } + } + + void OnLoadStart(CefRefPtr browser, + CefRefPtr frame, + TransitionType transition_type) override { + const std::string& url = frame->GetURL(); + if (url == kPopupNavPageUrl) { + EXPECT_FALSE(got_load_start_); + got_load_start_.yes(); + } else if (url == kPopupNavPopupUrl) { + EXPECT_FALSE(got_popup_load_start_); + got_popup_load_start_.yes(); + } else if (url == kPopupNavPopupUrl2) { + EXPECT_FALSE(got_popup_load_start2_); + got_popup_load_start2_.yes(); + } + } + + void OnLoadError(CefRefPtr browser, + CefRefPtr frame, + ErrorCode errorCode, + const CefString& errorText, + const CefString& failedUrl) override { + if (failedUrl == kPopupNavPageUrl) { + EXPECT_FALSE(got_load_error_); + got_load_error_.yes(); + } else if (failedUrl == kPopupNavPopupUrl) { + EXPECT_FALSE(got_popup_load_error_); + got_popup_load_error_.yes(); + } else if (failedUrl == kPopupNavPopupUrl2) { + EXPECT_FALSE(got_popup_load_error2_); + got_popup_load_error2_.yes(); + } + } + + void OnLoadEnd(CefRefPtr browser, + CefRefPtr frame, + int httpStatusCode) override { + const std::string& url = frame->GetURL(); + if (url == kPopupNavPageUrl) { + EXPECT_FALSE(got_load_end_); + got_load_end_.yes(); + + frame->ExecuteJavaScript("doPopup()", kPopupNavPageUrl, 0); + + if (mode_ == DENY) { + // Wait a bit to make sure the popup window isn't created. + CefPostDelayedTask(TID_UI, + base::Bind(&PopupNavTestHandler::DestroyTest, this), 200); + } + } else if (url == kPopupNavPopupUrl) { + EXPECT_FALSE(got_popup_load_end_); + got_popup_load_end_.yes(); + + if (mode_ == ALLOW_CLOSE_POPUP_FIRST) { + // Close the popup browser first. + CloseBrowser(browser, false); + } else if (mode_ == ALLOW_CLOSE_POPUP_LAST) { + // Close the main browser first. + CloseBrowser(GetBrowser(), false); + } else if (mode_ != NAVIGATE_AFTER_CREATION) { + EXPECT_FALSE(true); // Not reached. + } + } else if (url == kPopupNavPopupUrl2) { + EXPECT_FALSE(got_popup_load_end2_); + got_popup_load_end2_.yes(); + + if (mode_ == NAVIGATE_AFTER_CREATION) { + // Close the popup browser first. + CloseBrowser(browser, false); + } else { + EXPECT_FALSE(true); // Not reached. + } + } else { + EXPECT_FALSE(true); // Not reached. + } + } + + void OnBeforeClose(CefRefPtr browser) override { + TestHandler::OnBeforeClose(browser); + + bool destroy_test = false; + if (mode_ == ALLOW_CLOSE_POPUP_FIRST || mode_ == NAVIGATE_AFTER_CREATION) { + // Destroy the test after the popup browser closes. + if (browser->IsPopup()) + destroy_test = true; + } else if (mode_ == ALLOW_CLOSE_POPUP_LAST) { + // Destroy the test after the main browser closes. + if (!browser->IsPopup()) + destroy_test = true; + } + + if (destroy_test) { + CefPostTask(TID_UI, base::Bind(&PopupNavTestHandler::DestroyTest, this)); + } + } + + private: + void DestroyTest() override { + EXPECT_TRUE(got_load_start_); + EXPECT_FALSE(got_load_error_); + EXPECT_TRUE(got_load_end_); + EXPECT_TRUE(got_on_before_popup_); + if (mode_ == ALLOW_CLOSE_POPUP_FIRST || mode_ == ALLOW_CLOSE_POPUP_LAST) { + EXPECT_TRUE(got_popup_load_start_); + EXPECT_FALSE(got_popup_load_error_); + EXPECT_TRUE(got_popup_load_end_); + EXPECT_FALSE(got_popup_load_start2_); + EXPECT_FALSE(got_popup_load_error2_); + EXPECT_FALSE(got_popup_load_end2_); + } else if (mode_ == DENY) { + EXPECT_FALSE(got_popup_load_start_); + EXPECT_FALSE(got_popup_load_error_); + EXPECT_FALSE(got_popup_load_end_); + EXPECT_FALSE(got_popup_load_start2_); + EXPECT_FALSE(got_popup_load_error2_); + EXPECT_FALSE(got_popup_load_end2_); + } else if (mode_ == NAVIGATE_AFTER_CREATION) { + EXPECT_FALSE(got_popup_load_start_); + EXPECT_TRUE(got_popup_load_error_); + EXPECT_FALSE(got_popup_load_end_); + EXPECT_TRUE(got_popup_load_start2_); + EXPECT_FALSE(got_popup_load_error2_); + EXPECT_TRUE(got_popup_load_end2_); + } + + // Will trigger destruction of all remaining browsers. + TestHandler::DestroyTest(); + } + + const Mode mode_; + const RCMode rc_mode_; + + TrackCallback got_on_before_popup_; + TrackCallback got_load_start_; + TrackCallback got_load_error_; + TrackCallback got_load_end_; + TrackCallback got_popup_load_start_; + TrackCallback got_popup_load_error_; + TrackCallback got_popup_load_end_; + TrackCallback got_popup_load_start2_; + TrackCallback got_popup_load_error2_; + TrackCallback got_popup_load_end2_; + + IMPLEMENT_REFCOUNTING(PopupNavTestHandler); +}; + +} // namespace + +#define POPUP_TEST(name, test_mode, rc_mode)\ + TEST(RequestContextTest, Popup##name) {\ + CefRefPtr handler =\ + new PopupNavTestHandler(PopupNavTestHandler::test_mode,\ + PopupNavTestHandler::rc_mode);\ + handler->ExecuteTest();\ + ReleaseAndWaitForDestructor(handler);\ + } + +#define POPUP_TEST_GROUP(name, test_mode)\ + POPUP_TEST(name##RCNone, test_mode, RC_MODE_NONE);\ + POPUP_TEST(name##RCImpl, test_mode, RC_MODE_IMPL);\ + POPUP_TEST(name##RCProxy, test_mode, RC_MODE_PROXY); + +// Test allowing popups and closing the popup browser first. +POPUP_TEST_GROUP(AllowClosePopupFirst, ALLOW_CLOSE_POPUP_FIRST); + +// Test allowing popups and closing the main browser first to verify that +// internal objects are tracked correctly (see issue #2162). +POPUP_TEST_GROUP(AllowClosePopupLast, ALLOW_CLOSE_POPUP_LAST); + +// Test denying popups. +POPUP_TEST_GROUP(Deny, DENY); + +// Test navigation to a different origin after popup creation to verify that +// internal objects are tracked correctly (see issue #1392). +POPUP_TEST_GROUP(NavigateAfterCreation, NAVIGATE_AFTER_CREATION); + + namespace { const char kResolveOrigin[] = "http://www.google.com"; diff --git a/tests/ceftests/test_handler.cc b/tests/ceftests/test_handler.cc index 94f8e262e..fd4a7c3fd 100644 --- a/tests/ceftests/test_handler.cc +++ b/tests/ceftests/test_handler.cc @@ -222,38 +222,33 @@ void TestHandler::OnAfterCreated(CefRefPtr browser) { browser_count_++; - if (!browser->IsPopup()) { - // Keep non-popup browsers. - const int browser_id = browser->GetIdentifier(); - EXPECT_EQ(browser_map_.find(browser_id), browser_map_.end()); - if (browser_map_.empty()) { - first_browser_id_ = browser_id; - first_browser_ = browser; - } - browser_map_.insert(std::make_pair(browser_id, browser)); + const int browser_id = browser->GetIdentifier(); + EXPECT_EQ(browser_map_.find(browser_id), browser_map_.end()); + if (browser_map_.empty()) { + first_browser_id_ = browser_id; + first_browser_ = browser; } + browser_map_.insert(std::make_pair(browser_id, browser)); } void TestHandler::OnBeforeClose(CefRefPtr browser) { EXPECT_UI_THREAD(); - if (!browser->IsPopup()) { - // Free the browser pointer so that the browser can be destroyed. - const int browser_id = browser->GetIdentifier(); - BrowserMap::iterator it = browser_map_.find(browser_id); - EXPECT_NE(it, browser_map_.end()); - browser_map_.erase(it); + // Free the browser pointer so that the browser can be destroyed. + const int browser_id = browser->GetIdentifier(); + BrowserMap::iterator it = browser_map_.find(browser_id); + EXPECT_NE(it, browser_map_.end()); + browser_map_.erase(it); - if (browser_id == first_browser_id_) { - first_browser_id_ = 0; - first_browser_ = NULL; - } + if (browser_id == first_browser_id_) { + first_browser_id_ = 0; + first_browser_ = NULL; + } - if (browser_map_.empty() && - signal_completion_when_all_browsers_close_) { - // Signal that the test is now complete. - TestComplete(); - } + if (browser_map_.empty() && + signal_completion_when_all_browsers_close_) { + // Signal that the test is now complete. + TestComplete(); } browser_count_--; @@ -352,7 +347,7 @@ void TestHandler::DestroyTest() { // iterating. BrowserMap browser_map = browser_map_; - // Tell all non-popup browsers to close. + // Tell all browsers to close. BrowserMap::const_iterator it = browser_map.begin(); for (; it != browser_map.end(); ++it) CloseBrowser(it->second, false);