Skip to content

Commit

Permalink
Use CLIENT_OWNS_WIDGET for Views-related unittests.
Browse files Browse the repository at this point in the history
Change-Id: I9ade72dc5b5919b5bc962caad4662676c2b5f9b6
Bug: 338254375
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5689110
Reviewed-by: Keren Zhu <[email protected]>
Commit-Queue: Allen Bauer <[email protected]>
Code-Coverage: [email protected] <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1326350}
  • Loading branch information
Allen Bauer authored and Chromium LUCI CQ committed Jul 11, 2024
1 parent 67bcf36 commit b83a31f
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 23 deletions.
6 changes: 3 additions & 3 deletions ui/views/actions/action_view_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ TEST_F(ActionViewControllerTest, TestActionViewDestroyed) {

TEST_F(ActionViewControllerTest, TriggerAction) {
std::unique_ptr<Widget> test_widget =
CreateTestWidget(Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET);
CreateTestWidget(Widget::InitParams::CLIENT_OWNS_WIDGET);
View* parent_view = test_widget->SetContentsView(std::make_unique<View>());
MdTextButton* action_view =
parent_view->AddChildView(std::make_unique<MdTextButton>());
Expand Down Expand Up @@ -197,7 +197,7 @@ TEST_F(ActionViewControllerTest, TestActionInvocationContext) {
base::BindRepeating(invoke_action_callback));

std::unique_ptr<Widget> test_widget =
CreateTestWidget(Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET);
CreateTestWidget(Widget::InitParams::CLIENT_OWNS_WIDGET);
View* parent_view = test_widget->SetContentsView(std::make_unique<View>());
TestActionButton* test_button =
parent_view->AddChildView(std::make_unique<TestActionButton>());
Expand All @@ -214,7 +214,7 @@ TEST_F(ActionViewControllerTest, TestActionInvocationContext) {

TEST_F(ActionViewControllerTest, TestOnViewChanged) {
std::unique_ptr<Widget> test_widget =
CreateTestWidget(Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET);
CreateTestWidget(Widget::InitParams::CLIENT_OWNS_WIDGET);
View* parent_view = test_widget->SetContentsView(std::make_unique<View>());
TestActionButton* test_view =
parent_view->AddChildView(std::make_unique<TestActionButton>());
Expand Down
4 changes: 2 additions & 2 deletions ui/views/animation/ink_drop_host_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ class InkDropInWidgetTest : public ViewsTestBase {
protected:
void SetUp() override {
ViewsTestBase::SetUp();
widget_ = CreateTestWidget(Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET);
widget_ = CreateTestWidget(Widget::InitParams::CLIENT_OWNS_WIDGET);
view_ =
widget_->SetContentsView(std::make_unique<BasicTestViewWithInkDrop>());
}
Expand Down Expand Up @@ -436,7 +436,7 @@ class InkDropHostAttentionTest : public ViewsTestBase {
InkDrop::Install(host_view_.get(),
std::make_unique<InkDropHost>(host_view_.get()));
ink_drop_host_ = InkDrop::Get(host_view_.get());
widget_ = CreateTestWidget(Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET);
widget_ = CreateTestWidget(Widget::InitParams::CLIENT_OWNS_WIDGET);
ink_drop_host_test_api_ =
std::make_unique<InkDropHostTestApi>(ink_drop_host_);

Expand Down
2 changes: 1 addition & 1 deletion ui/views/animation/ink_drop_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class InkDropImplTest : public ViewsTestBase {

void SetUp() override {
ViewsTestBase::SetUp();
widget_ = CreateTestWidget(Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET);
widget_ = CreateTestWidget(Widget::InitParams::CLIENT_OWNS_WIDGET);
widget_->SetContentsView(
std::make_unique<TestInkDropHost>(auto_highlight_mode_));
InkDrop::Get(ink_drop_host())->SetMode(views::InkDropHost::InkDropMode::ON);
Expand Down
5 changes: 2 additions & 3 deletions ui/views/animation/slide_out_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,8 @@ class SlideOutControllerTest : public ViewsTestBase {

widget_ = std::make_unique<Widget>();

Widget::InitParams params =
CreateParams(Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET,
Widget::InitParams::TYPE_POPUP);
Widget::InitParams params = CreateParams(
Widget::InitParams::CLIENT_OWNS_WIDGET, Widget::InitParams::TYPE_POPUP);
params.bounds = gfx::Rect(50, 50, 650, 650);
widget_->Init(std::move(params));
View* root = widget_->GetRootView();
Expand Down
2 changes: 1 addition & 1 deletion ui/views/animation/widget_fade_animator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class WidgetFadeAnimatorTest : public test::WidgetTest {
public:
void SetUp() override {
test::WidgetTest::SetUp();
widget_ = CreateTestWidget(Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET,
widget_ = CreateTestWidget(Widget::InitParams::CLIENT_OWNS_WIDGET,
Widget::InitParams::Type::TYPE_WINDOW);
delegate_ = std::make_unique<TestWidgetFadeAnimator>(widget_.get());
delegate_->set_fade_in_duration(kFadeDuration);
Expand Down
44 changes: 31 additions & 13 deletions ui/views/cocoa/bridged_native_widget_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "base/functional/bind.h"
#import "base/mac/mac_util.h"
#include "base/memory/raw_ptr.h"
#include "base/scoped_observation.h"
#include "base/strings/stringprintf.h"
#include "base/strings/sys_string_conversions.h"
#include "base/strings/utf_string_conversions.h"
Expand Down Expand Up @@ -430,13 +431,16 @@ void ReorderNativeViews() override {

// Helper test base to construct a NativeWidgetNSWindowBridge with a valid
// parent.
class BridgedNativeWidgetTestBase : public ui::CocoaTest {
class BridgedNativeWidgetTestBase : public ui::CocoaTest,
public WidgetObserver {
public:
struct SkipInitialization {};

BridgedNativeWidgetTestBase()
: widget_(new Widget),
native_widget_mac_(new MockNativeWidgetMac(widget_.get())) {}
native_widget_mac_(new MockNativeWidgetMac(widget_.get())) {
observation_.Observe(widget_.get());
}

explicit BridgedNativeWidgetTestBase(SkipInitialization tag)
: native_widget_mac_(nullptr) {}
Expand All @@ -446,10 +450,11 @@ explicit BridgedNativeWidgetTestBase(SkipInitialization tag)
delete;

remote_cocoa::NativeWidgetNSWindowBridge* bridge() {
return native_widget_mac_->GetInProcessNSWindowBridge();
return native_widget_mac_ ? native_widget_mac_->GetInProcessNSWindowBridge()
: nullptr;
}
NativeWidgetMacNSWindowHost* GetNSWindowHost() {
return native_widget_mac_->GetNSWindowHost();
return native_widget_mac_ ? native_widget_mac_->GetNSWindowHost() : nullptr;
}

// Generate an autoreleased KeyDown NSEvent* in |widget_| for pressing the
Expand All @@ -471,11 +476,9 @@ explicit BridgedNativeWidgetTestBase(SkipInitialization tag)
void SetUp() override {
ui::CocoaTest::SetUp();

Widget::InitParams init_params(
views::Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET);
Widget::InitParams init_params(ownership_);
init_params.native_widget = native_widget_mac_.get();
init_params.type = type_;
init_params.ownership = ownership_;
init_params.opacity = opacity_;
init_params.bounds = bounds_;
init_params.shadow_type = shadow_type_;
Expand All @@ -489,7 +492,10 @@ void TearDown() override {
// be sure to destroy the widget (which will destroy its NSWindow)
// beforehand.
native_widget_mac_ = nullptr;
widget_.reset();
if (widget_) {
observation_.Reset();
widget_->CloseNow();
}
ui::CocoaTest::TearDown();
}

Expand All @@ -504,17 +510,26 @@ bool BridgeWindowHasShadow() {
bridge_window()) hasShadowForTesting];
}

// Overridden from WidgetObserver:
void OnWidgetDestroyed(Widget* widget) override {
native_widget_mac_ = nullptr;
if (observation_.IsObservingSource(widget)) {
observation_.Reset();
}
}

protected:
std::unique_ptr<Widget> widget_;
base::ScopedObservation<Widget, WidgetObserver> observation_{this};
raw_ptr<MockNativeWidgetMac> native_widget_mac_; // Owned by `widget_`.

// Use a frameless window, otherwise Widget will try to center the window
// before the tests covering the Init() flow are ready to do that.
Widget::InitParams::Type type_ = Widget::InitParams::TYPE_WINDOW_FRAMELESS;
// To control the lifetime without an actual window that must be closed,
// tests in this file need to use WIDGET_OWNS_NATIVE_WIDGET.
// tests in this file use CLIENT_OWNS_WIDGET.
Widget::InitParams::Ownership ownership_ =
Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
Widget::InitParams::CLIENT_OWNS_WIDGET;
// Opacity defaults to "infer" which is usually updated by ViewsDelegate.
Widget::InitParams::WindowOpacity opacity_ =
Widget::InitParams::WindowOpacity::kOpaque;
Expand Down Expand Up @@ -977,16 +992,19 @@ bool AcceleratorPressed(const ui::Accelerator& accelerator) override {
// Prepares a new |window_| and |widget_| for a call to PerformInit().
void CreateNewWidgetToInit() {
native_widget_mac_ = nullptr;
if (widget_) {
observation_.Reset();
widget_->CloseNow();
}
widget_ = std::make_unique<Widget>();
observation_.Observe(widget_.get());
native_widget_mac_ = new MockNativeWidgetMac(widget_.get());
}

void PerformInit() {
Widget::InitParams init_params(
views::Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET);
Widget::InitParams init_params(ownership_);
init_params.native_widget = native_widget_mac_.get();
init_params.type = type_;
init_params.ownership = ownership_;
init_params.opacity = opacity_;
init_params.bounds = bounds_;
init_params.shadow_type = shadow_type_;
Expand Down

0 comments on commit b83a31f

Please sign in to comment.