Fix a crash when calling CefShutdown() before destroying all browser windows (issue #159).

git-svn-id: https://chromiumembedded.googlecode.com/svn/trunk@157 5089003a-bbd8-11dd-ad1f-f1f9622dbc98

libcef/browser_impl.cc

@@ -483,8 +483,11 @@ bool CefBrowser::CreateBrowser(CefWindowInfo& windowInfo, bool popup,
                                CefRefPtr<CefHandler> handler,
                                const CefString& url)
 {
-  if(!_Context.get())
+  // Verify that the context is in a valid state.
+  if (!CONTEXT_STATE_VALID()) {
+    NOTREACHED();
     return false;
+  }
 
   CefString newUrl = url;
   CefBrowserSettings settings(_Context->browser_defaults());
@@ -510,8 +513,17 @@ bool CefBrowser::CreateBrowser(CefWindowInfo& windowInfo, bool popup,
 CefRefPtr<CefBrowser> CefBrowser::CreateBrowserSync(CefWindowInfo& windowInfo,
     bool popup, CefRefPtr<CefHandler> handler, const CefString& url)
 {
-  if(!_Context.get() || !CefThread::CurrentlyOn(CefThread::UI))
+  // Verify that the context is in a valid state.
+  if (!CONTEXT_STATE_VALID()) {
+    NOTREACHED();
     return NULL;
+  }
+
+  // Verify that this method is being called on the UI thread.
+  if (!CefThread::CurrentlyOn(CefThread::UI)) {
+    NOTREACHED();
+    return NULL;
+  }
 
   CefString newUrl = url;
   CefRefPtr<CefBrowser> alternateBrowser;

libcef/cef_context.cc

@@ -21,7 +21,7 @@ CefRefPtr<CefContext> _Context;
 bool CefInitialize(const CefSettings& settings,
                    const CefBrowserSettings& browser_defaults)
 {
-  // Return true if the context is already initialized
+  // Return true if the global context already exists.
   if(_Context.get())
     return true;
 
@@ -31,33 +31,48 @@ bool CefInitialize(const CefSettings& settings,
     return false;
   }
 
-  // Create the new global context object
+  // Create the new global context object.
   _Context = new CefContext();
 
-  // Initialize the global context
+  // Initialize the global context.
   return _Context->Initialize(settings, browser_defaults);
 }
 
 void CefShutdown()
 {
-  // Verify that the context is already initialized
-  if(!_Context.get())
+  // Verify that the context is in a valid state.
+  if (!CONTEXT_STATE_VALID()) {
+    NOTREACHED();
     return;
+  }
 
-  // Shut down the global context
+  // Must always be called on the same thread as Initialize.
+  if(!_Context->process()->CalledOnValidThread()) {
+    NOTREACHED();
+    return;
+  }
+
+  // Shut down the global context. This will block until shutdown is complete.
   _Context->Shutdown();
-  // Delete the global context object
+
+  // Delete the global context object.
   _Context = NULL;
 }
 
 void CefDoMessageLoopWork()
 {
-  // Verify that the context is already initialized.
-  if(!_Context.get() || !_Context->process())
+  // Verify that the context is in a valid state.
+  if (!CONTEXT_STATE_VALID()) {
+    NOTREACHED();
     return;
+  }
 
-  if(!_Context->process()->CalledOnValidThread())
+  // Must always be called on the same thread as Initialize.
+  if(!_Context->process()->CalledOnValidThread()) {
+    NOTREACHED();
     return;
+  }
+
   _Context->process()->DoMessageLoopIteration();
 }
 
@@ -104,8 +119,11 @@ static void UIT_RegisterPlugin(struct CefPluginInfo* plugin_info)
 
 bool CefRegisterPlugin(const struct CefPluginInfo& plugin_info)
 {
-  if(!_Context.get())
+  // Verify that the context is in a valid state.
+  if (!CONTEXT_STATE_VALID()) {
+    NOTREACHED();
     return false;
+  }
 
   CefPluginInfo* pPluginInfo = new CefPluginInfo;
   *pPluginInfo = plugin_info;
@@ -173,15 +191,15 @@ bool CefPostDelayedTask(CefThreadId threadId, CefRefPtr<CefTask> task,
 
 // CefContext
 
-CefContext::CefContext() : process_(NULL)
+CefContext::CefContext() : initialized_(false), shutting_down_(false)
 {
   
 }
 
 CefContext::~CefContext()
 {
-  // Just in case CefShutdown() isn't called
-  Shutdown();
+  if(!shutting_down_)
+    Shutdown();
 }
 
 bool CefContext::Initialize(const CefSettings& settings,
@@ -200,19 +218,38 @@ bool CefContext::Initialize(const CefSettings& settings,
   process_ = new CefProcess(settings_.multi_threaded_message_loop);
   process_->CreateChildThreads();
 
+  initialized_ = true;
+
   return true;
 }
 
 void CefContext::Shutdown()
 {
-  // Remove all existing browsers.
-  //RemoveAllBrowsers();
+  // Must always be called on the same thread as Initialize.
+  DCHECK(process_->CalledOnValidThread());
+  
+  shutting_down_ = true;
 
-  // Deleting the process will destroy the child threads.
+  if(settings_.multi_threaded_message_loop) {
+    // Event that will be used to signal when shutdown is complete. Start in
+    // non-signaled mode so that the event will block.
+    base::WaitableEvent event(false, false);
+
+    // Finish shutdown on the UI thread.
+    CefThread::PostTask(CefThread::UI, FROM_HERE,
+        NewRunnableMethod(this, &CefContext::UIT_FinishShutdown, &event));
+
+    // Block until shutdown is complete.
+    event.Wait();
+  } else {
+    // Finish shutdown on the current thread, which should be the UI thread.
+    UIT_FinishShutdown(NULL);
+  }
+
+  // Delete the process to destroy the child threads.
   process_ = NULL;
 }
 
-
 bool CefContext::AddBrowser(CefRefPtr<CefBrowserImpl> browser)
 {
   bool found = false;
@@ -286,3 +323,27 @@ CefRefPtr<CefBrowserImpl> CefContext::GetBrowserByID(int id)
 
   return browser;
 }
+
+void CefContext::UIT_FinishShutdown(base::WaitableEvent* event)
+{
+  DCHECK(CefThread::CurrentlyOn(CefThread::UI));
+
+  BrowserList list;
+
+  Lock();
+  if (!browserlist_.empty()) {
+    list = browserlist_;
+    browserlist_.clear();
+  }
+  Unlock();
+
+  // Destroy any remaining browser windows.
+  if (!list.empty()) {
+    BrowserList::iterator it = list.begin();
+    for(; it != list.end(); ++it)
+      (*it)->UIT_DestroyBrowser();
+  }
+
+  if(event)
+    event->Signal();
+}

libcef/cef_context.h

@@ -33,6 +33,12 @@ public:
                   const CefBrowserSettings& browser_defaults);
   void Shutdown();
 
+  // Returns true if the context is initialized.
+  bool initialized() { return initialized_; }
+
+  // Returns true if the context is shutting down.
+  bool shutting_down() { return shutting_down_; }
+
   scoped_refptr<CefProcess> process() { return process_; }
 
   bool AddBrowser(CefRefPtr<CefBrowserImpl> browser);
@@ -59,7 +65,17 @@ public:
     { storage_context_.reset(storage_context); }
   DOMStorageContext* storage_context() { return storage_context_.get(); }
 
+  static bool ImplementsThreadSafeReferenceCounting() { return true; }
+
 private:
+  // Performs shutdown actions that need to occur on the UI thread before any
+  // threads are destroyed.
+  void UIT_FinishShutdown(base::WaitableEvent* event);
+
+  // Track context state.
+  bool initialized_;
+  bool shutting_down_;
+
   // Manages the various process threads.
   scoped_refptr<CefProcess> process_;
 
@@ -80,7 +96,11 @@ private:
   int next_browser_id_;
 };
 
-// Global context object pointer
+// Global context object pointer.
 extern CefRefPtr<CefContext> _Context;
 
+// Helper macro that returns true if the global context is in a valid state.
+#define CONTEXT_STATE_VALID() \
+    (_Context.get() && _Context->initialized() && !_Context->shutting_down())
+
 #endif // _CEF_CONTEXT_H

libcef/scheme_impl.cc

@@ -393,9 +393,11 @@ bool CefRegisterScheme(const CefString& scheme_name,
                        const CefString& host_name,
                        CefRefPtr<CefSchemeHandlerFactory> factory)
 {
-  // Verify that the context is already initialized
-  if(!_Context.get())
+  // Verify that the context is in a valid state.
+  if (!CONTEXT_STATE_VALID()) {
+    NOTREACHED();
     return false;
+  }
 
   // Use a smart pointer for the wrapper object because
   // RunnableMethodTraits::RetainCallee() (originating from NewRunnableMethod)

libcef/v8_impl.cc

@@ -143,12 +143,16 @@ bool CefRegisterExtension(const CefString& extension_name,
                           const CefString& javascript_code,
                           CefRefPtr<CefV8Handler> handler)
 {
-  // Verify that the context is already initialized
-  if(!_Context.get())
+  // Verify that the context is in a valid state.
+  if (!CONTEXT_STATE_VALID()) {
+    NOTREACHED();
     return false;
-  
-  if(!handler.get())
+  }
+
+  if(!handler.get()) {
+    NOTREACHED();
     return false;
+  }
 
   TrackString* name = new TrackString(extension_name);
   TrackAdd(name);