Views views_unittests FocusManagerTest.FocusNativeControls use after free

Fixes issue where NativeTabbedPaneGtk gets retained by the top-level FocusManager during the Widget::Close tear-down sequence.  This fix clears focus before proceeding with the tear-down.  This avoids redundant operations with the FocusManger as views get deleted, specifically in the FocusManager::ViewRemoved() call.

Caught by Valgrind as a use after free violation:
  sh tools/valgrind/chrome_tests.sh views --gtest_filter=FocusManagerTest.FocusNativeControls

BUG=89596
TEST=tools/valgrind/chrome_tests.sh views --gtest_filter=FocusManagerTest.FocusNativeControls


Review URL: http://codereview.chromium.org/7468037

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

3 files changed, 9 insertions, 15 deletions

diff --git a/tools/valgrind/memcheck/suppressions.txt b/tools/valgrind/memcheck/suppressions.txt
index dee6edc..84d205b 100644
--- a/tools/valgrind/memcheck/suppressions.txt
+++ b/tools/valgrind/memcheck/suppressions.txt
@@ -4834,20 +4834,6 @@
    fun:_ZN3net16HttpStreamParser21DoReadHeadersCompleteEi
 }
 {
-   bug_89596
-   Memcheck:Addr4
-   fun:_ZNK5views4View8ContainsEPKS0_
-   fun:_ZN5views12FocusManager11ViewRemovedEPNS_4ViewE
-   fun:_ZN5views6Widget20ViewHierarchyChangedEbPNS_4ViewES2_
-   fun:_ZN5views8internal8RootView20ViewHierarchyChangedEbPNS_4ViewES3_
-   fun:_ZN5views4View24ViewHierarchyChangedImplEbbPS0_S1_
-   ...
-   fun:_ZN5views4View28PropagateRemoveNotificationsEPS0_
-   fun:_ZN5views4View17DoRemoveChildViewEPS0_bbb
-   fun:_ZN5views4View19RemoveAllChildViewsEb
-   fun:_ZN5views8internal8RootViewD0Ev
-}
-{
    bug_89677
    Memcheck:Leak
    fun:_Znw*
diff --git a/views/focus/focus_manager_unittest.cc b/views/focus/focus_manager_unittest.cc
index e5d7c0b..94d9a88 100644
--- a/views/focus/focus_manager_unittest.cc
+++ b/views/focus/focus_manager_unittest.cc
@@ -961,7 +961,6 @@ TEST_F(FocusManagerTest, FocusNativeControls) {
   EXPECT_EQ(tab_button, GetFocusManager()->GetFocusedView());
 }
 
-
 // On linux, we don't store/restore focused view because gtk handles
 // this (and pure views will be the same).
 #if defined(OS_WIN)
diff --git a/views/widget/widget.cc b/views/widget/widget.cc
index d00168c..bfead84 100644
--- a/views/widget/widget.cc
+++ b/views/widget/widget.cc
@@ -423,6 +423,15 @@ void Widget::Close() {
     can_close = non_client_view_->CanClose();
   if (can_close) {
     SaveWindowPosition();
+
+    // During tear-down the top-level focus manager becomes unavailable to
+    // GTK tabbed panes and their children, so normal deregistration via
+    // |FormManager::ViewRemoved()| calls are fouled.  We clear focus here
+    // to avoid these redundant steps and to avoid accessing deleted views
+    // that may have been in focus.
+    if (GetTopLevelWidget() == this && focus_manager_.get())
+      focus_manager_->SetFocusedView(NULL);
+
     native_widget_->Close();
     widget_closed_ = true;
   }