Try to make widgets clean up after themselves a bit better:

* When they get "destroy" window manager messages, clean up internal state so nothing will call into them and in theory someone could re-Init() successfully
* When they're deleted, ensure they're also "destroyed" from the window manager point of view

BUG=27347
TEST=none
Review URL: http://codereview.chromium.org/391010

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

4 files changed, 33 insertions, 14 deletions

diff --git a/app/win/window_impl.cc b/app/win/window_impl.cc
index b28eab5..8ddd2bc 100644
--- a/app/win/window_impl.cc
+++ b/app/win/window_impl.cc
@@ -180,7 +180,15 @@ LRESULT CALLBACK WindowImpl::WndProc(HWND hwnd,
   if (!window)
     return 0;
 
-  return window->OnWndProc(message, w_param, l_param);
+  LRESULT return_code = window->OnWndProc(message, w_param, l_param);
+
+  // Clean up state.  Do this after the above call so subclasses still have a
+  // valid hwnd().  Can't wait until WM_NCDESTROY because by then |hwnd| is not
+  // guaranteed to be reported to us.
+  if (message == WM_DESTROY)
+    window->OnDestroy();
+
+  return return_code;
 }
 
 std::wstring WindowImpl::GetWindowClassName() {
@@ -211,4 +219,8 @@ std::wstring WindowImpl::GetWindowClassName() {
   return name;
 }
 
+void WindowImpl::OnDestroy() {
+  hwnd_ = 0;
+}
+
 }  // namespace app
diff --git a/app/win/window_impl.h b/app/win/window_impl.h
index fb5d512..dd7ef5a 100644
--- a/app/win/window_impl.h
+++ b/app/win/window_impl.h
@@ -87,6 +87,10 @@ class WindowImpl : public MessageMapInterface {
   // If necessary, this registers the window class.
   std::wstring GetWindowClassName();
 
+  // Called when the HWND gets a WM_DESTROY message; should cause us to clean
+  // up any state.
+  void OnDestroy();
+
   // All classes registered by WidgetWin start with this name.
   static const wchar_t* const kBaseClassName;
 
diff --git a/views/widget/widget_gtk.cc b/views/widget/widget_gtk.cc
index 8762e14..64a9063 100644
--- a/views/widget/widget_gtk.cc
+++ b/views/widget/widget_gtk.cc
@@ -106,9 +106,10 @@ WidgetGtk::WidgetGtk(Type type)
 }
 
 WidgetGtk::~WidgetGtk() {
-  if (type_ != TYPE_CHILD)
-    ActiveWindowWatcherX::RemoveObserver(this);
-  MessageLoopForUI::current()->RemoveObserver(this);
+  // If we're deleted directly, instead of via OnDestroy(), then clean up after
+  // ourselves.
+  delete_on_destroy_ = false;  // Prevent double free.
+  CloseNow();
 }
 
 GtkWindow* WidgetGtk::GetTransientParent() const {
@@ -818,6 +819,9 @@ void WidgetGtk::OnGrabNotify(GtkWidget* widget, gboolean was_grabbed) {
 
 void WidgetGtk::OnDestroy() {
   widget_ = window_contents_ = NULL;
+  if (type_ != TYPE_CHILD)
+    ActiveWindowWatcherX::RemoveObserver(this);
+  MessageLoopForUI::current()->RemoveObserver(this);
   if (delete_on_destroy_) {
     // Delays the deletion of this WidgetGtk as we want its children to have
     // access to it when destroyed.
diff --git a/views/widget/widget_win.cc b/views/widget/widget_win.cc
index 79956a8..2c07c3d 100644
--- a/views/widget/widget_win.cc
+++ b/views/widget/widget_win.cc
@@ -58,7 +58,10 @@ WidgetWin::WidgetWin()
 }
 
 WidgetWin::~WidgetWin() {
-  MessageLoopForUI::current()->RemoveObserver(this);
+  // If we're deleted directly, instead of via a WM_NCDESTROY, then clean up
+  // after ourselves.
+  delete_on_destroy_ = false;  // Prevent double free.
+  CloseNow();
 }
 
 // static
@@ -115,17 +118,12 @@ void WidgetWin::Init(gfx::NativeView parent, const gfx::Rect& bounds) {
   // Windows special DWM window frame requires a special tooltip manager so
   // that window controls in Chrome windows don't flicker when you move your
   // mouse over them. See comment in aero_tooltip_manager.h.
-  if (GetThemeProvider()->ShouldUseNativeFrame()) {
-    tooltip_manager_.reset(new AeroTooltipManager(this));
-  } else {
-    tooltip_manager_.reset(new TooltipManagerWin(this));
-  }
+  tooltip_manager_.reset(GetThemeProvider()->ShouldUseNativeFrame() ?
+      new AeroTooltipManager(this) : new TooltipManagerWin(this));
 
   // This message initializes the window so that focus border are shown for
   // windows.
-  SendMessage(hwnd(),
-              WM_CHANGEUISTATE,
-              MAKELPARAM(UIS_CLEAR, UISF_HIDEFOCUS),
+  SendMessage(hwnd(), WM_CHANGEUISTATE, MAKELPARAM(UIS_CLEAR, UISF_HIDEFOCUS),
               0);
 
   // Bug 964884: detach the IME attached to this window.
@@ -827,6 +825,7 @@ void WidgetWin::OnThemeChanged() {
 }
 
 void WidgetWin::OnFinalMessage(HWND window) {
+  MessageLoopForUI::current()->RemoveObserver(this);
   if (delete_on_destroy_)
     delete this;
 }
@@ -1120,7 +1119,7 @@ LRESULT WidgetWin::OnWndProc(UINT message, WPARAM w_param, LPARAM l_param) {
   if (!ProcessWindowMessage(window, message, w_param, l_param, result))
     result = DefWindowProc(window, message, w_param, l_param);
   if (message == WM_NCDESTROY)
-    OnFinalMessage(window);
+    OnFinalMessage(window);  // CAUTION: May reset members and/or delete |this|!
   if (message == WM_ACTIVATE)
     PostProcessActivateMessage(this, LOWORD(w_param));
   if (message == WM_ENABLE && restore_focus_when_enabled_) {