Avoid dangling references to Views from FocusManager.

When handling a native view hierarchy change, make sure any Views
that are going to become disassociated from the FocusManager are
removed from focus.

BUG=75172
TEST=none

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

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

6 files changed, 39 insertions, 20 deletions

diff --git a/chrome/browser/ui/views/tab_contents/native_tab_contents_container_win.cc b/chrome/browser/ui/views/tab_contents/native_tab_contents_container_win.cc
index e4dfd02..08e4bf7 100644
--- a/chrome/browser/ui/views/tab_contents/native_tab_contents_container_win.cc
+++ b/chrome/browser/ui/views/tab_contents/native_tab_contents_container_win.cc
@@ -38,9 +38,13 @@ void NativeTabContentsContainerWin::AttachContents(TabContents* contents) {
 }
 
 void NativeTabContentsContainerWin::DetachContents(TabContents* contents) {
-  // TODO(brettw) should this move to NativeViewHost::Detach which is called
-  // below?
-  // It needs cleanup regardless.
+  // Detach the TabContents.  Do this before we unparent the
+  // TabContentsViewWin so that the window hierarchy is intact for any
+  // cleanup during Detach().
+  Detach();
+
+  // TODO(brettw) should this move to NativeViewHost::Detach?  It
+  // needs cleanup regardless.
   HWND container_hwnd = contents->GetNativeView();
   if (container_hwnd) {
     // Hide the contents before adjusting its parent to avoid a full desktop
@@ -50,9 +54,6 @@ void NativeTabContentsContainerWin::DetachContents(TabContents* contents) {
     // Reset the parent to NULL to ensure hidden tabs don't receive messages.
     static_cast<TabContentsViewWin*>(contents->view())->Unparent();
   }
-
-  // Now detach the TabContents.
-  Detach();
 }
 
 void NativeTabContentsContainerWin::SetFastResize(bool fast_resize) {
diff --git a/views/controls/native/native_view_host_win.cc b/views/controls/native/native_view_host_win.cc
index dbb8461..f8bda8e 100644
--- a/views/controls/native/native_view_host_win.cc
+++ b/views/controls/native/native_view_host_win.cc
@@ -43,7 +43,7 @@ void NativeViewHostWin::NativeViewAttached() {
   NativeWidget::GetAllNativeWidgets(host_->native_view(), &widgets);
   for (NativeWidget::NativeWidgets::iterator it = widgets.begin();
        it != widgets.end(); ++it) {
-    (*it)->GetWidget()->GetRootView()->NotifyNativeViewHierarchyChanged(
+    (*it)->GetWidget()->NotifyNativeViewHierarchyChanged(
         true,
         host_->GetWidget()->GetNativeView());
   }
@@ -58,7 +58,7 @@ void NativeViewHostWin::NativeViewDetaching(bool destroyed) {
   NativeWidget::GetAllNativeWidgets(host_->native_view(), &widgets);
   for (NativeWidget::NativeWidgets::iterator it = widgets.begin();
        it != widgets.end(); ++it) {
-    (*it)->GetWidget()->GetRootView()->NotifyNativeViewHierarchyChanged(
+    (*it)->GetWidget()->NotifyNativeViewHierarchyChanged(
         false,
         host_->GetWidget()->GetNativeView());
   }
diff --git a/views/focus/focus_manager.cc b/views/focus/focus_manager.cc
index 86df1f0..5712b2e 100644
--- a/views/focus/focus_manager.cc
+++ b/views/focus/focus_manager.cc
@@ -483,9 +483,13 @@ bool FocusManager::IsTabTraversalKeyEvent(const KeyEvent& key_event) {
   return key_event.key_code() == ui::VKEY_TAB && !key_event.IsControlDown();
 }
 
-void FocusManager::ViewRemoved(View* parent, View* removed) {
-  if (focused_view_ && focused_view_ == removed)
-    ClearFocus();
+void FocusManager::ViewRemoved(View* removed) {
+  // If the view being removed contains (or is) the focused view,
+  // clear the focus.  However, it's not safe to call ClearFocus()
+  // (and in turn ClearNativeFocus()) here because ViewRemoved() can
+  // be called while the top level widget is being destroyed.
+  if (focused_view_ && removed && removed->Contains(focused_view_))
+    SetFocusedView(NULL);
 }
 
 void FocusManager::AddFocusChangeListener(FocusChangeListener* listener) {
diff --git a/views/focus/focus_manager.h b/views/focus/focus_manager.h
index 250adb3..25669ad 100644
--- a/views/focus/focus_manager.h
+++ b/views/focus/focus_manager.h
@@ -254,10 +254,11 @@ class FocusManager {
   // Returns true if an accelerator was activated.
   bool ProcessAccelerator(const Accelerator& accelerator);
 
-  // Called by a RootView when a view within its hierarchy is removed from its
-  // parent. This will only be called by a RootView in a hierarchy of Widgets
-  // that this FocusManager is attached to the parent Widget of.
-  void ViewRemoved(View* parent, View* removed);
+  // Called by a RootView when a view within its hierarchy is removed
+  // from its parent. This will only be called by a RootView in a
+  // hierarchy of Widgets that this FocusManager is attached to the
+  // parent Widget of.
+  void ViewRemoved(View* removed);
 
   // Adds/removes a listener.  The FocusChangeListener is notified every time
   // the focused view is about to change.
diff --git a/views/widget/widget.cc b/views/widget/widget.cc
index c34eb09..4a58b32 100644
--- a/views/widget/widget.cc
+++ b/views/widget/widget.cc
@@ -60,15 +60,24 @@ void Widget::ViewHierarchyChanged(bool is_add, View* parent, View* child) {
       dragged_view_ = NULL;
 
     FocusManager* focus_manager = GetFocusManager();
-    if (focus_manager) {
-      if (focus_manager->GetFocusedView() == child)
-        focus_manager->SetFocusedView(NULL);
-      focus_manager->ViewRemoved(parent, child);
-    }
+    if (focus_manager)
+      focus_manager->ViewRemoved(child);
     ViewStorage::GetInstance()->ViewRemoved(parent, child);
   }
 }
 
+void Widget::NotifyNativeViewHierarchyChanged(bool attached,
+                                              gfx::NativeView native_view) {
+  if (!attached) {
+    FocusManager* focus_manager = GetFocusManager();
+    // We are being removed from a window hierarchy.  Treat this as
+    // the root_view_ being removed.
+    if (focus_manager)
+      focus_manager->ViewRemoved(root_view_.get());
+  }
+  root_view_->NotifyNativeViewHierarchyChanged(attached, native_view);
+}
+
 // Converted methods (see header) ----------------------------------------------
 
 Widget* Widget::GetTopLevelWidget() {
diff --git a/views/widget/widget.h b/views/widget/widget.h
index 2886c04..e5dc678 100644
--- a/views/widget/widget.h
+++ b/views/widget/widget.h
@@ -130,6 +130,10 @@ class Widget : public internal::NativeWidgetDelegate,
   // Forwarded from the RootView so that the widget can do any cleanup.
   virtual void ViewHierarchyChanged(bool is_add, View* parent, View* child);
 
+  // Performs any necessary cleanup and forwards to RootView.
+  virtual void NotifyNativeViewHierarchyChanged(bool attached,
+                                                gfx::NativeView native_view);
+
   // Converted methods ---------------------------------------------------------
 
   // TODO(beng):