From 6e05c14db2955a46bf900b67cae812441d7dd984 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Thu, 17 Oct 2024 11:05:30 -0400 Subject: [PATCH] chrome: Support unload handlers with TryCloseBrowser TryCloseBrowser should potentially trigger JavaScript unload handlers and return false. Add CefBrowserHost::IsReadyToBeClosed for detecting mandatory close state. Enable, for Chrome style browsers, LifeSpanTest that don't require DoClose(). Update related documentation. --- include/capi/cef_browser_capi.h | 65 ++++++++++---- include/capi/cef_life_span_handler_capi.h | 85 +++++++++++-------- include/cef_api_hash.h | 8 +- include/cef_browser.h | 63 ++++++++++---- include/cef_life_span_handler.h | 78 ++++++++++------- libcef/browser/browser_host_base.cc | 14 +++ libcef/browser/browser_host_base.h | 1 + .../chrome/chrome_browser_host_impl.cc | 47 ++++++++-- .../browser/chrome/chrome_browser_host_impl.h | 2 +- libcef_dll/cpptoc/browser_host_cpptoc.cc | 21 ++++- libcef_dll/ctocpp/browser_host_ctocpp.cc | 23 ++++- libcef_dll/ctocpp/browser_host_ctocpp.h | 3 +- tests/ceftests/life_span_unittest.cc | 56 ++++++++++-- tests/ceftests/test_handler.cc | 2 + 14 files changed, 347 insertions(+), 121 deletions(-) diff --git a/include/capi/cef_browser_capi.h b/include/capi/cef_browser_capi.h index 041a827a9..b5dd342e1 100644 --- a/include/capi/cef_browser_capi.h +++ b/include/capi/cef_browser_capi.h @@ -33,7 +33,7 @@ // by hand. See the translator.README.txt file in the tools directory for // more information. // -// $hash=7c786570b1c7af912a31c6f0c3d742e8aeb38fd8$ +// $hash=e9f34d90eb4af614e35cbb29da0639b62acec7fd$ // #ifndef CEF_INCLUDE_CAPI_CEF_BROWSER_CAPI_H_ @@ -301,29 +301,62 @@ typedef struct _cef_browser_host_t { struct _cef_browser_host_t* self); /// - /// Request that the browser close. The JavaScript 'onbeforeunload' event will - /// be fired. If |force_close| is false (0) the event handler, if any, will be - /// allowed to prompt the user and the user can optionally cancel the close. - /// If |force_close| is true (1) the prompt will not be displayed and the - /// close will proceed. Results in a call to - /// cef_life_span_handler_t::do_close() if the event handler allows the close - /// or if |force_close| is true (1). See cef_life_span_handler_t::do_close() - /// documentation for additional usage information. + /// Request that the browser close. Closing a browser is a multi-stage process + /// that may complete either synchronously or asynchronously, and involves + /// callbacks such as cef_life_span_handler_t::DoClose (Alloy style only), + /// cef_life_span_handler_t::OnBeforeClose, and a top-level window close + /// handler such as cef_window_delegate_t::CanClose (or platform-specific + /// equivalent). In some cases a close request may be delayed or canceled by + /// the user. Using try_close_browser() instead of close_browser() is + /// recommended for most use cases. See cef_life_span_handler_t::do_close() + /// documentation for detailed usage and examples. + /// + /// If |force_close| is false (0) then JavaScript unload handlers, if any, may + /// be fired and the close may be delayed or canceled by the user. If + /// |force_close| is true (1) then the user will not be prompted and the close + /// will proceed immediately (possibly asynchronously). If browser close is + /// delayed and not canceled the default behavior is to call the top-level + /// window close handler once the browser is ready to be closed. This default + /// behavior can be changed for Alloy style browsers by implementing + /// cef_life_span_handler_t::do_close(). is_ready_to_be_closed() can be used + /// to detect mandatory browser close events when customizing close behavior + /// on the browser process UI thread. /// void(CEF_CALLBACK* close_browser)(struct _cef_browser_host_t* self, int force_close); /// - /// Helper for closing a browser. Call this function from the top-level window - /// close handler (if any). Internally this calls CloseBrowser(false (0)) if - /// the close has not yet been initiated. This function returns false (0) - /// while the close is pending and true (1) after the close has completed. See - /// close_browser() and cef_life_span_handler_t::do_close() documentation for - /// additional usage information. This function must be called on the browser - /// process UI thread. + /// Helper for closing a browser. This is similar in behavior to + /// CLoseBrowser(false (0)) but returns a boolean to reflect the immediate + /// close status. Call this function from a top-level window close handler + /// such as cef_window_delegate_t::CanClose (or platform-specific equivalent) + /// to request that the browser close, and return the result to indicate if + /// the window close should proceed. Returns false (0) if the close will be + /// delayed (JavaScript unload handlers triggered but still pending) or true + /// (1) if the close will proceed immediately (possibly asynchronously). See + /// close_browser() documentation for additional usage information. This + /// function must be called on the browser process UI thread. /// int(CEF_CALLBACK* try_close_browser)(struct _cef_browser_host_t* self); + /// + /// Returns true (1) if the browser is ready to be closed, meaning that the + /// close has already been initiated and that JavaScript unload handlers have + /// already executed or should be ignored. This can be used from a top-level + /// window close handler such as cef_window_delegate_t::CanClose (or platform- + /// specific equivalent) to distringuish between potentially cancelable + /// browser close events (like the user clicking the top-level window close + /// button before browser close has started) and mandatory browser close + /// events (like JavaScript `window.close()` or after browser close has + /// started in response to [Try]close_browser()). Not completing the browser + /// close for mandatory close events (when this function returns true (1)) + /// will leave the browser in a partially closed state that interferes with + /// proper functioning. See close_browser() documentation for additional usage + /// information. This function must be called on the browser process UI + /// thread. + /// + int(CEF_CALLBACK* is_ready_to_be_closed)(struct _cef_browser_host_t* self); + /// /// Set whether the browser is focused. /// diff --git a/include/capi/cef_life_span_handler_capi.h b/include/capi/cef_life_span_handler_capi.h index f78453852..f67bce990 100644 --- a/include/capi/cef_life_span_handler_capi.h +++ b/include/capi/cef_life_span_handler_capi.h @@ -33,7 +33,7 @@ // by hand. See the translator.README.txt file in the tools directory for // more information. // -// $hash=5232dd6bf16af9b6d195a47bb41de0dfb880a65e$ +// $hash=6aad2ccf30a6c519bbeee64d83866e82a41a48d8$ // #ifndef CEF_INCLUDE_CAPI_CEF_LIFE_SPAN_HANDLER_CAPI_H_ @@ -138,35 +138,44 @@ typedef struct _cef_life_span_handler_t { struct _cef_browser_t* browser); /// - /// Called when a browser has received a request to close. This may result - /// directly from a call to cef_browser_host_t::*close_browser() or indirectly - /// if the browser is parented to a top-level window created by CEF and the - /// user attempts to close that window (by clicking the 'X', for example). The - /// do_close() function will be called after the JavaScript 'onunload' event - /// has been fired. + /// Called when an Alloy style browser is ready to be closed, meaning that the + /// close has already been initiated and that JavaScript unload handlers have + /// already executed or should be ignored. This may result directly from a + /// call to cef_browser_host_t::[Try]close_browser() or indirectly if the + /// browser's top-level parent window was created by CEF and the user attempts + /// to close that window (by clicking the 'X', for example). do_close() will + /// not be called if the browser's host window/view has already been destroyed + /// (via parent window/view hierarchy tear-down, for example), as it is no + /// longer possible to customize the close behavior at that point. /// - /// An application should handle top-level owner window close notifications by - /// calling cef_browser_host_t::try_close_browser() or + /// An application should handle top-level parent window close notifications + /// by calling cef_browser_host_t::try_close_browser() or /// cef_browser_host_t::CloseBrowser(false (0)) instead of allowing the window /// to close immediately (see the examples below). This gives CEF an - /// opportunity to process the 'onbeforeunload' event and optionally cancel + /// opportunity to process JavaScript unload handlers and optionally cancel /// the close before do_close() is called. /// - /// When windowed rendering is enabled CEF will internally create a window or - /// view to host the browser. In that case returning false (0) from do_close() - /// will send the standard close notification to the browser's top-level owner - /// window (e.g. WM_CLOSE on Windows, performClose: on OS X, "delete_event" on - /// Linux or cef_window_delegate_t::can_close() callback from Views). If the - /// browser's host window/view has already been destroyed (via view hierarchy - /// tear-down, for example) then do_close() will not be called for that - /// browser since is no longer possible to cancel the close. + /// When windowed rendering is enabled CEF will create an internal child + /// window/view to host the browser. In that case returning false (0) from + /// do_close() will send the standard close notification to the browser's top- + /// level parent window (e.g. WM_CLOSE on Windows, performClose: on OS X, + /// "delete_event" on Linux or cef_window_delegate_t::can_close() callback + /// from Views). /// - /// When windowed rendering is disabled returning false (0) from do_close() - /// will cause the browser object to be destroyed immediately. + /// When windowed rendering is disabled there is no internal window/view and + /// returning false (0) from do_close() will cause the browser object to be + /// destroyed immediately. /// - /// If the browser's top-level owner window requires a non-standard close + /// If the browser's top-level parent window requires a non-standard close /// notification then send that notification from do_close() and return true - /// (1). + /// (1). You are still required to complete the browser close as soon as + /// possible (either by calling [Try]close_browser() or by proceeding with + /// window/view hierarchy tear-down), otherwise the browser will be left in a + /// partially closed state that interferes with proper functioning. Top-level + /// windows created on the browser process UI thread can alternately call + /// cef_browser_host_t::is_ready_to_be_closed() in the close handler to check + /// close status instead of relying on custom do_close() handling. See + /// documentation on that function for additional details. /// /// The cef_life_span_handler_t::on_before_close() function will be called /// after do_close() (if do_close() is called) and immediately before the @@ -182,22 +191,26 @@ typedef struct _cef_life_span_handler_t { /// which sends a close notification /// to the application's top-level window. /// 2. Application's top-level window receives the close notification and - /// calls TryCloseBrowser() (which internally calls CloseBrowser(false)). + /// calls TryCloseBrowser() (similar to calling CloseBrowser(false)). /// TryCloseBrowser() returns false so the client cancels the window /// close. /// 3. JavaScript 'onbeforeunload' handler executes and shows the close /// confirmation dialog (which can be overridden via /// CefJSDialogHandler::OnBeforeUnloadDialog()). /// 4. User approves the close. 5. JavaScript 'onunload' handler executes. - /// 6. CEF sends a close notification to the application's top-level window - /// (because DoClose() returned false by default). - /// 7. Application's top-level window receives the close notification and + /// 6. Application's do_close() handler is called and returns false (0) by + /// default. + /// 7. CEF sends a close notification to the application's top-level window + /// (because DoClose() returned false). + /// 8. Application's top-level window receives the close notification and /// calls TryCloseBrowser(). TryCloseBrowser() returns true so the client /// allows the window close. - /// 8. Application's top-level window is destroyed. 9. Application's - /// on_before_close() handler is called and the browser object + /// 9. Application's top-level window is destroyed, triggering destruction + /// of the child browser window. + /// 10. Application's on_before_close() handler is called and the browser + /// object /// is destroyed. - /// 10. Application exits by calling cef_quit_message_loop() if no other + /// 11. Application exits by calling cef_quit_message_loop() if no other /// browsers /// exist. /// @@ -215,13 +228,17 @@ typedef struct _cef_life_span_handler_t { /// CefJSDialogHandler::OnBeforeUnloadDialog()). /// 4. User approves the close. 5. JavaScript 'onunload' handler executes. /// 6. Application's do_close() handler is called. Application will: - /// A. Set a flag to indicate that the next close attempt will be allowed. + /// A. Set a flag to indicate that the next top-level window close attempt + /// will be allowed. /// B. Return false. - /// 7. CEF sends an close notification to the application's top-level window. + /// 7. CEF sends a close notification to the application's top-level window + /// (because DoClose() returned false). /// 8. Application's top-level window receives the close notification and - /// allows the window to close based on the flag from #6B. - /// 9. Application's top-level window is destroyed. 10. Application's - /// on_before_close() handler is called and the browser object + /// allows the window to close based on the flag from #6A. + /// 9. Application's top-level window is destroyed, triggering destruction + /// of the child browser window. + /// 10. Application's on_before_close() handler is called and the browser + /// object /// is destroyed. /// 11. Application exits by calling cef_quit_message_loop() if no other /// browsers diff --git a/include/cef_api_hash.h b/include/cef_api_hash.h index 93b4c5507..ce9d77a71 100644 --- a/include/cef_api_hash.h +++ b/include/cef_api_hash.h @@ -42,13 +42,13 @@ // way that may cause binary incompatibility with other builds. The universal // hash value will change if any platform is affected whereas the platform hash // values will change only if that particular platform is affected. -#define CEF_API_HASH_UNIVERSAL "9717d7221d63adfd79ee52e2a31c9ac7acdd6d50" +#define CEF_API_HASH_UNIVERSAL "676af077d6826353caf40425f5f2bae1262347ea" #if defined(OS_WIN) -#define CEF_API_HASH_PLATFORM "072a4fe61a512f21fd0d664495902fca6ec2193b" +#define CEF_API_HASH_PLATFORM "51848171cdea10858c4e0fca0f7099b0fdc759f9" #elif defined(OS_MAC) -#define CEF_API_HASH_PLATFORM "ee7f0e9247add8df0827d02da32559e38e8079d0" +#define CEF_API_HASH_PLATFORM "8cc826c5f5fe97c275dfa3b9c020470678a5d2fd" #elif defined(OS_LINUX) -#define CEF_API_HASH_PLATFORM "88996e58ee062016efd054bfbafd03dd3daa99ed" +#define CEF_API_HASH_PLATFORM "0aec2348de1bf14fafa7a23baa0df942d342d409" #endif #ifdef __cplusplus diff --git a/include/cef_browser.h b/include/cef_browser.h index 1574dbd62..011e51271 100644 --- a/include/cef_browser.h +++ b/include/cef_browser.h @@ -331,30 +331,63 @@ class CefBrowserHost : public virtual CefBaseRefCounted { virtual CefRefPtr GetBrowser() = 0; /// - /// Request that the browser close. The JavaScript 'onbeforeunload' event will - /// be fired. If |force_close| is false the event handler, if any, will be - /// allowed to prompt the user and the user can optionally cancel the close. - /// If |force_close| is true the prompt will not be displayed and the close - /// will proceed. Results in a call to CefLifeSpanHandler::DoClose() if the - /// event handler allows the close or if |force_close| is true. See - /// CefLifeSpanHandler::DoClose() documentation for additional usage - /// information. + /// Request that the browser close. Closing a browser is a multi-stage process + /// that may complete either synchronously or asynchronously, and involves + /// callbacks such as CefLifeSpanHandler::DoClose (Alloy style only), + /// CefLifeSpanHandler::OnBeforeClose, and a top-level window close handler + /// such as CefWindowDelegate::CanClose (or platform-specific equivalent). In + /// some cases a close request may be delayed or canceled by the user. Using + /// TryCloseBrowser() instead of CloseBrowser() is recommended for most use + /// cases. See CefLifeSpanHandler::DoClose() documentation for detailed usage + /// and examples. + /// + /// If |force_close| is false then JavaScript unload handlers, if any, may be + /// fired and the close may be delayed or canceled by the user. If + /// |force_close| is true then the user will not be prompted and the close + /// will proceed immediately (possibly asynchronously). If browser close is + /// delayed and not canceled the default behavior is to call the top-level + /// window close handler once the browser is ready to be closed. This default + /// behavior can be changed for Alloy style browsers by implementing + /// CefLifeSpanHandler::DoClose(). IsReadyToBeClosed() can be used to detect + /// mandatory browser close events when customizing close behavior on the + /// browser process UI thread. /// /*--cef()--*/ virtual void CloseBrowser(bool force_close) = 0; /// - /// Helper for closing a browser. Call this method from the top-level window - /// close handler (if any). Internally this calls CloseBrowser(false) if the - /// close has not yet been initiated. This method returns false while the - /// close is pending and true after the close has completed. See - /// CloseBrowser() and CefLifeSpanHandler::DoClose() documentation for - /// additional usage information. This method must be called on the browser - /// process UI thread. + /// Helper for closing a browser. This is similar in behavior to + /// CLoseBrowser(false) but returns a boolean to reflect the immediate close + /// status. Call this method from a top-level window close handler such as + /// CefWindowDelegate::CanClose (or platform-specific equivalent) to request + /// that the browser close, and return the result to indicate if the window + /// close should proceed. Returns false if the close will be delayed + /// (JavaScript unload handlers triggered but still pending) or true if the + /// close will proceed immediately (possibly asynchronously). See + /// CloseBrowser() documentation for additional usage information. This method + /// must be called on the browser process UI thread. /// /*--cef()--*/ virtual bool TryCloseBrowser() = 0; + /// + /// Returns true if the browser is ready to be closed, meaning that the close + /// has already been initiated and that JavaScript unload handlers have + /// already executed or should be ignored. This can be used from a top-level + /// window close handler such as CefWindowDelegate::CanClose (or + /// platform-specific equivalent) to distringuish between potentially + /// cancelable browser close events (like the user clicking the top-level + /// window close button before browser close has started) and mandatory + /// browser close events (like JavaScript `window.close()` or after browser + /// close has started in response to [Try]CloseBrowser()). Not completing the + /// browser close for mandatory close events (when this method returns true) + /// will leave the browser in a partially closed state that interferes with + /// proper functioning. See CloseBrowser() documentation for additional usage + /// information. This method must be called on the browser process UI thread. + /// + /*--cef()--*/ + virtual bool IsReadyToBeClosed() = 0; + /// /// Set whether the browser is focused. /// diff --git a/include/cef_life_span_handler.h b/include/cef_life_span_handler.h index 45bf84a01..30e17e8a5 100644 --- a/include/cef_life_span_handler.h +++ b/include/cef_life_span_handler.h @@ -131,34 +131,44 @@ class CefLifeSpanHandler : public virtual CefBaseRefCounted { virtual void OnAfterCreated(CefRefPtr browser) {} /// - /// Called when a browser has received a request to close. This may result - /// directly from a call to CefBrowserHost::*CloseBrowser() or indirectly if - /// the browser is parented to a top-level window created by CEF and the user - /// attempts to close that window (by clicking the 'X', for example). The - /// DoClose() method will be called after the JavaScript 'onunload' event has - /// been fired. + /// Called when an Alloy style browser is ready to be closed, meaning that the + /// close has already been initiated and that JavaScript unload handlers have + /// already executed or should be ignored. This may result directly from a + /// call to CefBrowserHost::[Try]CloseBrowser() or indirectly if the browser's + /// top-level parent window was created by CEF and the user attempts to + /// close that window (by clicking the 'X', for example). DoClose() will not + /// be called if the browser's host window/view has already been destroyed + /// (via parent window/view hierarchy tear-down, for example), as it is no + /// longer possible to customize the close behavior at that point. /// - /// An application should handle top-level owner window close notifications by - /// calling CefBrowserHost::TryCloseBrowser() or + /// An application should handle top-level parent window close notifications + /// by calling CefBrowserHost::TryCloseBrowser() or /// CefBrowserHost::CloseBrowser(false) instead of allowing the window to /// close immediately (see the examples below). This gives CEF an opportunity - /// to process the 'onbeforeunload' event and optionally cancel the close + /// to process JavaScript unload handlers and optionally cancel the close /// before DoClose() is called. /// - /// When windowed rendering is enabled CEF will internally create a window or - /// view to host the browser. In that case returning false from DoClose() will - /// send the standard close notification to the browser's top-level owner - /// window (e.g. WM_CLOSE on Windows, performClose: on OS X, "delete_event" on - /// Linux or CefWindowDelegate::CanClose() callback from Views). If the - /// browser's host window/view has already been destroyed (via view hierarchy - /// tear-down, for example) then DoClose() will not be called for that browser - /// since is no longer possible to cancel the close. + /// When windowed rendering is enabled CEF will create an internal child + /// window/view to host the browser. In that case returning false from + /// DoClose() will send the standard close notification to the browser's + /// top-level parent window (e.g. WM_CLOSE on Windows, performClose: on OS X, + /// "delete_event" on Linux or CefWindowDelegate::CanClose() callback from + /// Views). /// - /// When windowed rendering is disabled returning false from DoClose() will - /// cause the browser object to be destroyed immediately. + /// When windowed rendering is disabled there is no internal window/view + /// and returning false from DoClose() will cause the browser object to be + /// destroyed immediately. /// - /// If the browser's top-level owner window requires a non-standard close + /// If the browser's top-level parent window requires a non-standard close /// notification then send that notification from DoClose() and return true. + /// You are still required to complete the browser close as soon as possible + /// (either by calling [Try]CloseBrowser() or by proceeding with window/view + /// hierarchy tear-down), otherwise the browser will be left in a partially + /// closed state that interferes with proper functioning. Top-level windows + /// created on the browser process UI thread can alternately call + /// CefBrowserHost::IsReadyToBeClosed() in the close handler to check close + /// status instead of relying on custom DoClose() handling. See documentation + /// on that method for additional details. /// /// The CefLifeSpanHandler::OnBeforeClose() method will be called after /// DoClose() (if DoClose() is called) and immediately before the browser @@ -174,7 +184,7 @@ class CefLifeSpanHandler : public virtual CefBaseRefCounted { /// 1. User clicks the window close button which sends a close notification /// to the application's top-level window. /// 2. Application's top-level window receives the close notification and - /// calls TryCloseBrowser() (which internally calls CloseBrowser(false)). + /// calls TryCloseBrowser() (similar to calling CloseBrowser(false)). /// TryCloseBrowser() returns false so the client cancels the window /// close. /// 3. JavaScript 'onbeforeunload' handler executes and shows the close @@ -182,15 +192,18 @@ class CefLifeSpanHandler : public virtual CefBaseRefCounted { /// CefJSDialogHandler::OnBeforeUnloadDialog()). /// 4. User approves the close. /// 5. JavaScript 'onunload' handler executes. - /// 6. CEF sends a close notification to the application's top-level window - /// (because DoClose() returned false by default). - /// 7. Application's top-level window receives the close notification and + /// 6. Application's DoClose() handler is called and returns false by + /// default. + /// 7. CEF sends a close notification to the application's top-level window + /// (because DoClose() returned false). + /// 8. Application's top-level window receives the close notification and /// calls TryCloseBrowser(). TryCloseBrowser() returns true so the client /// allows the window close. - /// 8. Application's top-level window is destroyed. - /// 9. Application's OnBeforeClose() handler is called and the browser object + /// 9. Application's top-level window is destroyed, triggering destruction + /// of the child browser window. + /// 10. Application's OnBeforeClose() handler is called and the browser object /// is destroyed. - /// 10. Application exits by calling CefQuitMessageLoop() if no other browsers + /// 11. Application exits by calling CefQuitMessageLoop() if no other browsers /// exist. /// /// Example 2: Using CefBrowserHost::CloseBrowser(false) and implementing the @@ -208,12 +221,15 @@ class CefLifeSpanHandler : public virtual CefBaseRefCounted { /// 4. User approves the close. /// 5. JavaScript 'onunload' handler executes. /// 6. Application's DoClose() handler is called. Application will: - /// A. Set a flag to indicate that the next close attempt will be allowed. + /// A. Set a flag to indicate that the next top-level window close attempt + /// will be allowed. /// B. Return false. - /// 7. CEF sends an close notification to the application's top-level window. + /// 7. CEF sends a close notification to the application's top-level window + /// (because DoClose() returned false). /// 8. Application's top-level window receives the close notification and - /// allows the window to close based on the flag from #6B. - /// 9. Application's top-level window is destroyed. + /// allows the window to close based on the flag from #6A. + /// 9. Application's top-level window is destroyed, triggering destruction + /// of the child browser window. /// 10. Application's OnBeforeClose() handler is called and the browser object /// is destroyed. /// 11. Application exits by calling CefQuitMessageLoop() if no other browsers diff --git a/libcef/browser/browser_host_base.cc b/libcef/browser/browser_host_base.cc index 6cf749ebc..907092c84 100644 --- a/libcef/browser/browser_host_base.cc +++ b/libcef/browser/browser_host_base.cc @@ -452,6 +452,20 @@ bool CefBrowserHostBase::HasView() { return is_views_hosted_; } +bool CefBrowserHostBase::IsReadyToBeClosed() { + if (!CEF_CURRENTLY_ON_UIT()) { + DCHECK(false) << "called on invalid thread"; + return false; + } + + if (auto web_contents = GetWebContents()) { + return static_cast( + web_contents->GetPrimaryMainFrame()) + ->IsPageReadyToBeClosed(); + } + return true; +} + void CefBrowserHostBase::SetFocus(bool focus) { if (!CEF_CURRENTLY_ON_UIT()) { CEF_POST_TASK(CEF_UIT, diff --git a/libcef/browser/browser_host_base.h b/libcef/browser/browser_host_base.h index ba1439ad7..ee7147812 100644 --- a/libcef/browser/browser_host_base.h +++ b/libcef/browser/browser_host_base.h @@ -218,6 +218,7 @@ class CefBrowserHostBase : public CefBrowserHost, double GetZoomLevel() override; void SetZoomLevel(double zoomLevel) override; bool HasView() override; + bool IsReadyToBeClosed() override; void SetFocus(bool focus) override; void RunFileDialog(FileDialogMode mode, const CefString& title, diff --git a/libcef/browser/chrome/chrome_browser_host_impl.cc b/libcef/browser/chrome/chrome_browser_host_impl.cc index a36a875ef..c3bd9766b 100644 --- a/libcef/browser/chrome/chrome_browser_host_impl.cc +++ b/libcef/browser/chrome/chrome_browser_host_impl.cc @@ -165,14 +165,47 @@ void ChromeBrowserHostImpl::OnSetFocus(cef_focus_source_t source) { } void ChromeBrowserHostImpl::CloseBrowser(bool force_close) { + if (!CEF_CURRENTLY_ON_UIT()) { + CEF_POST_TASK(CEF_UIT, base::BindOnce(&ChromeBrowserHostImpl::CloseBrowser, + this, force_close)); + return; + } + + if (!force_close) { + TryCloseBrowser(); + return; + } + // Always do this asynchronously because TabStripModel is not re-entrant. - CEF_POST_TASK(CEF_UIT, base::BindOnce(&ChromeBrowserHostImpl::DoCloseBrowser, - this, force_close)); + CEF_POST_TASK(CEF_UIT, + base::BindOnce(&ChromeBrowserHostImpl::DoCloseBrowser, this)); } bool ChromeBrowserHostImpl::TryCloseBrowser() { - // TODO(chrome): Handle the case where the browser may not close immediately. - CloseBrowser(true); + if (!CEF_CURRENTLY_ON_UIT()) { + DCHECK(false) << "called on invalid thread"; + return false; + } + + if (auto* web_contents = GetWebContents()) { + // This check works as follows: + // 1. Returns false if the main frame is ready to close + // (IsPageReadyToBeClosed returns true). + // 2. Otherwise returns true if any frame in the frame tree needs to run + // beforeunload or unload-time event handlers. + // 3. Otherwise returns false. + if (web_contents->NeedToFireBeforeUnloadOrUnloadEvents()) { + // Will result in a call to Browser::BeforeUnloadFired and, if the close + // isn't canceled, Browser::CloseContents which indirectly calls + // TabStripModel::CloseWebContentsAt (similar to DoCloseBrowser but + // without CLOSE_USER_GESTURE). Additional calls to DispatchBeforeUnload + // while the unload is pending will be ignored. + web_contents->DispatchBeforeUnload(/*auto_cancel=*/false); + return false; + } + } + + CloseBrowser(/*force_close=*/true); return true; } @@ -551,7 +584,7 @@ void ChromeBrowserHostImpl::DestroyBrowser() { // WebContents first. See comments on CefBrowserHostBase::DestroyBrowser. if (GetWebContents()) { // Triggers a call to OnWebContentsDestroyed. - DoCloseBrowser(/*force_close=*/true); + DoCloseBrowser(); DCHECK(!GetWebContents()); } @@ -565,15 +598,13 @@ void ChromeBrowserHostImpl::DestroyBrowser() { CefBrowserHostBase::DestroyBrowser(); } -void ChromeBrowserHostImpl::DoCloseBrowser(bool force_close) { +void ChromeBrowserHostImpl::DoCloseBrowser() { CEF_REQUIRE_UIT(); if (browser_) { // Like chrome::CloseTab() but specifying the WebContents. const int tab_index = GetCurrentTabIndex(); if (tab_index != TabStripModel::kNoTab) { // This will trigger destruction of the Browser and WebContents. - // TODO(chrome): Handle the case where this method returns false, - // indicating that the contents were not closed immediately. browser_->tab_strip_model()->CloseWebContentsAt( tab_index, TabCloseTypes::CLOSE_CREATE_HISTORICAL_TAB | TabCloseTypes::CLOSE_USER_GESTURE); diff --git a/libcef/browser/chrome/chrome_browser_host_impl.h b/libcef/browser/chrome/chrome_browser_host_impl.h index 9c807360f..8d039ef13 100644 --- a/libcef/browser/chrome/chrome_browser_host_impl.h +++ b/libcef/browser/chrome/chrome_browser_host_impl.h @@ -169,7 +169,7 @@ class ChromeBrowserHostImpl : public CefBrowserHostBase { bool WillBeDestroyed() const override; void DestroyBrowser() override; - void DoCloseBrowser(bool force_close); + void DoCloseBrowser(); // Returns the current tab index for the associated WebContents, or // TabStripModel::kNoTab if not found. diff --git a/libcef_dll/cpptoc/browser_host_cpptoc.cc b/libcef_dll/cpptoc/browser_host_cpptoc.cc index d46036cd4..ffa1ba749 100644 --- a/libcef_dll/cpptoc/browser_host_cpptoc.cc +++ b/libcef_dll/cpptoc/browser_host_cpptoc.cc @@ -9,7 +9,7 @@ // implementations. See the translator.README.txt file in the tools directory // for more information. // -// $hash=434a753c90262b051077f7a79f3106ac52ffbf75$ +// $hash=8a2a8a4853c3869876ffad3e6c175945ac1c5021$ // #include "libcef_dll/cpptoc/browser_host_cpptoc.h" @@ -191,6 +191,24 @@ browser_host_try_close_browser(struct _cef_browser_host_t* self) { return _retval; } +int CEF_CALLBACK +browser_host_is_ready_to_be_closed(struct _cef_browser_host_t* self) { + shutdown_checker::AssertNotShutdown(); + + // AUTO-GENERATED CONTENT - DELETE THIS COMMENT BEFORE MODIFYING + + DCHECK(self); + if (!self) { + return 0; + } + + // Execute + bool _retval = CefBrowserHostCppToC::Get(self)->IsReadyToBeClosed(); + + // Return type: bool + return _retval; +} + void CEF_CALLBACK browser_host_set_focus(struct _cef_browser_host_t* self, int focus) { shutdown_checker::AssertNotShutdown(); @@ -1514,6 +1532,7 @@ CefBrowserHostCppToC::CefBrowserHostCppToC() { GetStruct()->get_browser = browser_host_get_browser; GetStruct()->close_browser = browser_host_close_browser; GetStruct()->try_close_browser = browser_host_try_close_browser; + GetStruct()->is_ready_to_be_closed = browser_host_is_ready_to_be_closed; GetStruct()->set_focus = browser_host_set_focus; GetStruct()->get_window_handle = browser_host_get_window_handle; GetStruct()->get_opener_window_handle = browser_host_get_opener_window_handle; diff --git a/libcef_dll/ctocpp/browser_host_ctocpp.cc b/libcef_dll/ctocpp/browser_host_ctocpp.cc index 82f3af621..33159c2a6 100644 --- a/libcef_dll/ctocpp/browser_host_ctocpp.cc +++ b/libcef_dll/ctocpp/browser_host_ctocpp.cc @@ -9,7 +9,7 @@ // implementations. See the translator.README.txt file in the tools directory // for more information. // -// $hash=dac19ba091b3acf3e1587b176e28bc9f9c8c8dd0$ +// $hash=2319d794dd3a38c448908114d1b4ea37b34f89dd$ // #include "libcef_dll/ctocpp/browser_host_ctocpp.h" @@ -131,6 +131,23 @@ NO_SANITIZE("cfi-icall") bool CefBrowserHostCToCpp::TryCloseBrowser() { return _retval ? true : false; } +NO_SANITIZE("cfi-icall") bool CefBrowserHostCToCpp::IsReadyToBeClosed() { + shutdown_checker::AssertNotShutdown(); + + cef_browser_host_t* _struct = GetStruct(); + if (CEF_MEMBER_MISSING(_struct, is_ready_to_be_closed)) { + return false; + } + + // AUTO-GENERATED CONTENT - DELETE THIS COMMENT BEFORE MODIFYING + + // Execute + int _retval = _struct->is_ready_to_be_closed(_struct); + + // Return type: bool + return _retval ? true : false; +} + NO_SANITIZE("cfi-icall") void CefBrowserHostCToCpp::SetFocus(bool focus) { shutdown_checker::AssertNotShutdown(); @@ -1111,8 +1128,8 @@ void CefBrowserHostCToCpp::DragSourceSystemDragEnded() { } NO_SANITIZE("cfi-icall") -CefRefPtr< - CefNavigationEntry> CefBrowserHostCToCpp::GetVisibleNavigationEntry() { +CefRefPtr +CefBrowserHostCToCpp::GetVisibleNavigationEntry() { shutdown_checker::AssertNotShutdown(); cef_browser_host_t* _struct = GetStruct(); diff --git a/libcef_dll/ctocpp/browser_host_ctocpp.h b/libcef_dll/ctocpp/browser_host_ctocpp.h index f11330682..51068d572 100644 --- a/libcef_dll/ctocpp/browser_host_ctocpp.h +++ b/libcef_dll/ctocpp/browser_host_ctocpp.h @@ -9,7 +9,7 @@ // implementations. See the translator.README.txt file in the tools directory // for more information. // -// $hash=9f40e4ce3e46a895b5bf644bebdc2d802c9b598b$ +// $hash=73d8659f17a4ae3319b5bf20807d5c69a1759c04$ // #ifndef CEF_LIBCEF_DLL_CTOCPP_BROWSER_HOST_CTOCPP_H_ @@ -41,6 +41,7 @@ class CefBrowserHostCToCpp : public CefCToCppRefCounted GetBrowser() override; void CloseBrowser(bool force_close) override; bool TryCloseBrowser() override; + bool IsReadyToBeClosed() override; void SetFocus(bool focus) override; CefWindowHandle GetWindowHandle() override; CefWindowHandle GetOpenerWindowHandle() override; diff --git a/tests/ceftests/life_span_unittest.cc b/tests/ceftests/life_span_unittest.cc index b7cbb5d1a..b41cf8d80 100644 --- a/tests/ceftests/life_span_unittest.cc +++ b/tests/ceftests/life_span_unittest.cc @@ -6,6 +6,7 @@ #include "include/test/cef_test_helpers.h" #include "include/wrapper/cef_closure_task.h" #include "tests/ceftests/routing_test_handler.h" +#include "tests/ceftests/test_util.h" #include "tests/gtest/include/gtest/gtest.h" namespace { @@ -58,6 +59,8 @@ class LifeSpanTestHandler : public RoutingTestHandler { } bool DoClose(CefRefPtr browser) override { + EXPECT_TRUE(browser->GetHost()->IsReadyToBeClosed()); + if (executing_delay_close_) { return false; } @@ -75,6 +78,8 @@ class LifeSpanTestHandler : public RoutingTestHandler { } void OnBeforeClose(CefRefPtr browser) override { + EXPECT_TRUE(browser->GetHost()->IsReadyToBeClosed()); + if (!executing_delay_close_) { got_before_close_.yes(); EXPECT_TRUE(browser->IsSame(GetBrowser())); @@ -87,6 +92,8 @@ class LifeSpanTestHandler : public RoutingTestHandler { const CefString& message_text, bool is_reload, CefRefPtr callback) override { + EXPECT_FALSE(browser->GetHost()->IsReadyToBeClosed()); + if (executing_delay_close_) { callback->Continue(true, CefString()); return true; @@ -124,6 +131,8 @@ class LifeSpanTestHandler : public RoutingTestHandler { CefExecuteJavaScriptWithUserGestureForTests(frame, CefString()); } + EXPECT_FALSE(browser->GetHost()->IsReadyToBeClosed()); + // Attempt to close the browser. CloseBrowser(browser, settings_.force_close); } @@ -180,12 +189,15 @@ class LifeSpanTestHandler : public RoutingTestHandler { TEST(LifeSpanTest, DoCloseAllow) { LifeSpanTestHandler::Settings settings; - settings.allow_do_close = true; CefRefPtr handler = new LifeSpanTestHandler(settings); handler->ExecuteTest(); EXPECT_TRUE(handler->got_after_created_); - EXPECT_TRUE(handler->got_do_close_); + if (handler->use_alloy_style_browser()) { + EXPECT_TRUE(handler->got_do_close_); + } else { + EXPECT_FALSE(handler->got_do_close_); + } EXPECT_TRUE(handler->got_before_close_); EXPECT_FALSE(handler->got_before_unload_dialog_); EXPECT_TRUE(handler->got_unload_message_); @@ -197,13 +209,16 @@ TEST(LifeSpanTest, DoCloseAllow) { TEST(LifeSpanTest, DoCloseAllowForce) { LifeSpanTestHandler::Settings settings; - settings.allow_do_close = true; settings.force_close = true; CefRefPtr handler = new LifeSpanTestHandler(settings); handler->ExecuteTest(); EXPECT_TRUE(handler->got_after_created_); - EXPECT_TRUE(handler->got_do_close_); + if (handler->use_alloy_style_browser()) { + EXPECT_TRUE(handler->got_do_close_); + } else { + EXPECT_FALSE(handler->got_do_close_); + } EXPECT_TRUE(handler->got_before_close_); EXPECT_FALSE(handler->got_before_unload_dialog_); EXPECT_TRUE(handler->got_unload_message_); @@ -214,6 +229,11 @@ TEST(LifeSpanTest, DoCloseAllowForce) { } TEST(LifeSpanTest, DoCloseDisallow) { + // Test not supported with Chrome style browser. + if (!UseAlloyStyleBrowserGlobal()) { + return; + } + LifeSpanTestHandler::Settings settings; settings.allow_do_close = false; CefRefPtr handler = new LifeSpanTestHandler(settings); @@ -231,6 +251,11 @@ TEST(LifeSpanTest, DoCloseDisallow) { } TEST(LifeSpanTest, DoCloseDisallowForce) { + // Test not supported with Chrome style browser. + if (!UseAlloyStyleBrowserGlobal()) { + return; + } + LifeSpanTestHandler::Settings settings; settings.allow_do_close = false; settings.force_close = true; @@ -249,6 +274,11 @@ TEST(LifeSpanTest, DoCloseDisallowForce) { } TEST(LifeSpanTest, DoCloseDisallowWithOnUnloadAllow) { + // Test not supported with Chrome style browser. + if (!UseAlloyStyleBrowserGlobal()) { + return; + } + LifeSpanTestHandler::Settings settings; settings.allow_do_close = false; settings.add_onunload_handler = true; @@ -269,14 +299,17 @@ TEST(LifeSpanTest, DoCloseDisallowWithOnUnloadAllow) { TEST(LifeSpanTest, DoCloseAllowWithOnUnloadForce) { LifeSpanTestHandler::Settings settings; - settings.allow_do_close = true; settings.add_onunload_handler = true; settings.force_close = true; CefRefPtr handler = new LifeSpanTestHandler(settings); handler->ExecuteTest(); EXPECT_TRUE(handler->got_after_created_); - EXPECT_TRUE(handler->got_do_close_); + if (handler->use_alloy_style_browser()) { + EXPECT_TRUE(handler->got_do_close_); + } else { + EXPECT_FALSE(handler->got_do_close_); + } EXPECT_TRUE(handler->got_before_close_); EXPECT_TRUE(handler->got_before_unload_dialog_); EXPECT_TRUE(handler->got_unload_message_); @@ -287,6 +320,11 @@ TEST(LifeSpanTest, DoCloseAllowWithOnUnloadForce) { } TEST(LifeSpanTest, DoCloseDisallowWithOnUnloadForce) { + // Test not supported with Chrome style browser. + if (!UseAlloyStyleBrowserGlobal()) { + return; + } + LifeSpanTestHandler::Settings settings; settings.allow_do_close = false; settings.add_onunload_handler = true; @@ -313,7 +351,11 @@ TEST(LifeSpanTest, OnUnloadAllow) { handler->ExecuteTest(); EXPECT_TRUE(handler->got_after_created_); - EXPECT_TRUE(handler->got_do_close_); + if (handler->use_alloy_style_browser()) { + EXPECT_TRUE(handler->got_do_close_); + } else { + EXPECT_FALSE(handler->got_do_close_); + } EXPECT_TRUE(handler->got_before_close_); EXPECT_TRUE(handler->got_before_unload_dialog_); EXPECT_TRUE(handler->got_unload_message_); diff --git a/tests/ceftests/test_handler.cc b/tests/ceftests/test_handler.cc index 8439d90ce..bb239cb3d 100644 --- a/tests/ceftests/test_handler.cc +++ b/tests/ceftests/test_handler.cc @@ -322,6 +322,8 @@ void TestHandler::OnAfterCreated(CefRefPtr browser) { void TestHandler::OnBeforeClose(CefRefPtr browser) { EXPECT_UI_THREAD(); + EXPECT_TRUE(browser->GetHost()->IsReadyToBeClosed()); + // 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);