RootView should be destroyed in WM_NCDESTROY not after it.

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

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

3 files changed, 16 insertions, 11 deletions

diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc
index ccbb950..40b0d3b 100644
--- a/chrome/browser/views/frame/browser_view.cc
+++ b/chrome/browser/views/frame/browser_view.cc
@@ -629,16 +629,10 @@ void BrowserView::Observe(NotificationType type,
 // BrowserView, TabStripModelObserver implementation:
 
 void BrowserView::TabDetachedAt(TabContents* contents, int index) {
-  // We use index here rather than comparing |contents| because by this time
-  // the model has already removed |contents| from its list, so
-  // browser_->GetSelectedTabContents() will return NULL or something else.
-  if (index == browser_->tabstrip_model()->selected_index()) {
-    // We need to reset the current tab contents to NULL before it gets
-    // freed. This is because the focus manager performs some operations
-    // on the selected TabContents when it is removed.
-    infobar_container_->ChangeTabContents(NULL);
-    contents_container_->SetTabContents(NULL);
-  }
+  // We need to break the TabContentsContainerView's attachment to whatever HWND
+  // it's hosting since if the window is destroyed it'll try to hide it even
+  // after the new window has parented it.
+  contents_container_->SetTabContents(NULL);
 }
 
 void BrowserView::TabSelectedAt(TabContents* old_contents,
diff --git a/chrome/views/widget_win.cc b/chrome/views/widget_win.cc
index cd0e913..2707894 100644
--- a/chrome/views/widget_win.cc
+++ b/chrome/views/widget_win.cc
@@ -132,7 +132,6 @@ WidgetWin::WidgetWin()
 }
 
 WidgetWin::~WidgetWin() {
-  MessageLoopForUI::current()->RemoveObserver(this);
 }
 
 void WidgetWin::Init(HWND parent, const gfx::Rect& bounds,
@@ -550,6 +549,16 @@ LRESULT WidgetWin::OnMouseRange(UINT msg, WPARAM w_param, LPARAM l_param) {
   return 0;
 }
 
+void WidgetWin::OnNCDestroy() {
+  // Destroy the view hierarchy here as the window is destroyed, _before_ the
+  // Widget is destroyed and before our associated focus manager is destroyed.
+  // Views in the view hierarchy may attempt to manipulate the focus manager or
+  // other aspects of the window in ways that are not compatible with the fact
+  // that it has just been destroyed.
+  MessageLoopForUI::current()->RemoveObserver(this);
+  root_view_.reset(NULL);
+}
+
 void WidgetWin::OnNCLButtonDblClk(UINT flags, const CPoint& point) {
   SetMsgHandled(FALSE);
 }
diff --git a/chrome/views/widget_win.h b/chrome/views/widget_win.h
index 9e402e8..5bcc166 100644
--- a/chrome/views/widget_win.h
+++ b/chrome/views/widget_win.h
@@ -207,6 +207,7 @@ class WidgetWin : public Widget,
     MSG_WM_MOVING(OnMoving)
     MSG_WM_NCACTIVATE(OnNCActivate)
     MSG_WM_NCCALCSIZE(OnNCCalcSize)
+    MSG_WM_NCDESTROY(OnNCDestroy)
     MSG_WM_NCHITTEST(OnNCHitTest)
     MSG_WM_NCMOUSEMOVE(OnNCMouseMove)
     MSG_WM_NCLBUTTONDBLCLK(OnNCLButtonDblClk)
@@ -384,6 +385,7 @@ class WidgetWin : public Widget,
   virtual LRESULT OnMouseRange(UINT msg, WPARAM w_param, LPARAM l_param);
   virtual LRESULT OnNCActivate(BOOL active) { SetMsgHandled(FALSE); return 0; }
   virtual LRESULT OnNCCalcSize(BOOL w_param, LPARAM l_param) { SetMsgHandled(FALSE); return 0; }
+  virtual void OnNCDestroy();
   virtual LRESULT OnNCHitTest(const CPoint& pt) { SetMsgHandled(FALSE); return 0; }
   virtual void OnNCLButtonDblClk(UINT flags, const CPoint& point);
   virtual void OnNCLButtonDown(UINT flags, const CPoint& point);