views: Fix a case of user-after-free in bubbles.

If the anchor is destroyed while the bubble-widget is invisible, then the
bubble-widget holds a reference to the destroyed anchor-widget without
realizing that it has been destroyed, causing the user-after-free error.

BUG=127539
TEST=aura_shell_unittests:SystemTrayTest tests don't use-after-free.

Review URL: https://chromiumcodereview.appspot.com/10563017

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@142655 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 57 insertions, 12 deletions

diff --git a/ash/system/tray/system_tray_unittest.cc b/ash/system/tray/system_tray_unittest.cc
index 4499da7..1c50a43 100644
--- a/ash/system/tray/system_tray_unittest.cc
+++ b/ash/system/tray/system_tray_unittest.cc
@@ -103,8 +103,7 @@ TEST_F(SystemTrayTest, SystemTrayDefaultView) {
   ASSERT_FALSE(tray->CloseBubbleForTest());
 }
 
-// Disabled due to a use-after-free, see http://crbug.com/127539.
-TEST_F(SystemTrayTest, DISABLED_SystemTrayTestItems) {
+TEST_F(SystemTrayTest, SystemTrayTestItems) {
   SystemTray* tray = GetSystemTray();
   ASSERT_TRUE(tray->GetWidget());
 
@@ -160,8 +159,7 @@ TEST_F(SystemTrayTest, TrayWidgetAutoResizes) {
             tray->GetWidget()->GetWindowScreenBounds().size().ToString());
 }
 
-// Disabled due to a use-after-free, see http://crbug.com/127539.
-TEST_F(SystemTrayTest, DISABLED_SystemTrayNotifications) {
+TEST_F(SystemTrayTest, SystemTrayNotifications) {
   SystemTray* tray = GetSystemTray();
   ASSERT_TRUE(tray->GetWidget());
 
@@ -196,8 +194,7 @@ TEST_F(SystemTrayTest, DISABLED_SystemTrayNotifications) {
   ASSERT_TRUE(test_item->notification_view() != NULL);
 }
 
-// Disabled due to a use-after-free, see http://crbug.com/127539.
-TEST_F(SystemTrayTest, DISABLED_BubbleCreationTypesTest) {
+TEST_F(SystemTrayTest, BubbleCreationTypesTest) {
   SystemTray* tray = GetSystemTray();
   ASSERT_TRUE(tray->GetWidget());
 
diff --git a/ui/views/bubble/bubble_delegate.cc b/ui/views/bubble/bubble_delegate.cc
index 5dc3fcb..1ee9458 100644
--- a/ui/views/bubble/bubble_delegate.cc
+++ b/ui/views/bubble/bubble_delegate.cc
@@ -138,7 +138,10 @@ BubbleDelegateView::BubbleDelegateView(
   AddAccelerator(ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE));
 }
 
-BubbleDelegateView::~BubbleDelegateView() {}
+BubbleDelegateView::~BubbleDelegateView() {
+  if (anchor_widget_)
+    anchor_widget_->RemoveObserver(this);
+}
 
 // static
 Widget* BubbleDelegateView::CreateBubble(BubbleDelegateView* bubble_delegate) {
@@ -146,6 +149,9 @@ Widget* BubbleDelegateView::CreateBubble(BubbleDelegateView* bubble_delegate) {
   // Determine the anchor widget from the anchor view at bubble creation time.
   bubble_delegate->anchor_widget_ = bubble_delegate->anchor_view() ?
       bubble_delegate->anchor_view()->GetWidget() : NULL;
+  if (bubble_delegate->anchor_widget_)
+    bubble_delegate->anchor_widget_->AddObserver(bubble_delegate);
+
   Widget* bubble_widget = CreateBubbleWidget(bubble_delegate);
 
 #if defined(OS_WIN) && !defined(USE_AURA)
@@ -199,16 +205,12 @@ void BubbleDelegateView::OnWidgetVisibilityChanged(Widget* widget,
   if (visible) {
     if (border_widget_)
       border_widget_->Show();
-    if (anchor_widget())
-      anchor_widget()->AddObserver(this);
     GetFocusManager()->SetFocusedView(GetInitiallyFocusedView());
     if (anchor_widget() && anchor_widget()->GetTopLevelWidget())
       anchor_widget()->GetTopLevelWidget()->DisableInactiveRendering();
   } else {
     if (border_widget_)
       border_widget_->Hide();
-    if (anchor_widget())
-      anchor_widget()->RemoveObserver(this);
   }
 }
 
diff --git a/ui/views/bubble/bubble_delegate_unittest.cc b/ui/views/bubble/bubble_delegate_unittest.cc
index 10f7e12..17d6d9c 100644
--- a/ui/views/bubble/bubble_delegate_unittest.cc
+++ b/ui/views/bubble/bubble_delegate_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -29,4 +29,50 @@ TEST_F(BubbleDelegateTest, CreateDelegate) {
   RunPendingMessages();
 }
 
+TEST_F(BubbleDelegateTest, ResetAnchorWidget) {
+  // Create the anchor widget first.
+  Widget* widget = new Widget;
+  View* contents = new View;
+
+  Widget::InitParams params(Widget::InitParams::TYPE_WINDOW);
+  widget->Init(params);
+  widget->SetContentsView(contents);
+  widget->Show();
+
+  Widget* parent = new Widget;
+  parent->Init(params);
+  parent->SetContentsView(new View);
+  parent->Show();
+
+  // Make sure the bubble widget is parented to a widget other than the anchor
+  // widget so that closing the anchor widget does not close the bubble widget.
+  BubbleDelegateView* bubble_delegate =
+      new BubbleDelegateView(contents, BubbleBorder::NONE);
+  bubble_delegate->set_parent_window(parent->GetNativeView());
+  bubble_delegate->set_color(SK_ColorGREEN);
+  Widget* bubble_widget(
+      BubbleDelegateView::CreateBubble(bubble_delegate));
+  EXPECT_EQ(bubble_delegate, bubble_widget->widget_delegate());
+  EXPECT_EQ(bubble_widget, bubble_delegate->GetWidget());
+  EXPECT_EQ(widget, bubble_delegate->anchor_widget());
+
+  bubble_widget->Show();
+  RunPendingMessages();
+  EXPECT_EQ(widget, bubble_delegate->anchor_widget());
+
+  bubble_widget->Hide();
+  RunPendingMessages();
+  EXPECT_EQ(widget, bubble_delegate->anchor_widget());
+
+  // Closing the anchor widget should unset the reference to the anchor widget
+  // for the bubble.
+  widget->CloseNow();
+  RunPendingMessages();
+  EXPECT_FALSE(bubble_delegate->anchor_widget());
+
+  bubble_widget->CloseNow();
+  parent->CloseNow();
+  RunPendingMessages();
+}
+
 }  // namespace views