Commit bc575992 authored by Elly Fong-Jones's avatar Elly Fong-Jones Committed by Commit Bot

views: introduce unique_ptr variants of Widget::CreateWindowWith

These methods express that the caller is passing in ownership of the
delegate, and assert that the delegate believes it is owned by the
widget.

This change also corrects a flaw in ownership handling in an ash
unittest: some of the created delegates were passed around as raw
pointers, and one was sort of leaked (not actually leaked, but
destroyed in an extremely non-obvious way).

Bug: 1075649
Change-Id: I655dfe3e2466032b85247341f43b1b2d75c0b561
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2446015Reviewed-by: default avatarPeter Boström <pbos@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813416}
parent 0309ab24
...@@ -134,12 +134,13 @@ void ExpectAllContainers() { ...@@ -134,12 +134,13 @@ void ExpectAllContainers() {
EXPECT_FALSE(Shell::GetContainer(root_window, kShellWindowId_PhantomWindow)); EXPECT_FALSE(Shell::GetContainer(root_window, kShellWindowId_PhantomWindow));
} }
views::WidgetDelegateView* CreateModalWidgetDelegate() { std::unique_ptr<views::WidgetDelegateView> CreateModalWidgetDelegate() {
auto delegate = std::make_unique<views::WidgetDelegateView>(); auto delegate = std::make_unique<views::WidgetDelegateView>();
delegate->SetCanResize(true); delegate->SetCanResize(true);
delegate->SetModalType(ui::MODAL_TYPE_SYSTEM); delegate->SetModalType(ui::MODAL_TYPE_SYSTEM);
delegate->SetOwnedByWidget(true);
delegate->SetTitle(base::ASCIIToUTF16("Modal Window")); delegate->SetTitle(base::ASCIIToUTF16("Modal Window"));
return delegate.release(); return delegate;
} }
class SimpleMenuDelegate : public ui::SimpleMenuModel::Delegate { class SimpleMenuDelegate : public ui::SimpleMenuModel::Delegate {
...@@ -322,14 +323,6 @@ TEST_F(ShellTest, CreateModalWindow) { ...@@ -322,14 +323,6 @@ TEST_F(ShellTest, CreateModalWindow) {
widget->Close(); widget->Close();
} }
class TestModalDialogDelegate : public views::DialogDelegateView {
public:
TestModalDialogDelegate() = default;
// Overridden from views::WidgetDelegate:
ui::ModalType GetModalType() const override { return ui::MODAL_TYPE_SYSTEM; }
};
TEST_F(ShellTest, CreateLockScreenModalWindow) { TEST_F(ShellTest, CreateLockScreenModalWindow) {
views::Widget::InitParams widget_params( views::Widget::InitParams widget_params(
views::Widget::InitParams::TYPE_WINDOW); views::Widget::InitParams::TYPE_WINDOW);
...@@ -383,9 +376,9 @@ TEST_F(ShellTest, CreateLockScreenModalWindow) { ...@@ -383,9 +376,9 @@ TEST_F(ShellTest, CreateLockScreenModalWindow) {
Shell::GetPrimaryRootWindow(), kShellWindowId_SystemModalContainer); Shell::GetPrimaryRootWindow(), kShellWindowId_SystemModalContainer);
EXPECT_EQ(modal_container, modal_widget->GetNativeWindow()->parent()); EXPECT_EQ(modal_container, modal_widget->GetNativeWindow()->parent());
// Modal dialog without parent, caused crash see crbug.com/226141 // Modal widget without parent, caused crash see crbug.com/226141
views::Widget* modal_dialog = views::DialogDelegate::CreateDialogWidget( views::Widget* modal_dialog = views::DialogDelegate::CreateDialogWidget(
new TestModalDialogDelegate(), GetContext(), nullptr); CreateModalWidgetDelegate(), GetContext(), nullptr);
modal_dialog->Show(); modal_dialog->Show();
EXPECT_FALSE(modal_dialog->GetNativeView()->HasFocus()); EXPECT_FALSE(modal_dialog->GetNativeView()->HasFocus());
......
...@@ -197,6 +197,13 @@ Widget* Widget::CreateWindowWithParent(WidgetDelegate* delegate, ...@@ -197,6 +197,13 @@ Widget* Widget::CreateWindowWithParent(WidgetDelegate* delegate,
return new Widget(std::move(params)); return new Widget(std::move(params));
} }
Widget* Widget::CreateWindowWithParent(std::unique_ptr<WidgetDelegate> delegate,
gfx::NativeView parent,
const gfx::Rect& bounds) {
DCHECK(delegate->owned_by_widget());
return CreateWindowWithParent(delegate.release(), parent, bounds);
}
// static // static
Widget* Widget::CreateWindowWithContext(WidgetDelegate* delegate, Widget* Widget::CreateWindowWithContext(WidgetDelegate* delegate,
gfx::NativeWindow context, gfx::NativeWindow context,
...@@ -208,6 +215,15 @@ Widget* Widget::CreateWindowWithContext(WidgetDelegate* delegate, ...@@ -208,6 +215,15 @@ Widget* Widget::CreateWindowWithContext(WidgetDelegate* delegate,
return new Widget(std::move(params)); return new Widget(std::move(params));
} }
// static
Widget* Widget::CreateWindowWithContext(
std::unique_ptr<WidgetDelegate> delegate,
gfx::NativeWindow context,
const gfx::Rect& bounds) {
DCHECK(delegate->owned_by_widget());
return CreateWindowWithContext(delegate.release(), context, bounds);
}
// static // static
Widget* Widget::GetWidgetForNativeView(gfx::NativeView native_view) { Widget* Widget::GetWidgetForNativeView(gfx::NativeView native_view) {
internal::NativeWidgetPrivate* native_widget = internal::NativeWidgetPrivate* native_widget =
......
...@@ -396,16 +396,26 @@ class VIEWS_EXPORT Widget : public internal::NativeWidgetDelegate, ...@@ -396,16 +396,26 @@ class VIEWS_EXPORT Widget : public internal::NativeWidgetDelegate,
// Creates a decorated window Widget with the specified properties. The // Creates a decorated window Widget with the specified properties. The
// returned Widget is owned by its NativeWidget; see Widget class comment for // returned Widget is owned by its NativeWidget; see Widget class comment for
// details. // details.
// The std::unique_ptr variant requires that delegate->owned_by_widget().
static Widget* CreateWindowWithParent(WidgetDelegate* delegate, static Widget* CreateWindowWithParent(WidgetDelegate* delegate,
gfx::NativeView parent, gfx::NativeView parent,
const gfx::Rect& bounds = gfx::Rect()); const gfx::Rect& bounds = gfx::Rect());
static Widget* CreateWindowWithParent(
std::unique_ptr<WidgetDelegate> delegate,
gfx::NativeView parent,
const gfx::Rect& bounds = gfx::Rect());
// Creates a decorated window Widget in the same desktop context as |context|. // Creates a decorated window Widget in the same desktop context as |context|.
// The returned Widget is owned by its NativeWidget; see Widget class comment // The returned Widget is owned by its NativeWidget; see Widget class comment
// for details. // for details.
// The std::unique_ptr variant requires that delegate->owned_by_widget().
static Widget* CreateWindowWithContext(WidgetDelegate* delegate, static Widget* CreateWindowWithContext(WidgetDelegate* delegate,
gfx::NativeWindow context, gfx::NativeWindow context,
const gfx::Rect& bounds = gfx::Rect()); const gfx::Rect& bounds = gfx::Rect());
static Widget* CreateWindowWithContext(
std::unique_ptr<WidgetDelegate> delegate,
gfx::NativeWindow context,
const gfx::Rect& bounds = gfx::Rect());
// Closes all Widgets that aren't identified as "secondary widgets". Called // Closes all Widgets that aren't identified as "secondary widgets". Called
// during application shutdown when the last non-secondary widget is closed. // during application shutdown when the last non-secondary widget is closed.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment