Popups must share the parent context to avoid crashes on parent browser destruction (issue #2162)

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;

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_--;
+  request_context_set_.erase(context);
-  DCHECK_GE(request_context_count_, 0);
 
-  // Delete non-global contexts when the reference count reaches zero.
+  // Delete ourselves when the reference count reaches zero.
-  if (request_context_count_ == 0 &&
+  if (request_context_set_.empty())
-      this != CefContentBrowserClient::Get()->browser_context()) {
     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_;
 }

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-
+  // Track associated CefRequestContextImpl objects. This object will delete
-  // global context then it will delete itself when the count reaches zero.
+  // itself when the count reaches zero.
-  void AddRequestContext();
+  void AddCefRequestContext(CefRequestContextImpl* context);
-  void RemoveRequestContext();
+  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<CefRequestContextHandler> GetHandler() const override;
   HostContentSettingsMap* GetHostContentSettingsMap() override;
@@ -97,7 +99,7 @@ class CefBrowserContextImpl : public CefBrowserContext,
   void RebuildTable(const scoped_refptr<URLEnumerator>& enumerator) override;
 
   // Guaranteed to exist once this object has been initialized.
-  scoped_refptr<CefURLRequestContextGetterImpl> request_context() const {
+  scoped_refptr<CefURLRequestContextGetterImpl> 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.
+  // CefRequestContextImpl objects referencing this object.
-  int request_context_count_ = 0;
+  std::set<CefRequestContextImpl*> request_context_set_;
 
   std::unique_ptr<PrefService> pref_service_;
   std::unique_ptr<PrefProxyConfigTracker> pref_proxy_config_tracker_;

libcef/browser/browser_context_proxy.cc

@@ -58,9 +58,11 @@ bool ShouldProxyUserData(const void* key) {
 }  // namespace
 
 CefBrowserContextProxy::CefBrowserContextProxy(
+    CefRequestContextImpl* const request_context,
     CefRefPtr<CefRequestContextHandler> 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<CefURLRequestContextGetterProxy> 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(

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<CefRequestContextHandler> handler,
+  CefBrowserContextProxy(CefRequestContextImpl* const request_context,
+                         CefRefPtr<CefRequestContextHandler> 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<CefRequestContextHandler> 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<CefRequestContextHandler> handler_;
   CefBrowserContextImpl* parent_;  // Guaranteed to outlive this object.

libcef/browser/browser_host_impl.cc

@@ -2286,16 +2286,14 @@ void CefBrowserHostImpl::WebContentsCreated(
   CefRefPtr<CefBrowserHostImpl> opener = GetBrowserForContents(source_contents);
   DCHECK(opener.get());
 
+  // Popups must share the same BrowserContext as the parent.
   CefBrowserContext* browser_context =
       static_cast<CefBrowserContext*>(new_contents->GetBrowserContext());
   DCHECK(browser_context);
-  CefRefPtr<CefRequestContext> request_context =
-      CefRequestContextImpl::CreateForBrowserContext(browser_context).get();
-  DCHECK(request_context.get());
 
   CefRefPtr<CefBrowserHostImpl> browser = CefBrowserHostImpl::CreateInternal(
-      settings, client, new_contents, info, opener, false, request_context,
+      settings, client, new_contents, info, opener, false,
-      std::move(platform_delegate));
+      browser_context->GetCefRequestContext(), std::move(platform_delegate));
 }
 
 void CefBrowserHostImpl::DidNavigateMainFramePostCommit(

libcef/browser/browser_main.cc

@@ -190,18 +190,20 @@ void CefBrowserMainParts::PreMainMessageLoopRun() {
   CefRequestContextSettings settings;
   CefContext::Get()->PopulateRequestContextSettings(&settings);
 
-  // Create the global BrowserContext.
+  // Create the global RequestContext.
-  global_browser_context_.reset(new CefBrowserContextImpl(settings));
+  global_request_context_ =
-  global_browser_context_->Initialize();
+      CefRequestContextImpl::CreateGlobalRequestContext(settings);
+  CefBrowserContextImpl* browser_context = static_cast<CefBrowserContextImpl*>(
+      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);

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 {
+  CefRefPtr<CefRequestContextImpl> request_context() const {
-    return global_browser_context_.get();
+    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<CefBrowserContextImpl> global_browser_context_;
+  CefRefPtr<CefRequestContextImpl> global_request_context_;
   CefDevToolsDelegate* devtools_delegate_;  // Deletes itself.
   std::unique_ptr<base::MessageLoop> message_loop_;
 

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<CefBrowserContextImpl*>(
+      CefContentBrowserClient::Get()->request_context()->GetBrowserContext());
 }
 
 }  // namespace

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 {
+CefRefPtr<CefRequestContextImpl>
-  return browser_main_parts_->browser_context();
+CefContentBrowserClient::request_context() const {
+  return browser_main_parts_->request_context();
 }
 
 CefDevToolsDelegate* CefContentBrowserClient::devtools_delegate() const {

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<CefRequestContextImpl> request_context() const;
   CefDevToolsDelegate* devtools_delegate() const;
 
  private:

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<PrefService> 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);

libcef/browser/request_context_impl.cc

@@ -90,7 +90,7 @@ CefRefPtr<CefRequestContext> CefRequestContext::GetGlobalContext() {
 
   CefRequestContextImpl::Config config;
   config.is_global = true;
-  return new CefRequestContextImpl(config);
+  return CefRequestContextImpl::GetOrCreateRequestContext(config);
 }
 
 // static
@@ -107,7 +107,7 @@ CefRefPtr<CefRequestContext> 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> CefRequestContext::CreateContext(
   config.other = static_cast<CefRequestContextImpl*>(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
+    // May result in |browser_context_impl_| being deleted if no other
-    // global context and no other CefRequestContextImpl are referencing it.
+    // CefRequestContextImpl are referencing it.
-    browser_context_impl_->RemoveRequestContext();
+    browser_context_impl_->RemoveCefRequestContext(this);
   }
 }
 
+// static
+CefRefPtr<CefRequestContextImpl>
+CefRequestContextImpl::CreateGlobalRequestContext(
+    const CefRequestContextSettings& settings) {
+  // Create and initialize the global context immediately.
+  Config config;
+  config.is_global = true;
+  config.settings = settings;
+  CefRefPtr<CefRequestContextImpl> impl = new CefRequestContextImpl(config);
+  impl->Initialize();
+  return impl;
+}
+
 // static
 CefRefPtr<CefRequestContextImpl>
 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);
+  return CefRequestContextImpl::GetOrCreateRequestContext(config);
-}
-
-// static
-CefRefPtr<CefRequestContextImpl> CefRequestContextImpl::CreateForBrowserContext(
-      CefBrowserContext* browser_context) {
-  DCHECK(browser_context);
-  CefRequestContextImpl::Config config;
-  config.handler = browser_context->GetHandler();
-  CefRefPtr<CefRequestContextImpl> 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;
 }
 
 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<CefRequestContext> 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<cef_errorcode_t>(retval);
 }
 
+// static
+CefRefPtr<CefRequestContextImpl>
+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) {
+  if (config_.other) {
-    // Share storage with |other_browser_context|.
+    // 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();
+  request_context_getter_impl_ =
-  DCHECK(request_context_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_)));
   }
 }
 

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<CefRequestContextImpl> CreateGlobalRequestContext(
+      const CefRequestContextSettings& settings);
+
   // Returns a CefRequestContextImpl for the specified |request_context|.
   // Will return the global context if |request_context| is NULL.
   static CefRefPtr<CefRequestContextImpl> GetOrCreateForRequestContext(
       CefRefPtr<CefRequestContext> request_context);
 
-  // Returns a CefRequestContextImpl for the specified |browser_context|.
-  // |browser_context| must be non-NULL.
-  static CefRefPtr<CefRequestContextImpl> 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<CefRequestContextImpl> 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);

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 = "<html><script>function doPopup() { window.open('" +
-                       std::string(kPopupNavPopupUrl) + "', '" +
-                       std::string(kPopupNavPopupName) +
-                       "'); }</script>Page</html>";
-    AddResource(kPopupNavPageUrl, page, "text/html");
-    AddResource(kPopupNavPopupUrl, "<html>Popup</html>", "text/html");
-    if (mode_ == NAVIGATE_AFTER_CREATION)
-      AddResource(kPopupNavPopupUrl2, "<html>Popup2</html>", "text/html");
-
-    // Create the browser.
-    CreateBrowser(kPopupNavPageUrl);
-
-    // Time out the test after a reasonable period of time.
-    SetTestTimeout();
-  }
-
-  bool OnBeforePopup(CefRefPtr<CefBrowser> browser,
-                     CefRefPtr<CefFrame> 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<CefClient>& 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<CefBrowser> 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<CefBrowser> browser,
-                   CefRefPtr<CefFrame> 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<CefBrowser> browser,
-                   CefRefPtr<CefFrame> 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<CefBrowser> browser,
-                 CefRefPtr<CefFrame> 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<PopupNavTestHandler> handler =
-      new PopupNavTestHandler(PopupNavTestHandler::ALLOW);
-  handler->ExecuteTest();
-  ReleaseAndWaitForDestructor(handler);
-}
-
-// Test denying popups.
-TEST(NavigationTest, PopupDeny) {
-  CefRefPtr<PopupNavTestHandler> 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<PopupNavTestHandler> handler =
-      new PopupNavTestHandler(PopupNavTestHandler::NAVIGATE_AFTER_CREATION);
-  handler->ExecuteTest();
-  ReleaseAndWaitForDestructor(handler);
-}
-
-
 namespace {
 
 const char kSimultPopupMainUrl[] = "http://www.tests-sp.com/main.html";

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<CefRequestContext> 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_TRUE(context2->IsSame(context1));
-  EXPECT_FALSE(context1->IsSame(context2));
+  EXPECT_TRUE(context1->IsSame(context2));
   EXPECT_TRUE(context2->IsSharingWith(context2));
   EXPECT_TRUE(context2->IsSharingWith(context1));
   EXPECT_TRUE(context1->IsSharingWith(context2));
-
-  CefRefPtr<CefRequestContext> 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<CefRequestContext> 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 = "<html><script>function doPopup() { window.open('" +
+                       std::string(kPopupNavPopupUrl) + "', '" +
+                       std::string(kPopupNavPopupName) +
+                       "'); }</script>Page</html>";
+    AddResource(kPopupNavPageUrl, page, "text/html");
+    AddResource(kPopupNavPopupUrl, "<html>Popup</html>", "text/html");
+    if (mode_ == NAVIGATE_AFTER_CREATION)
+      AddResource(kPopupNavPopupUrl2, "<html>Popup2</html>", "text/html");
+
+    CefRefPtr<CefRequestContext> request_context;
+    CefRefPtr<CefRequestContextHandler> 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<CefBrowser> browser,
+                     CefRefPtr<CefFrame> 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<CefClient>& 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<CefBrowser> 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<CefBrowser> browser,
+                   CefRefPtr<CefFrame> 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<CefBrowser> browser,
+                   CefRefPtr<CefFrame> 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<CefBrowser> browser,
+                 CefRefPtr<CefFrame> 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<CefBrowser> 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<PopupNavTestHandler> 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";

tests/ceftests/test_handler.cc

@@ -222,8 +222,6 @@ void TestHandler::OnAfterCreated(CefRefPtr<CefBrowser> 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()) {
@@ -232,12 +230,10 @@ void TestHandler::OnAfterCreated(CefRefPtr<CefBrowser> browser) {
   }
   browser_map_.insert(std::make_pair(browser_id, browser));
 }
-}
 
 void TestHandler::OnBeforeClose(CefRefPtr<CefBrowser> 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);
@@ -254,7 +250,6 @@ void TestHandler::OnBeforeClose(CefRefPtr<CefBrowser> browser) {
     // 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);