From 7268dc8cd3497f23db6a2cdaf3017e92dc25a021 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Thu, 17 Oct 2024 11:11:27 -0400 Subject: [PATCH] cefclient: views: Add ability to pop out the overlay Browser (see #3790) When running with the overlay Browser enabled (`--show-overlay-browser`), pressing Alt+O will move the overlay Browser to a new top-level Window. Pressing Alt+O again or closing the new Window via the close button will return the Browser to the overlay. Closing the Browser via `window.close()` (in the new Window or overlay) will dismiss the overlay completely as required to maintain consistent internal state. Detection of this state is supported by the new CefBrowserHost::IsReadyToBeClosed method. Draggable regions in the main Browser are updated to account for the presence or absence of the overlay Browser. Support for draggable regions in the overlay Browser in not implemented in cefclient. Behavior with multiple overlays, `window.close()` and draggable regions can be tested by adding `--hide-frame --hide-controls`. --- libcef/browser/browser_platform_delegate.cc | 4 + tests/cefclient/browser/resource.h | 1 + .../browser/views_overlay_browser.cc | 215 +++++++++++++++++- .../cefclient/browser/views_overlay_browser.h | 23 ++ tests/cefclient/browser/views_window.cc | 8 + tests/cefclient/browser/views_window.h | 6 +- 6 files changed, 252 insertions(+), 5 deletions(-) diff --git a/libcef/browser/browser_platform_delegate.cc b/libcef/browser/browser_platform_delegate.cc index 690e4cb8b..6d6ae2964 100644 --- a/libcef/browser/browser_platform_delegate.cc +++ b/libcef/browser/browser_platform_delegate.cc @@ -63,6 +63,10 @@ class PopupWindowDelegate : public CefWindowDelegate { return true; } + cef_runtime_style_t GetWindowRuntimeStyle() override { + return browser_view_->GetRuntimeStyle(); + } + private: CefRefPtr browser_view_; diff --git a/tests/cefclient/browser/resource.h b/tests/cefclient/browser/resource.h index 81742d923..c1be80bde 100644 --- a/tests/cefclient/browser/resource.h +++ b/tests/cefclient/browser/resource.h @@ -21,6 +21,7 @@ #define IDC_NAV_STOP 203 #define ID_QUIT 32500 #define ID_FIND 32501 +#define ID_POPOUT_OVERLAY 32502 #define ID_TESTS_FIRST 32700 #define ID_TESTS_GETSOURCE 32700 #define ID_TESTS_GETTEXT 32701 diff --git a/tests/cefclient/browser/views_overlay_browser.cc b/tests/cefclient/browser/views_overlay_browser.cc index 6b9f20fdf..5a8b4d574 100644 --- a/tests/cefclient/browser/views_overlay_browser.cc +++ b/tests/cefclient/browser/views_overlay_browser.cc @@ -5,10 +5,111 @@ #include "tests/cefclient/browser/views_overlay_browser.h" #include "include/views/cef_window.h" +#include "tests/cefclient/browser/resource.h" #include "tests/cefclient/browser/views_window.h" namespace client { +namespace { + +void AddPopOutAccelerator(CefRefPtr window) { + // Add an accelerator to toggle the BrowserView popout. OnAccelerator will be + // called when the accelerator is triggered. + window->SetAccelerator(ID_POPOUT_OVERLAY, 'O', /*shift_pressed=*/false, + /*ctrl_pressed=*/false, /*alt_pressed=*/true, + /*high_priority=*/true); +} + +// Popout window delegate implementation. +class PopoutWindowDelegate : public CefWindowDelegate { + public: + PopoutWindowDelegate(base::WeakPtr overlay, + CefRefPtr browser_view) + : overlay_(overlay), browser_view_(browser_view) {} + + PopoutWindowDelegate(const PopoutWindowDelegate&) = delete; + PopoutWindowDelegate& operator=(const PopoutWindowDelegate&) = delete; + + static PopoutWindowDelegate* GetForWindow(CefRefPtr window) { + return static_cast(window->GetDelegate().get()); + } + + void OnWindowCreated(CefRefPtr window) override { + window->AddChildView(browser_view_); + window->Show(); + + // Add the popout accelerator to the popout Window. + AddPopOutAccelerator(window); + + browser_view_->RequestFocus(); + } + + bool CanClose(CefRefPtr window) override { + CefRefPtr browser = + browser_view_ ? browser_view_->GetBrowser() : nullptr; + + if (overlay_ && browser && !browser->GetHost()->IsReadyToBeClosed()) { + // Proceed with the window close, but don't close the browser. The browser + // will be returned to the overlay in OnWindowClosing(). + return_to_overlay_ = true; + return true; + } + + if (browser) { + // We must close the browser, either because the popout Window is the + // final owner of the BrowserView, or because the browser is ready to be + // closed internally (e.g. `window.close()` was called). + return browser->GetHost()->TryCloseBrowser(); + } + + return true; + } + + void OnWindowClosing(CefRefPtr window) override { + if (overlay_ && return_to_overlay_) { + // Give the browser back to the overlay. + overlay_->ToggleBrowserView(); + } + } + + void OnWindowDestroyed(CefRefPtr window) override { + if (overlay_) { + overlay_->PopOutWindowDestroyed(); + overlay_ = nullptr; + } + browser_view_ = nullptr; + } + + cef_runtime_style_t GetWindowRuntimeStyle() override { + return browser_view_->GetRuntimeStyle(); + } + + bool OnAccelerator(CefRefPtr window, int command_id) override { + if (overlay_) { + return overlay_->OnAccelerator(window, command_id); + } + return false; + } + + [[nodiscard]] CefRefPtr DetachBrowserView() { + overlay_ = nullptr; + auto browser_view = browser_view_; + browser_view_ = nullptr; + return browser_view; + } + + void OverlayDestroyed() { overlay_ = nullptr; } + + private: + base::WeakPtr overlay_; + CefRefPtr browser_view_; + bool return_to_overlay_ = false; + + IMPLEMENT_REFCOUNTING(PopoutWindowDelegate); +}; + +} // namespace + ViewsOverlayBrowser::ViewsOverlayBrowser(ViewsWindow* owner_window) : owner_window_(owner_window) {} @@ -22,10 +123,14 @@ void ViewsOverlayBrowser::Initialize( window_ = window; CHECK(window_); + // Add the accelerator to the main window. + AddPopOutAccelerator(window_); + browser_view_ = CefBrowserView::CreateBrowserView( client, url, settings, nullptr, request_context, this); CHECK(browser_view_); + // Add the BrowserView to an overlay in the main window. controller_ = window_->AddOverlayView(browser_view_, CEF_DOCKING_MODE_CUSTOM, /*can_activate=*/true); CHECK(controller_); @@ -33,18 +138,92 @@ void ViewsOverlayBrowser::Initialize( void ViewsOverlayBrowser::Destroy() { window_ = nullptr; + + if (popout_window_) { + // The BrowserView is popped out, and the main Window is closed first. + // Let the popout Window handle BrowserView destruction. + PopoutWindowDelegate::GetForWindow(popout_window_)->OverlayDestroyed(); + popout_window_->Close(); + popout_window_ = nullptr; + } + + if (controller_) { + if (controller_->IsValid()) { + controller_->Destroy(); + } + controller_ = nullptr; + owner_window_->UpdateDraggableRegions(); + } + + if (browser_view_) { + // We hold the last reference to the BrowserView, and releasing it will + // trigger overlay Browser destruction. OnBeforeClose for that Browser may + // be called synchronously or asynchronously depending on whether + // beforeunload needs to be dispatched. + DCHECK(browser_view_->HasOneRef()); + browser_view_ = nullptr; + } +} + +bool ViewsOverlayBrowser::IsValid() const { + // Intentionally not checking |popout_window_->IsValid()| here because the + // pop-in behavior will be triggered by |popout_window_| closing. + return (controller_ && controller_->IsValid()) || popout_window_; +} + +void ViewsOverlayBrowser::ToggleBrowserView() { + if (browser_view_) { + PopOutBrowserView(); + } else { + PopInBrowserView(); + } + + owner_window_->UpdateDraggableRegions(); +} + +void ViewsOverlayBrowser::PopOutBrowserView() { + CHECK(browser_view_); + + DCHECK(controller_ && controller_->IsValid()); controller_->Destroy(); controller_ = nullptr; - // We hold the last reference to the BrowserView, and releasing it will - // trigger overlay Browser destruction. OnBeforeClose for that Browser may be - // called synchronously or asynchronously depending on whether beforeunload - // needs to be dispatched. + // We hold the only reference to the BrowserView. DCHECK(browser_view_->HasOneRef()); + + // Create a new popout Window and pass ownership of the BrowserView. + CHECK(!popout_window_); + popout_window_ = CefWindow::CreateTopLevelWindow( + new PopoutWindowDelegate(weak_ptr_factory_.GetWeakPtr(), browser_view_)); browser_view_ = nullptr; } +void ViewsOverlayBrowser::PopInBrowserView() { + CHECK(!browser_view_); + + // Resume ownership of the BrowserView and close the popout Window. + CHECK(popout_window_); + browser_view_ = + PopoutWindowDelegate::GetForWindow(popout_window_)->DetachBrowserView(); + popout_window_->RemoveChildView(browser_view_); + popout_window_->Close(); + popout_window_ = nullptr; + + // We hold the only reference to the BrowserView. + DCHECK(browser_view_->HasOneRef()); + + // Add the BrowserView to an overlay in the main window. + controller_ = window_->AddOverlayView(browser_view_, CEF_DOCKING_MODE_CUSTOM, + /*can_activate=*/true); + CHECK(controller_); + + // Make sure the overlay is positioned correctly. + UpdateBounds(last_insets_); +} + void ViewsOverlayBrowser::UpdateBounds(CefInsets insets) { + last_insets_ = insets; + if (!controller_) { return; } @@ -78,10 +257,38 @@ void ViewsOverlayBrowser::UpdateDraggableRegions( } } +bool ViewsOverlayBrowser::OnAccelerator(CefRefPtr window, + int command_id) { + if (IsValid() && command_id == ID_POPOUT_OVERLAY) { + ToggleBrowserView(); + return true; + } + return false; +} + +void ViewsOverlayBrowser::PopOutWindowDestroyed() { + popout_window_ = nullptr; +} + CefSize ViewsOverlayBrowser::GetMinimumSize(CefRefPtr view) { return CefSize(200, 200); } +void ViewsOverlayBrowser::OnBrowserDestroyed( + CefRefPtr browser_view, + CefRefPtr browser) { + // Might be popped out currently. + if (!controller_) { + return; + } + + // Destroy the overlay controller if the browser is destroyed first (e.g. via + // `window.close()`). + controller_->Destroy(); + controller_ = nullptr; + owner_window_->UpdateDraggableRegions(); +} + CefRefPtr ViewsOverlayBrowser::GetDelegateForPopupBrowserView( CefRefPtr browser_view, diff --git a/tests/cefclient/browser/views_overlay_browser.h b/tests/cefclient/browser/views_overlay_browser.h index f55d8e48f..1dea50059 100644 --- a/tests/cefclient/browser/views_overlay_browser.h +++ b/tests/cefclient/browser/views_overlay_browser.h @@ -6,6 +6,7 @@ #define CEF_TESTS_CEFCLIENT_BROWSER_VIEWS_OVERLAY_BROWSER_H_ #pragma once +#include "include/base/cef_weak_ptr.h" #include "include/views/cef_browser_view.h" #include "include/views/cef_browser_view_delegate.h" #include "include/views/cef_overlay_controller.h" @@ -28,17 +29,28 @@ class ViewsOverlayBrowser : public CefBrowserViewDelegate { CefRefPtr request_context); void Destroy(); + bool IsValid() const; + + // Move the overlay BrowserView to/from a popout Window. + void ToggleBrowserView(); + // Update browser bounds. void UpdateBounds(CefInsets insets); // Exclude all regions obscured by overlays. void UpdateDraggableRegions(std::vector& window_regions); + bool OnAccelerator(CefRefPtr window, int command_id); + + void PopOutWindowDestroyed(); + private: // CefViewDelegate methods: CefSize GetMinimumSize(CefRefPtr view) override; // CefBrowserViewDelegate methods: + void OnBrowserDestroyed(CefRefPtr browser_view, + CefRefPtr browser) override; CefRefPtr GetDelegateForPopupBrowserView( CefRefPtr browser_view, const CefBrowserSettings& settings, @@ -49,10 +61,21 @@ class ViewsOverlayBrowser : public CefBrowserViewDelegate { bool is_devtools) override; cef_runtime_style_t GetBrowserRuntimeStyle() override; + // Move the BrowserView to a new top-level Window. + void PopOutBrowserView(); + + // Return the BrowserView to the overlay. + void PopInBrowserView(); + ViewsWindow* const owner_window_; CefRefPtr window_; CefRefPtr browser_view_; CefRefPtr controller_; + CefInsets last_insets_; + + CefRefPtr popout_window_; + + base::WeakPtrFactory weak_ptr_factory_{this}; IMPLEMENT_REFCOUNTING(ViewsOverlayBrowser); DISALLOW_COPY_AND_ASSIGN(ViewsOverlayBrowser); diff --git a/tests/cefclient/browser/views_window.cc b/tests/cefclient/browser/views_window.cc index 9449cd0ad..fe6aa765a 100644 --- a/tests/cefclient/browser/views_window.cc +++ b/tests/cefclient/browser/views_window.cc @@ -354,6 +354,8 @@ void ViewsWindow::SetDraggableRegions( return; } + last_regions_ = regions; + // Convert the regions from BrowserView to Window coordinates. std::vector window_regions = regions; for (auto& region : window_regions) { @@ -448,6 +450,10 @@ void ViewsWindow::SetTitlebarHeight(const std::optional& height) { NudgeWindow(); } +void ViewsWindow::UpdateDraggableRegions() { + SetDraggableRegions(last_regions_); +} + CefRefPtr ViewsWindow::GetDelegateForPopupBrowserView( CefRefPtr browser_view, const CefBrowserSettings& settings, @@ -914,6 +920,8 @@ bool ViewsWindow::OnAccelerator(CefRefPtr window, int command_id) { if (command_id == ID_QUIT) { delegate_->OnExit(); return true; + } else if (overlay_browser_) { + return overlay_browser_->OnAccelerator(window, command_id); } return false; diff --git a/tests/cefclient/browser/views_window.h b/tests/cefclient/browser/views_window.h index 1b81eb3fb..0ad0b7a20 100644 --- a/tests/cefclient/browser/views_window.h +++ b/tests/cefclient/browser/views_window.h @@ -129,6 +129,8 @@ class ViewsWindow : public CefBrowserViewDelegate, std::optional& dip_bounds); void SetTitlebarHeight(const std::optional& height); + void UpdateDraggableRegions(); + // CefBrowserViewDelegate methods: CefRefPtr GetDelegateForPopupBrowserView( CefRefPtr browser_view, @@ -252,7 +254,7 @@ class ViewsWindow : public CefBrowserViewDelegate, void NudgeWindow(); const WindowType type_; - Delegate* delegate_; // Not owned by this object. + Delegate* const delegate_; // Not owned by this object. const bool use_alloy_style_; bool use_alloy_style_window_; CefRefPtr browser_view_; @@ -301,6 +303,8 @@ class ViewsWindow : public CefBrowserViewDelegate, bool can_go_back_ = false; bool can_go_forward_ = false; + std::vector last_regions_; + IMPLEMENT_REFCOUNTING(ViewsWindow); DISALLOW_COPY_AND_ASSIGN(ViewsWindow); };