bcarrella
/
cef
mirror of https://bitbucket.org/chromiumembedded/cef
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

views: Fix destruction issues with CefOverlayViewHost

This commit is contained in:
Marshall Greenblatt 2024-04-10 17:34:11 -04:00
parent 6fafa6521f
commit e011687449
7 changed files with 305 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
libcef/browser/views/overlay_view_host.cc
View File

@ -31,7 +31,8 @@ class CefOverlayControllerImpl : public CefOverlayController {
}
bool IsSame(CefRefPtr<CefOverlayController> that) override {
return that && that->GetContentsView()->IsSame(view_);
return IsValid() && that && that->IsValid() &&
that->GetContentsView()->IsSame(view_);
}
CefRefPtr<CefView> GetContentsView() override { return view_; }
@ -52,11 +53,17 @@ class CefOverlayControllerImpl : public CefOverlayController {
void Destroy() override {
if (IsValid()) {
host_->Destroy();
view_ = nullptr;
// Results in a call to Destroyed().
host_->Close();
host_ = nullptr;
}
}
void Destroyed() {
DCHECK(view_);
view_ = nullptr;
}
void SetBounds(const CefRect& bounds) override {
if (IsValid() && host_->docking_mode() == CEF_DOCKING_MODE_CUSTOM) {
host_->SetOverlayBounds(
@ -155,7 +162,7 @@ class CefOverlayControllerImpl : public CefOverlayController {
bool IsDrawn() override { return IsVisible(); }
private:
CefOverlayViewHost* const host_;
CefOverlayViewHost* host_;
CefRefPtr<CefView> view_;
IMPLEMENT_REFCOUNTING(CefOverlayControllerImpl);
@ -180,7 +187,8 @@ void CefOverlayViewHost::Init(views::View* host_view,
cef_controller_ = new CefOverlayControllerImpl(this, view);
// Initialize the Widget.
// Initialize the Widget. |widget_| will be deleted by the NativeWidget or
// when WidgetDelegate::DeleteDelegate() deletes |this|.
widget_ = std::make_unique<ThemeCopyingWidget>(window_view_->GetWidget());
views::Widget::InitParams params(views::Widget::InitParams::TYPE_CONTROL);
params.delegate = this;
@ -193,13 +201,23 @@ void CefOverlayViewHost::Init(views::View* host_view,
: views::Widget::InitParams::Activatable::kNo;
widget_->Init(std::move(params));
// |widget_| should now be associated with |this|.
DCHECK_EQ(widget_.get(), GetWidget());
// Make the Widget background transparent. The View might still be opaque.
if (widget_->GetCompositor()) {
widget_->GetCompositor()->SetBackgroundColor(SK_ColorTRANSPARENT);
}
host_view_ = host_view;
view_util::SetHostView(widget_.get(), host_view);
// Cause WidgetDelegate::DeleteDelegate() to delete |this| after executing the
// registered DeleteDelegate callback.
SetOwnedByWidget(true);
RegisterDeleteDelegateCallback(
base::BindOnce(&CefOverlayViewHost::Cleanup, base::Unretained(this)));
if (cef::IsChromeRuntimeEnabled()) {
// Some attributes associated with a Chrome toolbar are located via the
// Widget. See matching logic in BrowserView::AddedToWidget.
@ -239,15 +257,12 @@ void CefOverlayViewHost::Init(views::View* host_view,
widget_->Hide();
}
void CefOverlayViewHost::Destroy() {
void CefOverlayViewHost::Close() {
if (widget_ && !widget_->IsClosed()) {
// Remove the child View immediately. It may be reused by the client.
auto view = view_util::GetFor(view_, /*find_known_parent=*/false);
widget_->GetContentsView()->RemoveChildView(view_);
if (view) {
view_util::ResumeOwnership(view);
}
// Remove all references ASAP, before the Widget is destroyed.
Cleanup();
// Eventually calls DeleteDelegate().
widget_->Close();
}
}
@ -333,3 +348,37 @@ gfx::Rect CefOverlayViewHost::ComputeBounds() const {
return gfx::Rect(x, y, prefsize.width(), prefsize.height());
}
void CefOverlayViewHost::Cleanup() {
// This method may be called multiple times. For example, explicitly after the
// client calls CefOverlayController::Destroy or implicitly when the host
// Widget is being closed or destroyed. In most implicit cases
// CefWindowView::WindowClosing will call this before the host Widget is
// destroyed, allowing the client to optionally reuse the child View. However,
// if CefWindowView::WindowClosing is not called, DeleteDelegate will call
// this after the host Widget and all associated Widgets/Views have been
// destroyed. In the DeleteDelegate case |widget_| will return nullptr.
if (view_ && widget_) {
// Remove the child View immediately. It may be reused by the client.
auto view = view_util::GetFor(view_, /*find_known_parent=*/false);
widget_->GetContentsView()->RemoveChildView(view_);
if (view) {
view_util::ResumeOwnership(view);
}
view_->RemoveObserver(this);
view_ = nullptr;
}
if (cef_controller_) {
CefOverlayControllerImpl* controller_impl =
static_cast<CefOverlayControllerImpl*>(cef_controller_.get());
controller_impl->Destroyed();
cef_controller_ = nullptr;
}
if (window_view_) {
window_view_->RemoveOverlayView(this, host_view_);
window_view_ = nullptr;
host_view_ = nullptr;
}
}

12
libcef/browser/views/overlay_view_host.h
View File

@ -12,6 +12,7 @@
#include "include/views/cef_view.h"
#include "ui/views/view_observer.h"
#include "ui/views/widget/unique_widget_ptr.h"
#include "ui/views/widget/widget_delegate.h"
class CefWindowView;
@ -34,7 +35,7 @@ class CefOverlayViewHost : public views::WidgetDelegate,
// relative to views with layers and views with associated NativeViews.
void Init(views::View* host_view, CefRefPtr<CefView> view, bool can_activate);
void Destroy();
void Close();
void MoveIfNecessary();
@ -55,17 +56,22 @@ class CefOverlayViewHost : public views::WidgetDelegate,
private:
gfx::Rect ComputeBounds() const;
void Cleanup();
// The CefWindowView that created us.
CefWindowView* const window_view_;
CefWindowView* window_view_;
const cef_docking_mode_t docking_mode_;
// The host view that the overlay is positioned relative to.
views::View* host_view_ = nullptr;
// Our view, which is responsible for drawing the UI.
views::View* view_ = nullptr;
// The Widget implementation that is created and maintained by the overlay.
// It contains |view_|.
std::unique_ptr<views::Widget> widget_;
views::UniqueWidgetPtr widget_;
CefRefPtr<CefOverlayController> cef_controller_;

42
libcef/browser/views/window_view.cc
View File

@ -19,6 +19,7 @@
#include "libcef/browser/views/widget.h"
#include "libcef/browser/views/window_impl.h"
#include "base/ranges/algorithm.h"
#include "ui/base/hit_test.h"
#include "ui/display/screen.h"
#include "ui/views/widget/widget.h"
@ -397,8 +398,8 @@ void CefWindowView::CreateWidget(gfx::AcceleratedWidget parent_widget) {
params.type = views::Widget::InitParams::TYPE_WINDOW;
}
// WidgetDelegate::DeleteDelegate() will delete |this| after executing the
// registered callback.
// Cause WidgetDelegate::DeleteDelegate() to delete |this| after executing the
// registered DeleteDelegate callback.
SetOwnedByWidget(true);
RegisterDeleteDelegateCallback(
base::BindOnce(&CefWindowView::DeleteDelegate, base::Unretained(this)));
@ -586,6 +587,9 @@ CefRefPtr<CefWindow> CefWindowView::GetCefWindow() const {
}
void CefWindowView::DeleteDelegate() {
// Any overlays should already be removed.
DCHECK(overlay_hosts_.empty());
// Remove all child Views before deleting the Window so that notifications
// resolve correctly.
RemoveAllChildViews();
@ -632,6 +636,14 @@ ui::ImageModel CefWindowView::GetWindowAppIcon() {
}
void CefWindowView::WindowClosing() {
// Close any overlays now, before the Widget is destroyed.
// Use a copy of the array because the original may be modified while
// iterating.
std::vector<CefOverlayViewHost*> overlay_hosts = overlay_hosts_;
for (auto* overlay_host : overlay_hosts) {
overlay_host->Close();
}
#if BUILDFLAG(IS_LINUX)
#if BUILDFLAG(IS_OZONE_X11)
if (host_widget()) {
@ -647,6 +659,8 @@ void CefWindowView::WindowClosing() {
#endif
window_delegate_->OnWindowClosing();
views::WidgetDelegateView::WindowClosing();
}
views::View* CefWindowView::GetContentsView() {
@ -754,7 +768,11 @@ void CefWindowView::OnWidgetActivationChanged(views::Widget* widget,
void CefWindowView::OnWidgetBoundsChanged(views::Widget* widget,
const gfx::Rect& new_bounds) {
MoveOverlaysIfNecessary();
// Size is set to zero when the host Widget is hidden. We don't need to move
// overlays in that case.
if (!new_bounds.IsEmpty()) {
MoveOverlaysIfNecessary();
}
if (cef_delegate()) {
cef_delegate()->OnWindowBoundsChanged(
@ -816,10 +834,10 @@ CefRefPtr<CefOverlayController> CefWindowView::AddOverlayView(
// Owned by the View hierarchy. Acts as a z-order reference for the overlay.
auto overlay_host_view = AddChildView(std::make_unique<views::View>());
overlay_hosts_.push_back(
std::make_unique<CefOverlayViewHost>(this, docking_mode));
// Owned by the resulting Widget, after calling Init().
auto* overlay_host = new CefOverlayViewHost(this, docking_mode);
overlay_hosts_.push_back(overlay_host);
auto& overlay_host = overlay_hosts_.back();
overlay_host->Init(overlay_host_view, view, can_activate);
return overlay_host->controller();
@ -828,6 +846,18 @@ CefRefPtr<CefOverlayController> CefWindowView::AddOverlayView(
return nullptr;
}
void CefWindowView::RemoveOverlayView(CefOverlayViewHost* host,
views::View* host_view) {
DCHECK_EQ(host_view->parent(), this);
RemoveChildView(host_view);
const auto it = base::ranges::find_if(
overlay_hosts_,
[host](CefOverlayViewHost* current) { return current == host; });
DCHECK(it != overlay_hosts_.end());
overlay_hosts_.erase(it);
}
void CefWindowView::MoveOverlaysIfNecessary() {
if (overlay_hosts_.empty()) {
return;

7
libcef/browser/views/window_view.h
View File

@ -110,6 +110,9 @@ class CefWindowView
cef_docking_mode_t docking_mode,
bool can_activate);
// Called from CefOverlayViewHost::Cleanup().
void RemoveOverlayView(CefOverlayViewHost* host, views::View* host_view);
// Set/get the draggable regions.
void SetDraggableRegions(const std::vector<CefDraggableRegion>& regions);
SkRegion* draggable_region() const { return draggable_region_.get(); }
@ -137,7 +140,7 @@ class CefWindowView
views::Widget* host_widget() const;
private:
// Called when removed from the Widget and before |this| is deleted.
// Called after Widget teardown starts, before |this| is deleted.
void DeleteDelegate();
void MoveOverlaysIfNecessary();
@ -165,7 +168,7 @@ class CefWindowView
std::unique_ptr<WidgetDestructionObserver> host_widget_destruction_observer_;
// Hosts for overlay widgets.
std::vector<std::unique_ptr<CefOverlayViewHost>> overlay_hosts_;
std::vector<CefOverlayViewHost*> overlay_hosts_;
};
#endif // CEF_LIBCEF_BROWSER_VIEWS_WINDOW_VIEW_H_

14
tests/ceftests/views/test_window_delegate.cc
View File

@ -35,7 +35,8 @@ const int TestWindowDelegate::kWSize = 400;
// static
void TestWindowDelegate::RunTest(CefRefPtr<CefWaitableEvent> event,
std::unique_ptr<Config> config) {
std::unique_ptr<Config> config,
TestWindowDelegateFactory factory) {
CefSize window_size{config->window_size, config->window_size};
if (!config->frameless) {
@ -64,8 +65,15 @@ void TestWindowDelegate::RunTest(CefRefPtr<CefWaitableEvent> event,
#endif
}
CefWindow::CreateTopLevelWindow(
new TestWindowDelegate(event, std::move(config), window_size));
TestWindowDelegate* delegate;
if (!factory.is_null()) {
delegate = std::move(factory).Run(event, std::move(config), window_size);
CHECK(delegate);
} else {
delegate = new TestWindowDelegate(event, std::move(config), window_size);
}
CefWindow::CreateTopLevelWindow(delegate);
}
void TestWindowDelegate::OnWindowCreated(CefRefPtr<CefWindow> window) {

15
tests/ceftests/views/test_window_delegate.h
View File

@ -5,6 +5,7 @@
#include <memory>
#include "include/base/cef_callback.h"
#include "include/base/cef_callback_helpers.h"
#include "include/base/cef_weak_ptr.h"
#include "include/cef_waitable_event.h"
#include "include/views/cef_window.h"
@ -41,6 +42,11 @@ class TestWindowDelegate : public CefWindowDelegate {
cef_show_state_t initial_show_state = CEF_SHOW_STATE_NORMAL;
};
using TestWindowDelegateFactory =
base::OnceCallback<TestWindowDelegate*(CefRefPtr<CefWaitableEvent> event,
std::unique_ptr<Config> config,
const CefSize& window_size)>;
// Creates a Window with a new TestWindowDelegate instance and executes
// |window_test| after the Window is created. |event| will be signaled once
// the Window is closed. If |frameless| is true the Window will be created
@ -48,7 +54,8 @@ class TestWindowDelegate : public CefWindowDelegate {
// immediately after |window_test| returns. Otherwise, the caller is
// responsible for closing the Window passed to |window_test|.
static void RunTest(CefRefPtr<CefWaitableEvent> event,
std::unique_ptr<Config> config);
std::unique_ptr<Config> config,
TestWindowDelegateFactory factory = base::NullCallback());
// CefWindowDelegate methods:
void OnWindowCreated(CefRefPtr<CefWindow> window) override;
@ -63,12 +70,16 @@ class TestWindowDelegate : public CefWindowDelegate {
bool OnKeyEvent(CefRefPtr<CefWindow> window,
const CefKeyEvent& event) override;
private:
protected:
TestWindowDelegate(CefRefPtr<CefWaitableEvent> event,
std::unique_ptr<Config> config,
const CefSize& window_size);
~TestWindowDelegate() override;
Config* config() const { return config_.get(); }
CefRefPtr<CefWindow> window() const { return window_; }
private:
void OnCloseWindow();
void OnTimeoutWindow();

170
tests/ceftests/views/window_unittest.cc
View File

@ -620,3 +620,173 @@ WINDOW_TEST_ASYNC(WindowFullscreenFrameless)
WINDOW_TEST_ASYNC(WindowIcon)
WINDOW_TEST_ASYNC(WindowIconFrameless)
WINDOW_TEST_ASYNC(WindowAccelerator)
namespace {
enum class OverlayTestMode {
// Destroy the overlay after the Window is destroyed.
kDestroyAfterWindowDestroyImplicit,
kDestroyAfterWindowDestroyExplicit,
// Destroy the overlay explicitly before the Window is shown.
kDestroyBeforeWindowShow,
kDestroyBeforeWindowShowAndAddAgain,
// Destroy the overlay explicitly after the Window is shown.
kDestroyAfterWindowShow,
kDestroyAfterWindowShowAndAddAgain,
};
class OverlayTestWindowDelegate : public TestWindowDelegate {
public:
static TestWindowDelegate* Factory(OverlayTestMode test_mode,
CefRefPtr<CefWaitableEvent> event,
std::unique_ptr<Config> config,
const CefSize& window_size) {
return new OverlayTestWindowDelegate(test_mode, event, std::move(config),
window_size);
}
private:
OverlayTestWindowDelegate(OverlayTestMode test_mode,
CefRefPtr<CefWaitableEvent> event,
std::unique_ptr<Config> config,
const CefSize& window_size)
: TestWindowDelegate(event, std::move(config), window_size),
test_mode_(test_mode) {
this->config()->on_window_created = base::BindOnce(
&OverlayTestWindowDelegate::RunWindowCreated, base::Unretained(this));
this->config()->on_window_destroyed = base::BindOnce(
&OverlayTestWindowDelegate::RunWindowDestroyed, base::Unretained(this));
}
bool DestroyBeforeShow() const {
return test_mode_ == OverlayTestMode::kDestroyBeforeWindowShow ||
test_mode_ == OverlayTestMode::kDestroyBeforeWindowShowAndAddAgain;
}
bool DestroyAfterShow() const {
return test_mode_ == OverlayTestMode::kDestroyAfterWindowShow ||
test_mode_ == OverlayTestMode::kDestroyAfterWindowShowAndAddAgain;
}
bool AddAgain() const {
return test_mode_ == OverlayTestMode::kDestroyBeforeWindowShowAndAddAgain ||
test_mode_ == OverlayTestMode::kDestroyAfterWindowShowAndAddAgain;
}
void RunWindowCreated(CefRefPtr<CefWindow> window) {
CreateOverlay();
if (DestroyBeforeShow()) {
DestroyOverlay();
}
window->Show();
if (DestroyAfterShow()) {
DestroyOverlay();
}
}
void RunWindowDestroyed(CefRefPtr<CefWindow> window) {
if (test_mode_ == OverlayTestMode::kDestroyAfterWindowDestroyExplicit) {
DestroyOverlay();
}
}
void CreateOverlay() {
// |view_| may be reused.
if (!view_) {
view_ = CefPanel::CreatePanel(nullptr);
}
// View is visible but not drawn.
EXPECT_EQ(nullptr, view_->GetWindow());
EXPECT_TRUE(view_->IsVisible());
EXPECT_FALSE(view_->IsDrawn());
EXPECT_FALSE(controller_);
controller_ = window()->AddOverlayView(view_, CEF_DOCKING_MODE_TOP_LEFT,
/*can_activate=*/false);
// View is visible/drawn (because it belongs to the controller), but the
// controller itself is not.
EXPECT_FALSE(controller_->IsVisible());
EXPECT_FALSE(controller_->IsDrawn());
EXPECT_TRUE(window()->IsSame(view_->GetWindow()));
EXPECT_TRUE(view_->IsVisible());
EXPECT_TRUE(view_->IsDrawn());
controller_->SetVisible(true);
EXPECT_TRUE(controller_->IsValid());
EXPECT_TRUE(controller_->GetContentsView()->IsSame(view_));
EXPECT_TRUE(controller_->GetWindow()->IsSame(window()));
EXPECT_EQ(CEF_DOCKING_MODE_TOP_LEFT, controller_->GetDockingMode());
// Controller is visible/drawn if the host window is drawn.
if (window()->IsDrawn()) {
EXPECT_TRUE(controller_->IsVisible());
EXPECT_TRUE(controller_->IsDrawn());
} else {
EXPECT_FALSE(controller_->IsVisible());
EXPECT_FALSE(controller_->IsDrawn());
}
EXPECT_TRUE(view_->IsVisible());
EXPECT_TRUE(view_->IsDrawn());
}
void DestroyOverlay() {
// Disassociates the controller from the view and host window.
controller_->Destroy();
EXPECT_FALSE(controller_->IsValid());
EXPECT_EQ(nullptr, controller_->GetContentsView());
EXPECT_EQ(nullptr, controller_->GetWindow());
EXPECT_FALSE(controller_->IsVisible());
EXPECT_FALSE(controller_->IsDrawn());
// View is still visible but no longer drawn (because it no longer belongs
// to the controller).
EXPECT_EQ(nullptr, view_->GetWindow());
EXPECT_TRUE(view_->IsVisible());
EXPECT_FALSE(view_->IsDrawn());
controller_ = nullptr;
if (AddAgain()) {
CreateOverlay();
}
}
OverlayTestMode const test_mode_;
CefRefPtr<CefView> view_;
CefRefPtr<CefOverlayController> controller_;
};
void WindowOverlay(OverlayTestMode test_mode,
CefRefPtr<CefWaitableEvent> event) {
auto config = std::make_unique<TestWindowDelegate::Config>();
TestWindowDelegate::RunTest(
event, std::move(config),
base::BindOnce(&OverlayTestWindowDelegate::Factory, test_mode));
}
} // namespace
#define WINDOW_OVERLAY_TEST(name) \
namespace { \
void WindowOverlay##name##Impl(CefRefPtr<CefWaitableEvent> event) { \
WindowOverlay(OverlayTestMode::k##name, event); \
} \
} \
WINDOW_TEST_ASYNC(WindowOverlay##name)
WINDOW_OVERLAY_TEST(DestroyAfterWindowDestroyImplicit)
WINDOW_OVERLAY_TEST(DestroyAfterWindowDestroyExplicit)
WINDOW_OVERLAY_TEST(DestroyBeforeWindowShow)
WINDOW_OVERLAY_TEST(DestroyBeforeWindowShowAndAddAgain)
WINDOW_OVERLAY_TEST(DestroyAfterWindowShow)
WINDOW_OVERLAY_TEST(DestroyAfterWindowShowAndAddAgain)