- Change the way that application shutdown works in order to support JavaScript 'onbeforeunload' handling (issue #853). See comments in cef_life_span_handler.h for a detailed description of the new shutdown process.

- Fix a crash on Linux during window destruction (issue #681).

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

libcef/browser/browser_host_impl.cc

@@ -459,19 +459,35 @@ CefRefPtr<CefBrowser> CefBrowserHostImpl::GetBrowser() {
   return this;
 }
 
-void CefBrowserHostImpl::CloseBrowser() {
+void CefBrowserHostImpl::CloseBrowser(bool force_close) {
   if (CEF_CURRENTLY_ON_UIT()) {
-    if (IsWindowRenderingDisabled()) {
-      if (AllowDestroyBrowser()) {
-        ParentWindowWillClose();
-        DestroyBrowser();
+    // Exit early if a close attempt is already pending and this method is
+    // called again from somewhere other than WindowDestroyed().
+    if (destruction_state_ >= DESTRUCTION_STATE_PENDING &&
+        (IsWindowRenderingDisabled() || !window_destroyed_)) {
+      if (force_close && destruction_state_ == DESTRUCTION_STATE_PENDING) {
+        // Upgrade the destruction state.
+        destruction_state_ = DESTRUCTION_STATE_ACCEPTED;
       }
+      return;
+    }
+
+    if (destruction_state_ < DESTRUCTION_STATE_ACCEPTED) {
+      destruction_state_ = (force_close ? DESTRUCTION_STATE_ACCEPTED :
+                                          DESTRUCTION_STATE_PENDING);
+    }
+
+    content::WebContents* contents = web_contents();
+    if (contents && contents->NeedToFireBeforeUnload()) {
+      // Will result in a call to BeforeUnloadFired() and, if the close isn't
+      // canceled, CloseContents().
+      contents->GetRenderViewHost()->FirePageBeforeUnload(false);
     } else {
-      PlatformCloseWindow();
+      CloseContents(contents);
     }
   } else {
     CEF_POST_TASK(CEF_UIT,
-        base::Bind(&CefBrowserHostImpl::CloseBrowser, this));
+        base::Bind(&CefBrowserHostImpl::CloseBrowser, this, force_close));
   }
 }
 
@@ -1024,22 +1040,18 @@ bool CefBrowserHostImpl::SendProcessMessage(
 // CefBrowserHostImpl public methods.
 // -----------------------------------------------------------------------------
 
-bool CefBrowserHostImpl::AllowDestroyBrowser() {
-  if (client_.get()) {
-    CefRefPtr<CefLifeSpanHandler> handler =
-        client_->GetLifeSpanHandler();
-    if (handler.get()) {
-      // Give the client a chance to handle this one.
-      return !handler->DoClose(this);
-    }
-  }
-
-  return true;
+void CefBrowserHostImpl::WindowDestroyed() {
+  CEF_REQUIRE_UIT();
+  DCHECK(!window_destroyed_);
+  window_destroyed_ = true;
+  CloseBrowser(true);
 }
 
 void CefBrowserHostImpl::DestroyBrowser() {
   CEF_REQUIRE_UIT();
 
+  destruction_state_ = DESTRUCTION_STATE_COMPLETED;
+
   if (client_.get()) {
     CefRefPtr<CefLifeSpanHandler> handler = client_->GetLifeSpanHandler();
     if (handler.get()) {
@@ -1412,7 +1424,41 @@ void CefBrowserHostImpl::LoadingStateChanged(content::WebContents* source) {
 }
 
 void CefBrowserHostImpl::CloseContents(content::WebContents* source) {
-  PlatformCloseWindow();
+  if (destruction_state_ == DESTRUCTION_STATE_COMPLETED)
+    return;
+
+  bool close_browser = true;
+
+  // If this method is called in response to something other than
+  // WindowDestroyed() ask the user if the browser should close.
+  if (IsWindowRenderingDisabled() || !window_destroyed_) {
+    CefRefPtr<CefLifeSpanHandler> handler =
+        client_->GetLifeSpanHandler();
+    if (handler.get()) {
+      close_browser = !handler->DoClose(this);
+    }
+  }
+
+  if (close_browser) {
+    if (destruction_state_ != DESTRUCTION_STATE_ACCEPTED)
+      destruction_state_ = DESTRUCTION_STATE_ACCEPTED;
+
+    if (!IsWindowRenderingDisabled() && !window_destroyed_) {
+      // A window exists so try to close it using the platform method. Will
+      // result in a call to WindowDestroyed() if/when the window is destroyed
+      // via the platform window destruction mechanism.
+      PlatformCloseWindow();
+    } else {
+      // No window exists. Destroy the browser immediately.
+      DestroyBrowser();
+      if (!IsWindowRenderingDisabled()) {
+        // Release the reference added in PlatformCreateWindow().
+        Release();
+      }
+    }
+  } else if (destruction_state_ != DESTRUCTION_STATE_NONE) {
+    destruction_state_ = DESTRUCTION_STATE_NONE;
+  }
 }
 
 void CefBrowserHostImpl::UpdateTargetURL(content::WebContents* source,
@@ -1439,6 +1485,17 @@ bool CefBrowserHostImpl::AddMessageToConsole(content::WebContents* source,
   return false;
 }
 
+void CefBrowserHostImpl::BeforeUnloadFired(content::WebContents* source,
+                                           bool proceed,
+                                           bool* proceed_to_fire_unload) {
+  if (destruction_state_ == DESTRUCTION_STATE_ACCEPTED || proceed) {
+    *proceed_to_fire_unload = true;
+  } else if (!proceed) {
+    *proceed_to_fire_unload = false;
+    destruction_state_ = DESTRUCTION_STATE_NONE;
+  }
+}
+
 bool CefBrowserHostImpl::TakeFocus(content::WebContents* source,
                                    bool reverse) {
   if (client_.get()) {
@@ -1948,6 +2005,8 @@ CefBrowserHostImpl::CefBrowserHostImpl(
       queue_messages_(true),
       main_frame_id_(CefFrameHostImpl::kInvalidFrameId),
       focused_frame_id_(CefFrameHostImpl::kInvalidFrameId),
+      destruction_state_(DESTRUCTION_STATE_NONE),
+      window_destroyed_(false),
       is_in_onsetfocus_(false),
       focus_on_editable_field_(false),
       file_chooser_pending_(false) {

libcef/browser/browser_host_impl.h

@@ -109,7 +109,7 @@ class CefBrowserHostImpl : public CefBrowserHost,
 
   // CefBrowserHost methods.
   virtual CefRefPtr<CefBrowser> GetBrowser() OVERRIDE;
-  virtual void CloseBrowser() OVERRIDE;
+  virtual void CloseBrowser(bool force_close) OVERRIDE;
   virtual void ParentWindowWillClose() OVERRIDE;
   virtual void SetFocus(bool enable) OVERRIDE;
   virtual CefWindowHandle GetWindowHandle() OVERRIDE;
@@ -165,9 +165,8 @@ class CefBrowserHostImpl : public CefBrowserHost,
       CefProcessId target_process,
       CefRefPtr<CefProcessMessage> message) OVERRIDE;
 
-  // Call LifeSpanHandler before destroying. Returns true if destruction
-  // is allowed at this time.
-  bool AllowDestroyBrowser();
+  // Called when the OS window hosting the browser is destroyed.
+  void WindowDestroyed();
 
   // Destroy the browser members. This method should only be called after the
   // native browser window is not longer processing messages.
@@ -253,6 +252,14 @@ class CefBrowserHostImpl : public CefBrowserHost,
   // Returns false if a popup is already pending.
   bool SetPendingPopupInfo(scoped_ptr<PendingPopupInfo> info);
 
+  enum DestructionState {
+    DESTRUCTION_STATE_NONE = 0,
+    DESTRUCTION_STATE_PENDING,
+    DESTRUCTION_STATE_ACCEPTED,
+    DESTRUCTION_STATE_COMPLETED
+  };
+  DestructionState destruction_state() const { return destruction_state_; }
+
  private:
   // content::WebContentsDelegate methods.
   virtual content::WebContents* OpenURLFromTab(
@@ -268,6 +275,9 @@ class CefBrowserHostImpl : public CefBrowserHost,
                                    const string16& message,
                                    int32 line_no,
                                    const string16& source_id) OVERRIDE;
+  virtual void BeforeUnloadFired(content::WebContents* source,
+                                 bool proceed,
+                                 bool* proceed_to_fire_unload) OVERRIDE;
   virtual bool TakeFocus(content::WebContents* source,
                          bool reverse) OVERRIDE;
   virtual void WebContentsFocused(content::WebContents* contents) OVERRIDE;
@@ -483,6 +493,14 @@ class CefBrowserHostImpl : public CefBrowserHost,
   // Used when no other frame exists. Provides limited functionality.
   CefRefPtr<CefFrameHostImpl> placeholder_frame_;
 
+  // Represents the current browser destruction state. Only accessed on the UI
+  // thread.
+  DestructionState destruction_state_;
+
+  // True if the OS window hosting the browser has been destroyed. Only accessed
+  // on the UI thread.
+  bool window_destroyed_;
+
   // True if currently in the OnSetFocus callback. Only accessed on the UI
   // thread.
   bool is_in_onsetfocus_;

libcef/browser/browser_host_impl_gtk.cc

@@ -22,15 +22,38 @@
 namespace {
 
 void DestroyBrowser(CefRefPtr<CefBrowserHostImpl> browser) {
-  browser->DestroyBrowser();
-  browser->Release();
+  // Force the browser to be destroyed and release the reference added in
+  // PlatformCreateWindow().
+  browser->WindowDestroyed();
 }
 
-void window_destroyed(GtkWidget* widget, CefBrowserHostImpl* browser) {
+void browser_destroy(GtkWidget* widget, CefBrowserHostImpl* browser) {
   // Destroy the browser host after window destruction is complete.
   CEF_POST_TASK(CEF_UIT, base::Bind(DestroyBrowser, browser));
 }
 
+void window_destroy(GtkWidget* widget, gpointer data) {
+}
+
+gboolean window_delete_event(GtkWidget* widget, GdkEvent* event,
+                             CefBrowserHostImpl* browser) {
+  // Protect against multiple requests to close while the close is pending.
+  if (browser && browser->destruction_state() <=
+      CefBrowserHostImpl::DESTRUCTION_STATE_PENDING) {
+    if (browser->destruction_state() ==
+        CefBrowserHostImpl::DESTRUCTION_STATE_NONE) {
+      // Request that the browser close.
+      browser->CloseBrowser(false);
+    }
+
+    // Cancel the close.
+    return TRUE;
+  }
+
+  // Allow the close.
+  return FALSE;
+}
+
 std::string GetDescriptionFromMimeType(const std::string& mime_type) {
   // Check for wild card mime types and return an appropriate description.
   static const struct {
@@ -225,6 +248,11 @@ bool CefBrowserHostImpl::PlatformCreateWindow() {
     gtk_window_set_position(GTK_WINDOW(window), GTK_WIN_POS_CENTER);
     gtk_widget_show_all(GTK_WIDGET(window));
 
+    g_signal_connect(G_OBJECT(window), "destroy",
+                     G_CALLBACK(window_destroy), NULL);
+    g_signal_connect(G_OBJECT(window), "delete_event",
+                     G_CALLBACK(window_delete_event), this);
+
     window_info_.parent_widget = parentView;
   }
 
@@ -237,7 +265,7 @@ bool CefBrowserHostImpl::PlatformCreateWindow() {
                     window_info_.widget);
 
   g_signal_connect(G_OBJECT(window_info_.widget), "destroy",
-                   G_CALLBACK(window_destroyed), this);
+                   G_CALLBACK(browser_destroy), this);
 
   // As an additional requirement on Linux, we must set the colors for the
   // render widgets in webkit.
@@ -260,7 +288,14 @@ void CefBrowserHostImpl::PlatformCloseWindow() {
   if (window_info_.widget != NULL) {
     GtkWidget* window =
         gtk_widget_get_toplevel(GTK_WIDGET(window_info_.widget));
-    gtk_signal_emit_by_name(GTK_OBJECT(window), "delete_event");
+
+    // Send the "delete_event" signal.
+    GdkEvent event;
+    memset(&event, 0, sizeof(GdkEvent));
+    event.any.type = GDK_DELETE;
+    event.any.send_event = TRUE;
+    event.any.window = window->window;
+    gtk_main_do_event(&event);
   }
 }
 

libcef/browser/browser_host_impl_mac.mm

@@ -42,8 +42,9 @@
 
 - (void) dealloc {
   if (browser_) {
-    browser_->DestroyBrowser();
-    browser_->Release();
+    // Force the browser to be destroyed and release the reference added in
+    // PlatformCreateWindow().
+    browser_->WindowDestroyed();
   }
 
   [super dealloc];
@@ -68,6 +69,51 @@
 
 @end
 
+// Receives notifications from the browser window. Will delete itself when done.
+@interface CefWindowDelegate : NSObject <NSWindowDelegate> {
+ @private
+  CefBrowserHostImpl* browser_;  // weak
+}
+
+@property (nonatomic, assign) CefBrowserHostImpl* browser;
+
+@end
+
+@implementation CefWindowDelegate
+
+@synthesize browser = browser_;
+
+- (BOOL)windowShouldClose:(id)window {
+  // Protect against multiple requests to close while the close is pending.
+  if (browser_ && browser_->destruction_state() <=
+      CefBrowserHostImpl::DESTRUCTION_STATE_PENDING) {
+    if (browser_->destruction_state() ==
+        CefBrowserHostImpl::DESTRUCTION_STATE_NONE) {
+      // Request that the browser close.
+      browser_->CloseBrowser(false);
+    }
+
+    // Cancel the close.
+    return NO;
+  }
+
+  // Clean ourselves up after clearing the stack of anything that might have the
+  // window on it.
+  [self performSelectorOnMainThread:@selector(cleanup:)
+                         withObject:window
+                      waitUntilDone:NO];
+
+  // Allow the close.
+  return YES;
+}
+
+- (void)cleanup:(id)window {
+  [self release];
+}
+
+@end
+
+
 namespace {
 
 // Accept-types to file-types helper.
@@ -231,6 +277,10 @@ bool CefBrowserHostImpl::PlatformCreateWindow() {
     contentRect.size.width = window_rect.size.width;
     contentRect.size.height = window_rect.size.height;
 
+    // Create the delegate for control and browser window events.
+    CefWindowDelegate* delegate = [[CefWindowDelegate alloc] init];
+    delegate.browser = this;
+
     newWnd = [[UnderlayOpenGLHostingWindow alloc]
               initWithContentRect:window_rect
               styleMask:(NSTitledWindowMask |
@@ -240,6 +290,7 @@ bool CefBrowserHostImpl::PlatformCreateWindow() {
                          NSUnifiedTitleAndToolbarWindowMask )
               backing:NSBackingStoreBuffered
               defer:NO];
+    [newWnd setDelegate:delegate];
     parentView = [newWnd contentView];
     window_info_.parent_view = parentView;
   }

libcef/browser/browser_host_impl_win.cc

@@ -456,30 +456,24 @@ LPCTSTR CefBrowserHostImpl::GetWndClass() {
 
 // static
 LRESULT CALLBACK CefBrowserHostImpl::WndProc(HWND hwnd, UINT message,
-                                         WPARAM wParam, LPARAM lParam) {
+                                             WPARAM wParam, LPARAM lParam) {
   CefBrowserHostImpl* browser =
       static_cast<CefBrowserHostImpl*>(ui::GetWindowUserData(hwnd));
 
   switch (message) {
   case WM_CLOSE:
-    if (browser) {
-      bool handled(false);
-
-      if (browser->client_.get()) {
-        CefRefPtr<CefLifeSpanHandler> handler =
-            browser->client_->GetLifeSpanHandler();
-        if (handler.get()) {
-          // Give the client a chance to handle this one.
-          handled = handler->DoClose(browser);
-        }
+    // Protect against multiple requests to close while the close is pending.
+    if (browser && browser->destruction_state() <= DESTRUCTION_STATE_PENDING) {
+      if (browser->destruction_state() == DESTRUCTION_STATE_NONE) {
+        // Request that the browser close.
+        browser->CloseBrowser(false);
       }
 
-      if (handled)
-        return 0;
-
-      // We are our own parent in this case.
-      browser->ParentWindowWillClose();
+      // Cancel the close.
+      return 0;
     }
+
+    // Allow the close.
     break;
 
   case WM_DESTROY:
@@ -487,11 +481,9 @@ LRESULT CALLBACK CefBrowserHostImpl::WndProc(HWND hwnd, UINT message,
       // Clear the user data pointer.
       ui::SetWindowUserData(hwnd, NULL);
 
-      // Destroy the browser.
-      browser->DestroyBrowser();
-
-      // Release the reference added in PlatformCreateWindow().
-      browser->Release();
+      // Force the browser to be destroyed and release the reference added in
+      // PlatformCreateWindow().
+      browser->WindowDestroyed();
     }
     return 0;
 
@@ -574,8 +566,10 @@ bool CefBrowserHostImpl::PlatformCreateWindow() {
 }
 
 void CefBrowserHostImpl::PlatformCloseWindow() {
-  if (window_info_.window != NULL)
-    PostMessage(window_info_.window, WM_CLOSE, 0, 0);
+  if (window_info_.window != NULL) {
+    HWND frameWnd = GetAncestor(window_info_.window, GA_ROOT);
+    PostMessage(frameWnd, WM_CLOSE, 0, 0);
+  }
 }
 
 void CefBrowserHostImpl::PlatformSizeTo(int width, int height) {

libcef/browser/context.cc

@@ -34,10 +34,6 @@
 #include "sandbox/win/src/sandbox_types.h"
 #endif
 
-// Both the CefContext constuctor and the CefContext::RemoveBrowser method need
-// to initialize or reset to the same value.
-const int kNextBrowserIdReset = 1;
-
 // Global CefContext pointer
 CefRefPtr<CefContext> _Context;
 

libcef/browser/javascript_dialog_manager.cc

@@ -136,6 +136,14 @@ void CefJavaScriptDialogManager::RunBeforeUnloadDialog(
     const string16& message_text,
     bool is_reload,
     const DialogClosedCallback& callback) {
+  if (browser_->destruction_state() >=
+      CefBrowserHostImpl::DESTRUCTION_STATE_ACCEPTED) {
+    // Currently destroying the browser. Accept the unload without showing
+    // the prompt.
+    callback.Run(true, string16());
+    return;
+  }
+
   CefRefPtr<CefClient> client = browser_->GetClient();
   if (client.get()) {
     CefRefPtr<CefJSDialogHandler> handler = client->GetJSDialogHandler();