From 71b440acbb273cd8fcc22a587b4604038cfd3a1e Mon Sep 17 00:00:00 2001 From: tapted Date: Wed, 17 Jun 2015 19:54:09 -0700 Subject: [PATCH] Make ViewsTestHelpers DCHECK that all Widgets are destroyed before unit test tear down Running custom_button_unittest.cc on Mac resulted in some use-after-free errors. The problem: Widgets hold on to a Compositor. If they are not all closed when the ContextFactory is torn down by ViewsTestBase, then bad stuff happens when the Widget is closed afterwards (e.g. by scoped_ptrs on the test harness with WIDGET_OWNS_NATIVE_WIDGET). Since many tests do not try to create Desktop Aura widgets, they will instead create Ash-style Widgets that are owned by the RootWindow. These get automatically destroyed when the RootWindow is torn down by AuraTestHelper. However, on Mac there are only desktop widgets, so any unclosed widgets remain "unowned". A browser_test can rely on Widget::CloseAllSecondaryWidgets(), but not a unit_test. This CL adds a check to ViewsTestHelperMac to anticipate these use-after-frees. The same check is added to ViewsTestHelperAura to help tests detect these errors before going through the CQ. Tests that were not closing Widgets are updated. Most unit tests did already close their Widgets. For the majority of those that were not, it seems accidental. BUG=412234 Review URL: https://codereview.chromium.org/1180623007 Cr-Commit-Position: refs/heads/master@{#334985} --- .../views/desktop_media_picker_views_unittest.cc | 20 ++--- ui/chromeos/ime/candidate_window_view_unittest.cc | 8 +- .../views/message_popup_collection_unittest.cc | 16 +++- ui/views/controls/button/custom_button_unittest.cc | 5 ++ .../native/native_view_host_aura_unittest.cc | 24 +++++- .../controls/native/native_view_host_test_base.cc | 5 ++ .../controls/native/native_view_host_test_base.h | 3 + ui/views/test/views_test_helper_aura.cc | 9 ++ ui/views/test/views_test_helper_mac.h | 3 + ui/views/test/views_test_helper_mac.mm | 15 ++++ .../touch_selection_menu_runner_views_unittest.cc | 3 + ui/views/view_unittest.cc | 98 +++++++++------------- ui/views/view_unittest_aura.cc | 3 +- ui/views/widget/native_widget_mac_unittest.mm | 4 + ui/views/widget/native_widget_unittest.cc | 25 +++--- ui/views/widget/widget_unittest.cc | 7 ++ ui/views/window/dialog_delegate_unittest.cc | 2 + 17 files changed, 166 insertions(+), 84 deletions(-) diff --git a/chrome/browser/ui/views/desktop_media_picker_views_unittest.cc b/chrome/browser/ui/views/desktop_media_picker_views_unittest.cc index 49700c50f691..d43e36c9b598 100644 --- a/chrome/browser/ui/views/desktop_media_picker_views_unittest.cc +++ b/chrome/browser/ui/views/desktop_media_picker_views_unittest.cc @@ -15,6 +15,7 @@ #include "ui/events/event_utils.h" #include "ui/views/test/scoped_views_test_helper.h" #include "ui/views/widget/widget.h" +#include "ui/views/window/dialog_client_view.h" #include "ui/views/window/dialog_delegate.h" namespace views { @@ -25,12 +26,6 @@ class DesktopMediaPickerViewsTest : public testing::Test { ~DesktopMediaPickerViewsTest() override {} void SetUp() override { - Widget::InitParams params(Widget::InitParams::TYPE_WINDOW); - params.context = test_helper_.GetContext(); - params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET; - parent_widget_.reset(new Widget); - parent_widget_->Init(params); - media_list_ = new FakeDesktopMediaList(); scoped_ptr media_list(media_list_); @@ -38,7 +33,7 @@ class DesktopMediaPickerViewsTest : public testing::Test { picker_views_.reset(new DesktopMediaPickerViews()); picker_views_->Show(NULL, - parent_widget_->GetNativeWindow(), + test_helper_.GetContext(), NULL, app_name, app_name, @@ -47,6 +42,13 @@ class DesktopMediaPickerViewsTest : public testing::Test { base::Unretained(this))); } + void TearDown() override { + if (GetPickerDialogView()) { + EXPECT_CALL(*this, OnPickerDone(content::DesktopMediaID())); + GetPickerDialogView()->GetWidget()->CloseNow(); + } + } + DesktopMediaPickerDialogView* GetPickerDialogView() const { return picker_views_->GetDialogViewForTesting(); } @@ -57,7 +59,6 @@ class DesktopMediaPickerViewsTest : public testing::Test { content::TestBrowserThreadBundle thread_bundle_; views::ScopedViewsTestHelper test_helper_; FakeDesktopMediaList* media_list_; - scoped_ptr parent_widget_; scoped_ptr picker_views_; }; @@ -73,7 +74,6 @@ TEST_F(DesktopMediaPickerViewsTest, DoneCallbackCalledOnOkButtonPressed) { EXPECT_CALL(*this, OnPickerDone(content::DesktopMediaID( content::DesktopMediaID::TYPE_WINDOW, kFakeId))); - media_list_->AddSource(kFakeId); EXPECT_FALSE( @@ -83,7 +83,7 @@ TEST_F(DesktopMediaPickerViewsTest, DoneCallbackCalledOnOkButtonPressed) { EXPECT_TRUE( GetPickerDialogView()->IsDialogButtonEnabled(ui::DIALOG_BUTTON_OK)); - GetPickerDialogView()->Accept(); + GetPickerDialogView()->GetDialogClientView()->AcceptWindow(); base::RunLoop().RunUntilIdle(); } diff --git a/ui/chromeos/ime/candidate_window_view_unittest.cc b/ui/chromeos/ime/candidate_window_view_unittest.cc index 6bfa0594deb0..6b18a48c5473 100644 --- a/ui/chromeos/ime/candidate_window_view_unittest.cc +++ b/ui/chromeos/ime/candidate_window_view_unittest.cc @@ -74,6 +74,11 @@ class CandidateWindowViewTest : public views::ViewsTestBase { candidate_window_view_->InitWidget(); } + void TearDown() override { + candidate_window_view_->GetWidget()->CloseNow(); + views::ViewsTestBase::TearDown(); + } + CandidateWindowView* candidate_window_view() { return candidate_window_view_; } @@ -109,8 +114,7 @@ class CandidateWindowViewTest : public views::ViewsTestBase { } private: - // owned by |parent_|. - CandidateWindowView* candidate_window_view_; + CandidateWindowView* candidate_window_view_; // Owned by its Widget. DISALLOW_COPY_AND_ASSIGN(CandidateWindowViewTest); }; diff --git a/ui/message_center/views/message_popup_collection_unittest.cc b/ui/message_center/views/message_popup_collection_unittest.cc index 466181b5725b..74934dd59a41 100644 --- a/ui/message_center/views/message_popup_collection_unittest.cc +++ b/ui/message_center/views/message_popup_collection_unittest.cc @@ -168,6 +168,8 @@ TEST_F(MessagePopupCollectionTest, ShutdownDuringShowing) { // See crbug.com/236448 GetWidget(id1)->CloseNow(); collection()->OnMouseExited(GetToast(id2)); + + GetWidget(id2)->CloseNow(); } TEST_F(MessagePopupCollectionTest, DefaultPositioning) { @@ -206,6 +208,7 @@ TEST_F(MessagePopupCollectionTest, DefaultPositioning) { CloseAllToasts(); EXPECT_EQ(0u, GetToastCounts()); + WaitForTransitionsDone(); } TEST_F(MessagePopupCollectionTest, DefaultPositioningWithRightTaskbar) { @@ -236,6 +239,8 @@ TEST_F(MessagePopupCollectionTest, DefaultPositioningWithRightTaskbar) { // Restore simulated taskbar position to bottom. SetDisplayInfo(gfx::Rect(0, 0, 600, 390), // Work-area. gfx::Rect(0, 0, 600, 400)); // Display-bounds. + + WaitForTransitionsDone(); } TEST_F(MessagePopupCollectionTest, TopDownPositioningWithTopTaskbar) { @@ -260,6 +265,7 @@ TEST_F(MessagePopupCollectionTest, TopDownPositioningWithTopTaskbar) { CloseAllToasts(); EXPECT_EQ(0u, GetToastCounts()); + WaitForTransitionsDone(); // Restore simulated taskbar position to bottom. SetDisplayInfo(gfx::Rect(0, 0, 600, 390), // Work-area. @@ -291,6 +297,7 @@ TEST_F(MessagePopupCollectionTest, TopDownPositioningWithLeftAndTopTaskbar) { CloseAllToasts(); EXPECT_EQ(0u, GetToastCounts()); + WaitForTransitionsDone(); // Restore simulated taskbar position to bottom. SetDisplayInfo(gfx::Rect(0, 0, 600, 390), // Work-area. @@ -322,6 +329,7 @@ TEST_F(MessagePopupCollectionTest, TopDownPositioningWithBottomAndTopTaskbar) { CloseAllToasts(); EXPECT_EQ(0u, GetToastCounts()); + WaitForTransitionsDone(); // Restore simulated taskbar position to bottom. SetDisplayInfo(gfx::Rect(0, 0, 600, 390), // Work-area. @@ -353,6 +361,7 @@ TEST_F(MessagePopupCollectionTest, LeftPositioningWithLeftTaskbar) { CloseAllToasts(); EXPECT_EQ(0u, GetToastCounts()); + WaitForTransitionsDone(); // Restore simulated taskbar position to bottom. SetDisplayInfo(gfx::Rect(0, 0, 600, 390), // Work-area. @@ -387,9 +396,9 @@ TEST_F(MessagePopupCollectionTest, DetectMouseHover) { EXPECT_TRUE(MouseInCollection()); toast1->OnMouseEntered(event); EXPECT_TRUE(MouseInCollection()); - toast0->WindowClosing(); + toast0->GetWidget()->CloseNow(); EXPECT_TRUE(MouseInCollection()); - toast1->WindowClosing(); + toast1->GetWidget()->CloseNow(); EXPECT_FALSE(MouseInCollection()); } @@ -417,6 +426,9 @@ TEST_F(MessagePopupCollectionTest, DetectMouseHoverWithUserClose) { WaitForTransitionsDone(); views::WidgetDelegateView* toast2 = GetToast(id2); EXPECT_TRUE(toast2 != NULL); + + CloseAllToasts(); + WaitForTransitionsDone(); } diff --git a/ui/views/controls/button/custom_button_unittest.cc b/ui/views/controls/button/custom_button_unittest.cc index 7a8fcc04949d..b377a9fa558f 100644 --- a/ui/views/controls/button/custom_button_unittest.cc +++ b/ui/views/controls/button/custom_button_unittest.cc @@ -81,6 +81,11 @@ class CustomButtonTest : public ViewsTestBase { widget_->SetContentsView(button_); } + void TearDown() override { + widget_.reset(); + ViewsTestBase::TearDown(); + } + Widget* widget() { return widget_.get(); } TestCustomButton* button() { return button_; } diff --git a/ui/views/controls/native/native_view_host_aura_unittest.cc b/ui/views/controls/native/native_view_host_aura_unittest.cc index 5f03fc86ee37..16053ede1697 100644 --- a/ui/views/controls/native/native_view_host_aura_unittest.cc +++ b/ui/views/controls/native/native_view_host_aura_unittest.cc @@ -28,6 +28,7 @@ class NativeViewHostWindowObserver : public aura::WindowObserver { EVENT_SHOWN, EVENT_HIDDEN, EVENT_BOUNDS_CHANGED, + EVENT_DESTROYED, }; struct EventDetails { @@ -67,6 +68,11 @@ class NativeViewHostWindowObserver : public aura::WindowObserver { events_.push_back(event); } + void OnWindowDestroyed(aura::Window* window) override { + EventDetails event = { EVENT_DESTROYED, window, gfx::Rect() }; + events_.push_back(event); + } + private: std::vector events_; gfx::Rect bounds_at_visibility_changed_; @@ -253,11 +259,24 @@ TEST_F(NativeViewHostAuraTest, ParentAfterDetach) { aura::Window* root_window = child_win->GetRootWindow(); aura::WindowTreeHost* child_win_tree_host = child_win->GetHost(); + NativeViewHostWindowObserver test_observer; + child_win->AddObserver(&test_observer); + host()->Detach(); EXPECT_EQ(root_window, child_win->GetRootWindow()); EXPECT_EQ(child_win_tree_host, child_win->GetHost()); DestroyHost(); + + // The window is detached, so no longer associated with any Widget hierarchy. + // The root window still owns it, but the test harness checks for orphaned + // windows during TearDown(). + DestroyTopLevel(); + EXPECT_EQ(0u, test_observer.events().size()); + delete child_win; + ASSERT_EQ(1u, test_observer.events().size()); + EXPECT_EQ(NativeViewHostWindowObserver::EVENT_DESTROYED, + test_observer.events().back().type); } // Ensure the clipping window is hidden before setting the native view's bounds. @@ -269,7 +288,9 @@ TEST_F(NativeViewHostAuraTest, RemoveClippingWindowOrder) { NativeViewHostWindowObserver test_observer; clipping_window()->AddObserver(&test_observer); - child()->GetNativeView()->AddObserver(&test_observer); + + aura::Window* child_win = child()->GetNativeView(); + child_win->AddObserver(&test_observer); host()->Detach(); @@ -288,6 +309,7 @@ TEST_F(NativeViewHostAuraTest, RemoveClippingWindowOrder) { child()->GetNativeView()->RemoveObserver(&test_observer); DestroyHost(); + delete child_win; // See comments in ParentAfterDetach. } // Ensure the native view receives the correct bounds notification when it is diff --git a/ui/views/controls/native/native_view_host_test_base.cc b/ui/views/controls/native/native_view_host_test_base.cc index 425d24ae170c..ab40692550e8 100644 --- a/ui/views/controls/native/native_view_host_test_base.cc +++ b/ui/views/controls/native/native_view_host_test_base.cc @@ -29,6 +29,11 @@ NativeViewHostTestBase::NativeViewHostTestBase() : host_destroyed_count_(0) { NativeViewHostTestBase::~NativeViewHostTestBase() { } +void NativeViewHostTestBase::TearDown() { + DestroyTopLevel(); + ViewsTestBase::TearDown(); +} + void NativeViewHostTestBase::CreateTopLevel() { toplevel_.reset(new Widget); Widget::InitParams toplevel_params = diff --git a/ui/views/controls/native/native_view_host_test_base.h b/ui/views/controls/native/native_view_host_test_base.h index 6a39f4d987b1..f663956da617 100644 --- a/ui/views/controls/native/native_view_host_test_base.h +++ b/ui/views/controls/native/native_view_host_test_base.h @@ -20,6 +20,9 @@ class NativeViewHostTestBase : public ViewsTestBase { NativeViewHostTestBase(); ~NativeViewHostTestBase() override; + // testing::Test: + void TearDown() override; + // Create the |toplevel_| widget. void CreateTopLevel(); diff --git a/ui/views/test/views_test_helper_aura.cc b/ui/views/test/views_test_helper_aura.cc index 9f5b87710eac..f3705b6d0830 100644 --- a/ui/views/test/views_test_helper_aura.cc +++ b/ui/views/test/views_test_helper_aura.cc @@ -42,6 +42,15 @@ void ViewsTestHelperAura::SetUp() { } void ViewsTestHelperAura::TearDown() { + // Ensure all Widgets (and windows) are closed in unit tests. This is done + // automatically when the RootWindow is torn down, but is an error on + // platforms that must ensure no Compositors are alive when the ContextFactory + // is torn down. + // So, although it's optional, check the root window to detect failures before + // they hit the CQ on other platforms. + DCHECK(aura_test_helper_->root_window()->children().empty()) + << "Not all windows were closed."; + if (screen_position_client_.get() == aura::client::GetScreenPositionClient(GetContext())) aura::client::SetScreenPositionClient(GetContext(), nullptr); diff --git a/ui/views/test/views_test_helper_mac.h b/ui/views/test/views_test_helper_mac.h index 5f54fbc56c2c..1fa8c8956fb9 100644 --- a/ui/views/test/views_test_helper_mac.h +++ b/ui/views/test/views_test_helper_mac.h @@ -19,6 +19,9 @@ class ViewsTestHelperMac : public ViewsTestHelper { ViewsTestHelperMac(); ~ViewsTestHelperMac() override; + // ViewsTestHelper: + void TearDown() override; + private: // Disable animations during tests. scoped_ptr zero_duration_mode_; diff --git a/ui/views/test/views_test_helper_mac.mm b/ui/views/test/views_test_helper_mac.mm index ac8881796a79..8ef73d27487d 100644 --- a/ui/views/test/views_test_helper_mac.mm +++ b/ui/views/test/views_test_helper_mac.mm @@ -6,8 +6,10 @@ #import +#import "base/mac/scoped_nsautorelease_pool.h" #include "ui/compositor/scoped_animation_duration_scale_mode.h" #include "ui/views/test/event_generator_delegate_mac.h" +#include "ui/views/widget/widget.h" namespace views { @@ -31,4 +33,17 @@ ViewsTestHelperMac::ViewsTestHelperMac() ViewsTestHelperMac::~ViewsTestHelperMac() { } +void ViewsTestHelperMac::TearDown() { + // Ensure all Widgets are closed explicitly in tests. The Widget may be + // hosting a Compositor. If that's torn down after the test ContextFactory + // then a lot of confusing use-after-free errors result. In browser tests, + // this is handled automatically by views::Widget::CloseAllSecondaryWidgets(). + // Unit tests on Aura may create Widgets owned by a RootWindow that gets torn + // down, but on Mac we need to be more explicit. + base::mac::ScopedNSAutoreleasePool pool; // Ensure the NSArray is released. + NSArray* native_windows = [NSApp windows]; + for (NSWindow* window : native_windows) + DCHECK(!Widget::GetWidgetForNativeWindow(window)) << "Widget not closed."; +} + } // namespace views diff --git a/ui/views/touchui/touch_selection_menu_runner_views_unittest.cc b/ui/views/touchui/touch_selection_menu_runner_views_unittest.cc index b5adfb3c2710..eee42581ec36 100644 --- a/ui/views/touchui/touch_selection_menu_runner_views_unittest.cc +++ b/ui/views/touchui/touch_selection_menu_runner_views_unittest.cc @@ -104,6 +104,9 @@ TEST_F(TouchSelectionMenuRunnerViewsTest, QuickMenuAdjustsAnchorRect) { ui::TouchSelectionMenuRunner::GetInstance()->OpenMenu( this, anchor_rect, handle_size, GetContext()); EXPECT_EQ(anchor_rect, GetMenuAnchorRect()); + + ui::TouchSelectionMenuRunner::GetInstance()->CloseMenu(); + RunPendingMessages(); } } // namespace views diff --git a/ui/views/view_unittest.cc b/ui/views/view_unittest.cc index 2bc5f396b0c0..dc07bcbbe390 100644 --- a/ui/views/view_unittest.cc +++ b/ui/views/view_unittest.cc @@ -431,12 +431,35 @@ void TestView::OnPaint(gfx::Canvas* canvas) { did_paint_ = true; } +namespace { + +// Helper class to create a Widget with standard parameters that is closed when +// the helper class goes out of scope. +class ScopedTestPaintWidget { + public: + explicit ScopedTestPaintWidget(const Widget::InitParams& params) + : widget_(new Widget) { + widget_->Init(params); + widget_->GetRootView()->SetBounds(0, 0, 25, 26); + } + + ~ScopedTestPaintWidget() { + widget_->CloseNow(); + } + + Widget* operator->() { return widget_; } + + private: + Widget* widget_; + + DISALLOW_COPY_AND_ASSIGN(ScopedTestPaintWidget); +}; + +} // namespace + TEST_F(ViewTest, PaintEmptyView) { - Widget* widget = new Widget; - Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP); - widget->Init(params); + ScopedTestPaintWidget widget(CreateParams(Widget::InitParams::TYPE_POPUP)); View* root_view = widget->GetRootView(); - root_view->SetBounds(0, 0, 25, 26); // |v1| is empty. TestView* v1 = new TestView; @@ -467,11 +490,8 @@ TEST_F(ViewTest, PaintEmptyView) { } TEST_F(ViewTest, PaintWithUnknownInvalidation) { - Widget* widget = new Widget; - Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP); - widget->Init(params); + ScopedTestPaintWidget widget(CreateParams(Widget::InitParams::TYPE_POPUP)); View* root_view = widget->GetRootView(); - root_view->SetBounds(0, 0, 25, 26); TestView* v1 = new TestView; v1->SetBounds(10, 11, 12, 13); @@ -510,11 +530,8 @@ TEST_F(ViewTest, PaintWithUnknownInvalidation) { } TEST_F(ViewTest, PaintContainsChildren) { - Widget* widget = new Widget; - Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP); - widget->Init(params); + ScopedTestPaintWidget widget(CreateParams(Widget::InitParams::TYPE_POPUP)); View* root_view = widget->GetRootView(); - root_view->SetBounds(0, 0, 25, 26); TestView* v1 = new TestView; v1->SetBounds(10, 11, 12, 13); @@ -546,12 +563,8 @@ TEST_F(ViewTest, PaintContainsChildren) { TEST_F(ViewTest, PaintContainsChildrenInRTL) { ScopedRTL rtl; - - Widget* widget = new Widget; - Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP); - widget->Init(params); + ScopedTestPaintWidget widget(CreateParams(Widget::InitParams::TYPE_POPUP)); View* root_view = widget->GetRootView(); - root_view->SetBounds(0, 0, 25, 26); TestView* v1 = new TestView; v1->SetBounds(10, 11, 12, 13); @@ -593,11 +606,8 @@ TEST_F(ViewTest, PaintContainsChildrenInRTL) { } TEST_F(ViewTest, PaintIntersectsChildren) { - Widget* widget = new Widget; - Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP); - widget->Init(params); + ScopedTestPaintWidget widget(CreateParams(Widget::InitParams::TYPE_POPUP)); View* root_view = widget->GetRootView(); - root_view->SetBounds(0, 0, 25, 26); TestView* v1 = new TestView; v1->SetBounds(10, 11, 12, 13); @@ -629,12 +639,8 @@ TEST_F(ViewTest, PaintIntersectsChildren) { TEST_F(ViewTest, PaintIntersectsChildrenInRTL) { ScopedRTL rtl; - - Widget* widget = new Widget; - Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP); - widget->Init(params); + ScopedTestPaintWidget widget(CreateParams(Widget::InitParams::TYPE_POPUP)); View* root_view = widget->GetRootView(); - root_view->SetBounds(0, 0, 25, 26); TestView* v1 = new TestView; v1->SetBounds(10, 11, 12, 13); @@ -676,11 +682,8 @@ TEST_F(ViewTest, PaintIntersectsChildrenInRTL) { } TEST_F(ViewTest, PaintIntersectsChildButNotGrandChild) { - Widget* widget = new Widget; - Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP); - widget->Init(params); + ScopedTestPaintWidget widget(CreateParams(Widget::InitParams::TYPE_POPUP)); View* root_view = widget->GetRootView(); - root_view->SetBounds(0, 0, 25, 26); TestView* v1 = new TestView; v1->SetBounds(10, 11, 12, 13); @@ -712,12 +715,8 @@ TEST_F(ViewTest, PaintIntersectsChildButNotGrandChild) { TEST_F(ViewTest, PaintIntersectsChildButNotGrandChildInRTL) { ScopedRTL rtl; - - Widget* widget = new Widget; - Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP); - widget->Init(params); + ScopedTestPaintWidget widget(CreateParams(Widget::InitParams::TYPE_POPUP)); View* root_view = widget->GetRootView(); - root_view->SetBounds(0, 0, 25, 26); TestView* v1 = new TestView; v1->SetBounds(10, 11, 12, 13); @@ -759,11 +758,8 @@ TEST_F(ViewTest, PaintIntersectsChildButNotGrandChildInRTL) { } TEST_F(ViewTest, PaintIntersectsNoChildren) { - Widget* widget = new Widget; - Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP); - widget->Init(params); + ScopedTestPaintWidget widget(CreateParams(Widget::InitParams::TYPE_POPUP)); View* root_view = widget->GetRootView(); - root_view->SetBounds(0, 0, 25, 26); TestView* v1 = new TestView; v1->SetBounds(10, 11, 12, 13); @@ -795,12 +791,8 @@ TEST_F(ViewTest, PaintIntersectsNoChildren) { TEST_F(ViewTest, PaintIntersectsNoChildrenInRTL) { ScopedRTL rtl; - - Widget* widget = new Widget; - Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP); - widget->Init(params); + ScopedTestPaintWidget widget(CreateParams(Widget::InitParams::TYPE_POPUP)); View* root_view = widget->GetRootView(); - root_view->SetBounds(0, 0, 25, 26); TestView* v1 = new TestView; v1->SetBounds(10, 11, 12, 13); @@ -842,11 +834,8 @@ TEST_F(ViewTest, PaintIntersectsNoChildrenInRTL) { } TEST_F(ViewTest, PaintIntersectsOneChild) { - Widget* widget = new Widget; - Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP); - widget->Init(params); + ScopedTestPaintWidget widget(CreateParams(Widget::InitParams::TYPE_POPUP)); View* root_view = widget->GetRootView(); - root_view->SetBounds(0, 0, 25, 26); TestView* v1 = new TestView; v1->SetBounds(10, 11, 12, 13); @@ -890,12 +879,8 @@ TEST_F(ViewTest, PaintIntersectsOneChild) { TEST_F(ViewTest, PaintIntersectsOneChildInRTL) { ScopedRTL rtl; - - Widget* widget = new Widget; - Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP); - widget->Init(params); + ScopedTestPaintWidget widget(CreateParams(Widget::InitParams::TYPE_POPUP)); View* root_view = widget->GetRootView(); - root_view->SetBounds(0, 0, 25, 26); TestView* v1 = new TestView; v1->SetBounds(10, 11, 12, 13); @@ -949,11 +934,8 @@ TEST_F(ViewTest, PaintIntersectsOneChildInRTL) { } TEST_F(ViewTest, PaintInPromotedToLayer) { - Widget* widget = new Widget; - Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP); - widget->Init(params); + ScopedTestPaintWidget widget(CreateParams(Widget::InitParams::TYPE_POPUP)); View* root_view = widget->GetRootView(); - root_view->SetBounds(0, 0, 25, 26); TestView* v1 = new TestView; v1->SetPaintToLayer(true); @@ -1590,6 +1572,8 @@ TEST_F(ViewTest, CanProcessEventsWithinSubtree) { result_view = NULL; result_view = root_view->GetTooltipHandlerForPoint(point_in_v); EXPECT_EQ(root_view, result_view); + + widget->CloseNow(); } TEST_F(ViewTest, NotifyEnterExitOnChild) { diff --git a/ui/views/view_unittest_aura.cc b/ui/views/view_unittest_aura.cc index 5b6491010500..f9f72f7d5003 100644 --- a/ui/views/view_unittest_aura.cc +++ b/ui/views/view_unittest_aura.cc @@ -153,8 +153,7 @@ TEST_F(ViewAuraTest, RecreateLayersWithWindows) { ui::Layer* v7_new_layer = w1_new_layer->children()[3]; ASSERT_EQ("v8 v9", ui::test::ChildLayerNamesAsString(*v7_new_layer)); } - // The views and the widgets are destroyed when AuraTestHelper::TearDown() - // destroys root_window(). + w1->CloseNow(); } } // namespace views diff --git a/ui/views/widget/native_widget_mac_unittest.mm b/ui/views/widget/native_widget_mac_unittest.mm index 05fd824eb7d9..4f1f019b801a 100644 --- a/ui/views/widget/native_widget_mac_unittest.mm +++ b/ui/views/widget/native_widget_mac_unittest.mm @@ -403,6 +403,8 @@ TEST_F(NativeWidgetMacTest, AccessibilityIntegration) { id hit = [widget->GetNativeWindow() accessibilityHitTest:midpoint]; id title = [hit accessibilityAttributeValue:NSAccessibilityTitleAttribute]; EXPECT_NSEQ(title, @"Green"); + + widget->CloseNow(); } // Tests creating a views::Widget parented off a native NSWindow. @@ -535,6 +537,8 @@ TEST_F(NativeWidgetMacTest, Tooltips) { // Move the mouse off of any view, tooltip should clear. event_generator.MoveMouseTo(gfx::Point(5, 5)); EXPECT_TRUE(TooltipTextForWidget(widget).empty()); + + widget->CloseNow(); } namespace { diff --git a/ui/views/widget/native_widget_unittest.cc b/ui/views/widget/native_widget_unittest.cc index 20b091e035fa..11930323ee08 100644 --- a/ui/views/widget/native_widget_unittest.cc +++ b/ui/views/widget/native_widget_unittest.cc @@ -81,19 +81,24 @@ TEST_F(NativeWidgetTest, GetTopLevelNativeWidget1) { // |toplevel_widget| has the toplevel NativeWidget. TEST_F(NativeWidgetTest, GetTopLevelNativeWidget2) { - ScopedTestWidget toplevel_widget(CreateNativeWidget()); + internal::NativeWidgetPrivate* child_widget = CreateNativeSubWidget(); + { + ScopedTestWidget toplevel_widget(CreateNativeWidget()); - // |toplevel_widget| owns |child_host|. - NativeViewHost* child_host = new NativeViewHost; - toplevel_widget->GetWidget()->SetContentsView(child_host); + // |toplevel_widget| owns |child_host|. + NativeViewHost* child_host = new NativeViewHost; + toplevel_widget->GetWidget()->SetContentsView(child_host); - // |child_host| owns |child_widget|. - internal::NativeWidgetPrivate* child_widget = CreateNativeSubWidget(); - child_host->Attach(child_widget->GetWidget()->GetNativeView()); + // |child_host| hosts |child_widget|'s NativeView. + child_host->Attach(child_widget->GetWidget()->GetNativeView()); - EXPECT_EQ(toplevel_widget.get(), - internal::NativeWidgetPrivate::GetTopLevelNativeWidget( - child_widget->GetWidget()->GetNativeView())); + EXPECT_EQ(toplevel_widget.get(), + internal::NativeWidgetPrivate::GetTopLevelNativeWidget( + child_widget->GetWidget()->GetNativeView())); + } + // NativeViewHost only had a weak reference to the |child_widget|'s + // NativeView. Delete it and the associated Widget. + child_widget->CloseNow(); } } // namespace views diff --git a/ui/views/widget/widget_unittest.cc b/ui/views/widget/widget_unittest.cc index 6fb2a143077e..ecdacc70e15c 100644 --- a/ui/views/widget/widget_unittest.cc +++ b/ui/views/widget/widget_unittest.cc @@ -1025,6 +1025,9 @@ TEST_F(WidgetObserverTest, WidgetBoundsChanged) { child2->OnNativeWidgetSizeChanged(gfx::Size()); EXPECT_EQ(child2, widget_bounds_changed()); + + child2->CloseNow(); + child1->CloseNow(); } // An extension to WidgetBoundsChanged to ensure notifications are forwarded @@ -1825,6 +1828,8 @@ TEST_F(WidgetTest, SynthesizeMouseMoveEvent) { widget->SynthesizeMouseMoveEvent(); EXPECT_EQ(1, v2->GetEventCount(ui::ET_MOUSE_ENTERED)); + + widget->CloseNow(); } // No touch on desktop Mac. Tracked in http://crbug.com/445520. @@ -2961,6 +2966,7 @@ TEST_F(WidgetTest, GetAllChildWidgets) { EXPECT_EQ(expected.size(), widgets.size()); EXPECT_TRUE(std::equal(expected.begin(), expected.end(), widgets.begin())); + toplevel->CloseNow(); } // Used by DestroyChildWidgetsInOrder. On destruction adds the supplied name to @@ -3439,6 +3445,7 @@ TEST_F(WidgetTest, AlwaysOnTop) { EXPECT_TRUE(widget->IsAlwaysOnTop()); widget->SetAlwaysOnTop(false); EXPECT_FALSE(widget->IsAlwaysOnTop()); + widget->CloseNow(); } } // namespace test diff --git a/ui/views/window/dialog_delegate_unittest.cc b/ui/views/window/dialog_delegate_unittest.cc index 9a653e65af0e..42923399b5e6 100644 --- a/ui/views/window/dialog_delegate_unittest.cc +++ b/ui/views/window/dialog_delegate_unittest.cc @@ -256,6 +256,8 @@ TEST_F(DialogTest, BoundsAccommodateTitle) { dialog()->GetWidget()->UpdateWindowTitle(); EXPECT_EQ(frame1->GetPreferredSize().height(), frame2->GetPreferredSize().height()); + + dialog2->TearDown(); } } // namespace views -- 2.11.4.GIT