- Clean up implementation of modal window callbacks (issue #281).

- Fix CefBrowser::GoForward documentation typo (issue #291).

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

include/cef.h

@@ -557,6 +557,14 @@ public:
                                                  const CefString& url,
                                                  const CefBrowserSettings& settings);
 
+  ///
+  // Call this method before destroying a contained browser window. This method
+  // performs any internal cleanup that may be needed before the browser window
+  // is destroyed.
+  ///
+  /*--cef()--*/
+  virtual void ParentWindowWillClose() =0;
+
   ///
   // Closes this browser window.
   ///
@@ -579,7 +587,7 @@ public:
   /*--cef()--*/
   virtual bool CanGoForward() =0;
   ///
-  // Navigate backwards.
+  // Navigate forwards.
   ///
   /*--cef()--*/
   virtual void GoForward() =0;
@@ -980,28 +988,30 @@ public:
   virtual void OnAfterCreated(CefRefPtr<CefBrowser> browser) {}
 
   ///
-  // Called just before a window is closed. If this is a modal window and you
-  // handled the RunModal() event you can use this callback to restore
-  // the opener window to a usable state.
-  ///
-  /*--cef()--*/
-  virtual void OnBeforeClose(CefRefPtr<CefBrowser> browser) {}
-
-  ///
-  // Called to enter the modal loop. Provide your own modal loop here. Return
-  // true if you ran your own modal loop and false to use the default. You can
-  // also use this event to know when a modal window is about to start.
+  // Called when a modal window is about to display and the modal loop should
+  // begin running. Return false to use the default modal loop implementation or
+  // true to use a custom implementation.
   ///
   /*--cef()--*/
   virtual bool RunModal(CefRefPtr<CefBrowser> browser) { return false; }
 
   ///
-  // Called when a modal browser window has been destroyed. You must implement
-  // this if you are handling RunModal(). You can also use this event to know
-  // when a modal window is about to be closed.
+  // Called when a window has recieved a request to close. Return false to
+  // proceed with the window close or true to cancel the window close. If this
+  // is a modal window and a custom modal loop implementation was provided in
+  // RunModal() this callback should be used to restore the opener window to a
+  // usable state.
   ///
   /*--cef()--*/
-  virtual void QuitModal(CefRefPtr<CefBrowser> browser) { }
+  virtual bool DoClose(CefRefPtr<CefBrowser> browser) { return false; }
+
+  ///
+  // Called just before a window is closed. If this is a modal window and a
+  // custom modal loop implementation was provided in RunModal() this callback
+  // should be used to exit the custom modal loop.
+  ///
+  /*--cef()--*/
+  virtual void OnBeforeClose(CefRefPtr<CefBrowser> browser) {}
 };
 
 

include/cef_capi.h

@@ -399,6 +399,13 @@ typedef struct _cef_browser_t
   // Base structure.
   cef_base_t base;
 
+  ///
+  // Call this function before destroying a contained browser window. This
+  // function performs any internal cleanup that may be needed before the
+  // browser window is destroyed.
+  ///
+  void (CEF_CALLBACK *parent_window_will_close)(struct _cef_browser_t* self);
+
   ///
   // Closes this browser window.
   ///
@@ -420,7 +427,7 @@ typedef struct _cef_browser_t
   int (CEF_CALLBACK *can_go_forward)(struct _cef_browser_t* self);
 
   ///
-  // Navigate backwards.
+  // Navigate forwards.
   ///
   void (CEF_CALLBACK *go_forward)(struct _cef_browser_t* self);
 
@@ -823,27 +830,29 @@ typedef struct _cef_life_span_handler_t
       struct _cef_browser_t* browser);
 
   ///
-  // Called just before a window is closed. If this is a modal window and you
-  // handled the run_modal() event you can use this callback to restore the
-  // opener window to a usable state.
-  ///
-  void (CEF_CALLBACK *on_before_close)(struct _cef_life_span_handler_t* self,
-      struct _cef_browser_t* browser);
-
-  ///
-  // Called to enter the modal loop. Provide your own modal loop here. Return
-  // true (1) if you ran your own modal loop and false (0) to use the default.
-  // You can also use this event to know when a modal window is about to start.
+  // Called when a modal window is about to display and the modal loop should
+  // begin running. Return false (0) to use the default modal loop
+  // implementation or true (1) to use a custom implementation.
   ///
   int (CEF_CALLBACK *run_modal)(struct _cef_life_span_handler_t* self,
       struct _cef_browser_t* browser);
 
   ///
-  // Called when a modal browser window has been destroyed. You must implement
-  // this if you are handling run_modal(). You can also use this event to know
-  // when a modal window is about to be closed.
+  // Called when a window has recieved a request to close. Return false (0) to
+  // proceed with the window close or true (1) to cancel the window close. If
+  // this is a modal window and a custom modal loop implementation was provided
+  // in run_modal() this callback should be used to restore the opener window to
+  // a usable state.
   ///
-  void (CEF_CALLBACK *quit_modal)(struct _cef_life_span_handler_t* self,
+  int (CEF_CALLBACK *do_close)(struct _cef_life_span_handler_t* self,
+      struct _cef_browser_t* browser);
+
+  ///
+  // Called just before a window is closed. If this is a modal window and a
+  // custom modal loop implementation was provided in run_modal() this callback
+  // should be used to exit the custom modal loop.
+  ///
+  void (CEF_CALLBACK *on_before_close)(struct _cef_life_span_handler_t* self,
       struct _cef_browser_t* browser);
 
 } cef_life_span_handler_t;

libcef/browser_impl.cc

@@ -680,30 +680,6 @@ WebFrame* CefBrowserImpl::UIT_GetWebFrame(CefRefPtr<CefFrame> frame)
 
 void CefBrowserImpl::UIT_DestroyBrowser()
 {
-#if defined(OS_WIN)
-  if (is_modal_) {
-    // Exit our own internal modal message loop now.
-    if (internal_modal_message_loop_is_active_) {
-      MessageLoop* message_loop = MessageLoop::current();
-      message_loop->QuitNow();
-    }
-
-    // If the client implemented its own modal loop then the above would not
-    // run, so this call is for the client to exit its loop. Otherwise, 
-    // QuitModal can be used to let clients know that the modal loop is about 
-    // to exit.
-    if (client_.get()) {
-      CefRefPtr<CefLifeSpanHandler> handler = client_->GetLifeSpanHandler();
-      if (handler.get()) {
-        // Notify the handler that it can exit its modal loop if it was in one.
-        handler->QuitModal(this);
-      }
-    }
-  }
-
-  UIT_GetWebViewDelegate()->RevokeDragDrop();
-#else
-  // Call OnBeforeClose() here for platforms that don't support modal dialogs.
   if (client_.get()) {
     CefRefPtr<CefLifeSpanHandler> handler = client_->GetLifeSpanHandler();
     if (handler.get()) {
@@ -711,6 +687,17 @@ void CefBrowserImpl::UIT_DestroyBrowser()
       handler->OnBeforeClose(this);
     }
   }
+
+#if defined(OS_WIN)
+  if (is_modal_) {
+    // Exit our own internal modal message loop now.
+    if (internal_modal_message_loop_is_active_) {
+      MessageLoop* message_loop = MessageLoop::current();
+      message_loop->QuitNow();
+    }
+  }
+
+  UIT_GetWebViewDelegate()->RevokeDragDrop();
 #endif
 
   // If the current browser window is a dev tools client then disconnect from
@@ -747,15 +734,6 @@ void CefBrowserImpl::UIT_CloseBrowser()
 {
   REQUIRE_UIT();
   if (IsWindowRenderingDisabled()) {
-    // There is no window here so we need to notify the client that this
-    // browser instance is about to go away.
-    if (client_.get()) {
-      CefRefPtr<CefLifeSpanHandler> handler = client_->GetLifeSpanHandler();
-      if (handler.get()) {
-        // Notify the handler that the window is about to be closed.
-        handler->OnBeforeClose(this);
-      }
-    }
     UIT_DestroyBrowser();
   } else {
     UIT_CloseView(UIT_GetMainWndHandle());

libcef/browser_impl.h

@@ -71,6 +71,7 @@ public:
   virtual void ReloadIgnoreCache() OVERRIDE;
   virtual void StopLoad() OVERRIDE;
   virtual void SetFocus(bool enable) OVERRIDE;
+  virtual void ParentWindowWillClose() OVERRIDE;
   virtual CefWindowHandle GetWindowHandle() OVERRIDE;
   virtual CefWindowHandle GetOpenerWindowHandle() OVERRIDE
       { return opener_window(); }

libcef/browser_impl_gtk.cc

@@ -18,6 +18,11 @@
 using WebKit::WebRect;
 using WebKit::WebSize;
 
+void CefBrowserImpl::ParentWindowWillClose()
+{
+  // TODO(port): Implement this method if necessary.
+}
+
 CefWindowHandle CefBrowserImpl::GetWindowHandle()
 {
   AutoLock lock_scope(this);

libcef/browser_impl_mac.mm

@@ -19,6 +19,11 @@
 using WebKit::WebRect;
 using WebKit::WebSize;
 
+void CefBrowserImpl::ParentWindowWillClose()
+{
+  // TODO(port): Implement this method if necessary.
+}
+
 CefWindowHandle CefBrowserImpl::GetWindowHandle()
 {
   AutoLock lock_scope(this);

libcef/browser_impl_win.cc

@@ -40,31 +40,23 @@ LRESULT CALLBACK CefBrowserImpl::WndProc(HWND hwnd, UINT message,
 
   switch (message) {
   case WM_CLOSE:
-    // It is the responsibility of the client to send this message to this
-    // window if it is created as a child window of a client's frame window.
-    // This is particularly important when this window is modal.
     if(browser) {
-      // Here the window really is about to get closed.
+      bool handled(false);
+
       if (browser->client_.get()) {
-        CefRefPtr<CefLifeSpanHandler> handler = 
+        CefRefPtr<CefLifeSpanHandler> handler =
             browser->client_->GetLifeSpanHandler();
         if (handler.get()) {
-          // Notify the handler that the window is about to be closed.
-          handler->OnBeforeClose(browser);
+          // Give the client a chance to handle this one.
+          handled = handler->DoClose(browser);
         }
       }
 
-      // We must re-enable the opener (owner of the modal window)
-      // before we close the popup to avoid focus/activation/z-order issues.
-      if (browser->opener_ && browser->opener_was_disabled_by_modal_loop_) {
-        HWND owner = ::GetAncestor(browser->opener_, GA_ROOT);
-        ::EnableWindow(owner, TRUE);
-      }
-
-      // Don't do the default if this is a contained window as the destruction
-      // will occur when the parent frame window is destroyed.
-      if (::GetAncestor(hwnd, GA_ROOT) != hwnd)
+      if (handled)
         return 0;
+
+      // We are our own parent in this case.
+      browser->ParentWindowWillClose();
     }
     break;
 
@@ -82,7 +74,7 @@ LRESULT CALLBACK CefBrowserImpl::WndProc(HWND hwnd, UINT message,
     if (browser && browser->UIT_GetWebView()) {
       // resize the web view window to the full size of the browser window
       RECT rc;
-      GetClientRect(browser->UIT_GetMainWndHandle(), &rc);
+      GetClientRect(hwnd, &rc);
       MoveWindow(browser->UIT_GetWebViewWndHandle(), 0, 0, rc.right, rc.bottom,
           TRUE);
     }
@@ -100,6 +92,16 @@ LRESULT CALLBACK CefBrowserImpl::WndProc(HWND hwnd, UINT message,
   return DefWindowProc(hwnd, message, wParam, lParam);
 }
 
+void CefBrowserImpl::ParentWindowWillClose()
+{
+  // We must re-enable the opener (owner of the modal window) before we close
+  // the popup to avoid focus/activation/z-order issues.
+  if (opener_ && opener_was_disabled_by_modal_loop_) {
+    HWND owner = ::GetAncestor(opener_, GA_ROOT);
+    ::EnableWindow(owner, TRUE);
+  }
+}
+
 CefWindowHandle CefBrowserImpl::GetWindowHandle()
 {
   AutoLock lock_scope(this);

libcef/browser_webview_delegate_win.cc

@@ -404,7 +404,7 @@ void BrowserWebViewDelegate::showContextMenu(
   if (!browser_->IsWindowRenderingDisabled()) {
     // Perform the conversion to screen coordinates only if window rendering is
     // enabled.
-    MapWindowPoints(browser_->UIT_GetMainWndHandle(), HWND_DESKTOP,
+    MapWindowPoints(browser_->UIT_GetWebViewWndHandle(), HWND_DESKTOP,
         &mouse_pt, 1);
     screenX = mouse_pt.x;
     screenY = mouse_pt.y;

libcef_dll/cpptoc/browser_cpptoc.cc

@@ -69,6 +69,15 @@ CEF_EXPORT cef_browser_t* cef_browser_create_sync(cef_window_info_t* windowInfo,
 
 // MEMBER FUNCTIONS - Body may be edited by hand.
 
+void CEF_CALLBACK browser_parent_window_will_close(struct _cef_browser_t* self)
+{
+  DCHECK(self);
+  if (!self)
+    return;
+
+  CefBrowserCppToC::Get(self)->ParentWindowWillClose();
+}
+
 void CEF_CALLBACK browser_close_browser(struct _cef_browser_t* self)
 {
   DCHECK(self);
@@ -450,6 +459,7 @@ void CEF_CALLBACK browser_send_capture_lost_event(struct _cef_browser_t* self)
 CefBrowserCppToC::CefBrowserCppToC(CefBrowser* cls)
     : CefCppToC<CefBrowserCppToC, CefBrowser, cef_browser_t>(cls)
 {
+  struct_.struct_.parent_window_will_close = browser_parent_window_will_close;
   struct_.struct_.close_browser = browser_close_browser;
   struct_.struct_.can_go_back = browser_can_go_back;
   struct_.struct_.go_back = browser_go_back;

libcef_dll/cpptoc/life_span_handler_cpptoc.cc

@@ -85,18 +85,6 @@ void CEF_CALLBACK life_span_handler_on_after_created(
       CefBrowserCToCpp::Wrap(browser));
 }
 
-void CEF_CALLBACK life_span_handler_on_before_close(
-    struct _cef_life_span_handler_t* self, cef_browser_t* browser)
-{
-  DCHECK(self);
-  DCHECK(browser);
-  if (!self || !browser)
-    return;
-
-  CefLifeSpanHandlerCppToC::Get(self)->OnBeforeClose(
-      CefBrowserCToCpp::Wrap(browser));
-}
-
 int CEF_CALLBACK life_span_handler_run_modal(
     struct _cef_life_span_handler_t* self, cef_browser_t* browser)
 {
@@ -109,7 +97,19 @@ int CEF_CALLBACK life_span_handler_run_modal(
       CefBrowserCToCpp::Wrap(browser));
 }
 
-void CEF_CALLBACK life_span_handler_quit_modal(
+int CEF_CALLBACK life_span_handler_do_close(
+    struct _cef_life_span_handler_t* self, cef_browser_t* browser)
+{
+  DCHECK(self);
+  DCHECK(browser);
+  if (!self || !browser)
+    return 0;
+
+  return CefLifeSpanHandlerCppToC::Get(self)->DoClose(
+      CefBrowserCToCpp::Wrap(browser));
+}
+
+void CEF_CALLBACK life_span_handler_on_before_close(
     struct _cef_life_span_handler_t* self, cef_browser_t* browser)
 {
   DCHECK(self);
@@ -117,7 +117,7 @@ void CEF_CALLBACK life_span_handler_quit_modal(
   if (!self || !browser)
     return;
 
-  CefLifeSpanHandlerCppToC::Get(self)->QuitModal( 
+  CefLifeSpanHandlerCppToC::Get(self)->OnBeforeClose(
       CefBrowserCToCpp::Wrap(browser));
 }
 
@@ -130,9 +130,9 @@ CefLifeSpanHandlerCppToC::CefLifeSpanHandlerCppToC(CefLifeSpanHandler* cls)
 {
   struct_.struct_.on_before_popup = life_span_handler_on_before_popup;
   struct_.struct_.on_after_created = life_span_handler_on_after_created;
-  struct_.struct_.on_before_close = life_span_handler_on_before_close;
   struct_.struct_.run_modal = life_span_handler_run_modal;
-  struct_.struct_.quit_modal = life_span_handler_quit_modal;
+  struct_.struct_.do_close = life_span_handler_do_close;
+  struct_.struct_.on_before_close = life_span_handler_on_before_close;
 }
 
 #ifndef NDEBUG

libcef_dll/ctocpp/browser_ctocpp.cc

@@ -40,6 +40,14 @@ CefRefPtr<CefBrowser> CefBrowser::CreateBrowserSync(CefWindowInfo& windowInfo,
 
 // VIRTUAL METHODS - Body may be edited by hand.
 
+void CefBrowserCToCpp::ParentWindowWillClose()
+{
+  if (CEF_MEMBER_MISSING(struct_, parent_window_will_close))
+    return;
+
+  struct_->parent_window_will_close(struct_);
+}
+
 void CefBrowserCToCpp::CloseBrowser()
 {
   if(CEF_MEMBER_MISSING(struct_, close_browser))

libcef_dll/ctocpp/browser_ctocpp.h

@@ -31,6 +31,7 @@ public:
   virtual ~CefBrowserCToCpp() {}
 
   // CefBrowser methods
+  virtual void ParentWindowWillClose() OVERRIDE;
   virtual void CloseBrowser() OVERRIDE;
   virtual bool CanGoBack() OVERRIDE;
   virtual void GoBack() OVERRIDE;

libcef_dll/ctocpp/life_span_handler_ctocpp.cc

@@ -55,14 +55,6 @@ void CefLifeSpanHandlerCToCpp::OnAfterCreated(CefRefPtr<CefBrowser> browser)
   struct_->on_after_created(struct_, CefBrowserCppToC::Wrap(browser));
 }
 
-void CefLifeSpanHandlerCToCpp::OnBeforeClose(CefRefPtr<CefBrowser> browser)
-{
-  if (CEF_MEMBER_MISSING(struct_, on_before_close))
-    return;
-
-  struct_->on_before_close(struct_, CefBrowserCppToC::Wrap(browser));
-}
-
 bool CefLifeSpanHandlerCToCpp::RunModal(CefRefPtr<CefBrowser> browser)
 {
   if (CEF_MEMBER_MISSING(struct_, run_modal))
@@ -73,12 +65,22 @@ bool CefLifeSpanHandlerCToCpp::RunModal(CefRefPtr<CefBrowser> browser)
   return (rv ? true : false);
 }
 
-void CefLifeSpanHandlerCToCpp::QuitModal(CefRefPtr<CefBrowser> browser)
+bool CefLifeSpanHandlerCToCpp::DoClose(CefRefPtr<CefBrowser> browser)
 {
-  if (CEF_MEMBER_MISSING(struct_, quit_modal))
+  if (CEF_MEMBER_MISSING(struct_, do_close))
+    return false;
+
+  int rv = struct_->do_close(struct_, CefBrowserCppToC::Wrap(browser));
+
+  return (rv ? true : false);
+}
+
+void CefLifeSpanHandlerCToCpp::OnBeforeClose(CefRefPtr<CefBrowser> browser)
+{
+  if (CEF_MEMBER_MISSING(struct_, on_before_close))
     return;
 
-  struct_->quit_modal(struct_, CefBrowserCppToC::Wrap(browser));
+  struct_->on_before_close(struct_, CefBrowserCppToC::Wrap(browser));
 }
 
 

libcef_dll/ctocpp/life_span_handler_ctocpp.h

@@ -38,9 +38,9 @@ public:
       const CefString& url, CefRefPtr<CefClient>& client,
       CefBrowserSettings& settings) OVERRIDE;
   virtual void OnAfterCreated(CefRefPtr<CefBrowser> browser) OVERRIDE;
-  virtual void OnBeforeClose(CefRefPtr<CefBrowser> browser) OVERRIDE;
   virtual bool RunModal(CefRefPtr<CefBrowser> browser) OVERRIDE;
-  virtual void QuitModal(CefRefPtr<CefBrowser> browser) OVERRIDE;
+  virtual bool DoClose(CefRefPtr<CefBrowser> browser) OVERRIDE;
+  virtual void OnBeforeClose(CefRefPtr<CefBrowser> browser) OVERRIDE;
 };
 
 #endif // BUILDING_CEF_SHARED

tests/cefclient/cefclient_win.cpp

@@ -612,17 +612,15 @@ LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
       break;
 
     case WM_CLOSE:
-      // All clients must forward the WM_CLOSE call to all contained browser
-      // windows to give those windows a chance to cleanup before the window
-      // closes. Don't forward this message if you are cancelling the request.
-      if(g_handler.get())
-      {
-        HWND hWnd = g_handler->GetBrowserHwnd();
-        if (hWnd)
-          ::SendMessage(hWnd, WM_CLOSE, 0, 0);
+      if (g_handler.get()) {
+        CefRefPtr<CefBrowser> browser = g_handler->GetBrowser();
+        if (browser.get()) {
+          // Let the browser window know we are about to destroy it.
+          browser->ParentWindowWillClose();
+        }
       }
       break;
-    
+
     case WM_DESTROY:
       // The frame window has exited
       PostQuitMessage(0);

tests/cefclient/client_handler.cpp

@@ -42,6 +42,26 @@ void ClientHandler::OnAfterCreated(CefRefPtr<CefBrowser> browser)
   }
 }
 
+bool ClientHandler::DoClose(CefRefPtr<CefBrowser> browser)
+{
+  REQUIRE_UI_THREAD();
+
+  if (!browser->IsPopup()) {
+    // Since the main window contains the browser window, we need to close
+    // the parent window instead of the browser window.
+    CloseMainWindow();
+
+    // Return true here so that we can skip closing the browser window 
+    // in this pass. (It will be destroyed due to the call to close
+    // the parent above.)
+    return true;
+  }
+
+  // A popup browser window is not contained in another window, so we can let
+  // these windows close by themselves.
+  return false;
+}
+
 void ClientHandler::OnBeforeClose(CefRefPtr<CefBrowser> browser)
 {
   REQUIRE_UI_THREAD();

tests/cefclient/client_handler.h

@@ -51,8 +51,9 @@ public:
                                CefRefPtr<CefClient>& client,
                                CefBrowserSettings& settings) OVERRIDE;
   virtual void OnAfterCreated(CefRefPtr<CefBrowser> browser) OVERRIDE;
+  virtual bool DoClose(CefRefPtr<CefBrowser> browser) OVERRIDE;
   virtual void OnBeforeClose(CefRefPtr<CefBrowser> browser) OVERRIDE;
-  
+
   // CefLoadHandler methods
   virtual void OnLoadStart(CefRefPtr<CefBrowser> browser,
                            CefRefPtr<CefFrame> frame) OVERRIDE;
@@ -146,6 +147,7 @@ public:
     NOTIFY_DOWNLOAD_ERROR,
   };
   void SendNotification(NotificationType type);
+  void CloseMainWindow();
 
 protected:
   void SetLoading(bool isLoading);

tests/cefclient/client_handler_mac.mm

@@ -141,3 +141,8 @@ void ClientHandler::SetNavState(bool canGoBack, bool canGoForward)
 {
   // TODO(port): Change button status.
 }
+
+void ClientHandler::CloseMainWindow()
+{
+  // TODO(port): Close window
+}

tests/cefclient/client_handler_win.cpp

@@ -162,3 +162,8 @@ void ClientHandler::SetNavState(bool canGoBack, bool canGoForward)
   EnableWindow(m_BackHwnd, canGoBack);
   EnableWindow(m_ForwardHwnd, canGoForward);
 }
+
+void ClientHandler::CloseMainWindow()
+{
+  ::PostMessage(m_MainHwnd, WM_CLOSE, 0, 0);
+}

tests/cefclient/res/modalmain.html

@@ -5,8 +5,9 @@
 </head> 
 <body>
 
-<h3>Test Modal Dialog</h3>
-Click this <button onclick="doModal()">button</button> to open a modal dialog.
+<h3>Tests</h3>
+<button onclick="doModal()">Open the modal dialog</button><br>
+<button onclick="window.close()">Close this window</button>
 
 <h3>Time (timers are suppresed while the modal dialog is open)</h3>
 <div id="time"></div>