This CL adds macro used to track the creation and destruction

of HWNDs, in an attempt to detect potential double-delete.
A double-delete of a HWND might be responsible for the crasher
http://crbug.com/4714



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

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

9 files changed, 126 insertions, 10 deletions

diff --git a/base/win_util.cc b/base/win_util.cc
index cfd7f2b..e18007b 100644
--- a/base/win_util.cc
+++ b/base/win_util.cc
@@ -4,12 +4,15 @@
 
 #include "base/win_util.h"
 
+#include <map>
 #include <sddl.h>
 
 #include "base/logging.h"
 #include "base/registry.h"
 #include "base/scoped_handle.h"
+#include "base/singleton.h"
 #include "base/string_util.h"
+#include "base/tracked.h"
 
 namespace win_util {
 
@@ -361,6 +364,66 @@ std::wstring FormatLastWin32Error() {
   return FormatMessage(GetLastError());
 }
 
+typedef std::map<HWND, tracked_objects::Location> HWNDInfoMap;
+struct HWNDBirthMapTrait : public DefaultSingletonTraits<HWNDInfoMap> {
+};
+struct HWNDDeathMapTrait : public DefaultSingletonTraits<HWNDInfoMap> {
+};
+
+void NotifyHWNDCreation(const tracked_objects::Location& from_here, HWND hwnd) {
+  HWNDInfoMap* birth_map = Singleton<HWNDInfoMap, HWNDBirthMapTrait>::get();
+  HWNDInfoMap::iterator birth_iter = birth_map->find(hwnd);
+  if (birth_iter != birth_map->end()) {
+    birth_map->erase(birth_iter);
+
+    // We have already seen this HWND, was it destroyed?
+    HWNDInfoMap* death_map = Singleton<HWNDInfoMap, HWNDDeathMapTrait>::get();
+    HWNDInfoMap::iterator death_iter = death_map->find(hwnd);
+    if (death_iter == death_map->end()) {
+      // We did not get a destruction notification.  The code is probably not
+      // calling NotifyHWNDDestruction for that HWND.
+      NOTREACHED() << "Creation of HWND reported for already tracked HWND. The "
+                      "HWND destruction is probably not tracked properly. "
+                      "Fix it!";
+    } else {
+      death_map->erase(death_iter);
+    }
+  }
+  birth_map->insert(std::pair<HWND, tracked_objects::Location>(hwnd,
+                                                              from_here));
+}
+
+void NotifyHWNDDestruction(const tracked_objects::Location& from_here,
+                           HWND hwnd) {
+  HWNDInfoMap* death_map = Singleton<HWNDInfoMap, HWNDDeathMapTrait>::get();
+  HWNDInfoMap::iterator death_iter = death_map->find(hwnd);
+
+  HWNDInfoMap* birth_map = Singleton<HWNDInfoMap, HWNDBirthMapTrait>::get();
+  HWNDInfoMap::iterator birth_iter = birth_map->find(hwnd);
+
+  if (death_iter != death_map->end()) {
+    std::string allocation, first_delete, second_delete;
+    if (birth_iter != birth_map->end())
+      birth_iter->second.Write(true, true, &allocation);
+    death_iter->second.Write(true, true, &first_delete);
+    from_here.Write(true, true, &second_delete);
+    NOTREACHED() << "Double delete of an HWND. Please file a bug with info on "
+        "how you got that assertion and the following information:\n"
+        "Double delete of HWND 0x" << hwnd << "\n" <<
+        "Allocated at " << allocation << "\n" <<
+        "Deleted first at " << first_delete << "\n" <<
+        "Deleted again at " << second_delete;
+    death_map->erase(death_iter);
+  }
+
+  if (birth_iter == birth_map->end()) {
+    NOTREACHED() << "Destruction of HWND reported for unknown HWND. The HWND "
+                    "construction is probably not tracked properly. Fix it!";
+  }
+  death_map->insert(std::pair<HWND, tracked_objects::Location>(hwnd,
+                                                               from_here));
+}
+
 }  // namespace win_util
 
 #ifdef _MSC_VER
diff --git a/base/win_util.h b/base/win_util.h
index 317c332..f6ce1c2 100644
--- a/base/win_util.h
+++ b/base/win_util.h
@@ -10,6 +10,8 @@
 
 #include <string>
 
+#include "base/tracked.h"
+
 namespace win_util {
 
 // NOTE: Keep these in order so callers can do things like
@@ -102,6 +104,22 @@ std::wstring FormatMessage(unsigned messageid);
 // Uses the last Win32 error to generate a human readable message string.
 std::wstring FormatLastWin32Error();
 
+// These 2 methods are used to track HWND creation/destruction to investigate
+// a mysterious crasher http://crbugs.com/4714 (crasher in on NCDestroy) that
+// might be caused by a multiple delete of an HWND.
+void NotifyHWNDCreation(const tracked_objects::Location& from_here, HWND hwnd);
+void NotifyHWNDDestruction(const tracked_objects::Location& from_here,
+                           HWND hwnd);
+
+#ifdef NDEBUG
+#define TRACK_HWND_CREATION(hwnd)
+#define TRACK_HWND_DESTRUCTION(hwnd)
+#else
+#define TRACK_HWND_CREATION(hwnd) win_util::NotifyHWNDCreation(FROM_HERE, hwnd)
+#define TRACK_HWND_DESTRUCTION(hwnd) \
+    win_util::NotifyHWNDDestruction(FROM_HERE, hwnd)
+#endif
+
 }  // namespace win_util
 
 #endif  // BASE_WIN_UTIL_H__
diff --git a/chrome/browser/plugin_process_host.cc b/chrome/browser/plugin_process_host.cc
index 0e06e3e..feab679 100644
--- a/chrome/browser/plugin_process_host.cc
+++ b/chrome/browser/plugin_process_host.cc
@@ -14,6 +14,7 @@
 #include "base/path_service.h"
 #include "base/process_util.h"
 #include "base/thread.h"
+#include "base/win_util.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_plugin_browsing_context.h"
 #include "chrome/browser/chrome_thread.h"
@@ -351,6 +352,7 @@ class CreateWindowTask : public Task {
         MAKEINTATOM(window_class), 0,
         WS_CHILD | WS_CLIPCHILDREN | WS_CLIPSIBLINGS,
         0, 0, 0, 0, parent_, 0, GetModuleHandle(NULL), 0);
+    TRACK_HWND_CREATION(window);
 
     PluginProcessHostMsg_CreateWindow::WriteReplyParams(
         reply_msg_, window);
@@ -372,6 +374,7 @@ class DestroyWindowTask : public Task {
 
   virtual void Run() {
     DestroyWindow(window_);
+    TRACK_HWND_DESTRUCTION(window_);
   }
 
  private:
diff --git a/chrome/browser/renderer_host/render_widget_host_view_win.cc b/chrome/browser/renderer_host/render_widget_host_view_win.cc
index 2c19e7e..f4c27dd 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_win.cc
+++ b/chrome/browser/renderer_host/render_widget_host_view_win.cc
@@ -396,6 +396,7 @@ LRESULT RenderWidgetHostViewWin::OnCreate(CREATESTRUCT* create_struct) {
   // Call the WM_INPUTLANGCHANGE message handler to initialize the input locale
   // of a browser process.
   OnInputLangChange(0, 0);
+  TRACK_HWND_CREATION(m_hWnd);
   return 0;
 }
 
@@ -413,6 +414,7 @@ void RenderWidgetHostViewWin::OnActivate(UINT action, BOOL minimized,
 void RenderWidgetHostViewWin::OnDestroy() {
   ResetTooltip();
   TrackMouseLeave(false);
+  TRACK_HWND_DESTRUCTION(m_hWnd);
 }
 
 void RenderWidgetHostViewWin::OnPaint(HDC dc) {
diff --git a/chrome/views/native_control.cc b/chrome/views/native_control.cc
index 3c0e8d0..1490f27 100644
--- a/chrome/views/native_control.cc
+++ b/chrome/views/native_control.cc
@@ -79,16 +79,20 @@ class NativeControlContainer : public CWindowImpl<NativeControlContainer,
  private:
 
   LRESULT OnCreate(LPCREATESTRUCT create_struct) {
+    TRACK_HWND_CREATION(m_hWnd);
+
     control_ = parent_->CreateNativeControl(m_hWnd);
+    TRACK_HWND_CREATION(control_);
+
     FocusManager::InstallFocusSubclass(control_, parent_);
-    if (parent_->NotifyOnKeyDown()) {
-      // We subclass the control hwnd so we get the WM_KEYDOWN messages.
-      WNDPROC original_handler =
-          win_util::SetWindowProc(control_,
-                                  &NativeControl::NativeControlWndProc);
-      SetProp(control_, kHandlerKey, original_handler);
-      SetProp(control_, kNativeControlKey , parent_);
-    }
+
+    // We subclass the control hwnd so we get the WM_KEYDOWN messages.
+    WNDPROC original_handler =
+        win_util::SetWindowProc(control_,
+                                &NativeControl::NativeControlWndProc);
+    SetProp(control_, kHandlerKey, original_handler);
+    SetProp(control_, kNativeControlKey , parent_);
+
     ::ShowWindow(control_, SW_SHOW);
     return 1;
   }
@@ -121,6 +125,7 @@ class NativeControlContainer : public CWindowImpl<NativeControlContainer,
   void OnDestroy() {
     if (parent_)
       parent_->OnDestroy();
+    TRACK_HWND_DESTRUCTION(m_hWnd);
   }
 
   void OnContextMenu(HWND window, const CPoint& location) {
@@ -363,7 +368,7 @@ LRESULT CALLBACK NativeControl::NativeControlWndProc(HWND window, UINT message,
       static_cast<NativeControl*>(GetProp(window, kNativeControlKey));
   DCHECK(native_control);
 
-  if (message == WM_KEYDOWN) {
+  if (message == WM_KEYDOWN && native_control->NotifyOnKeyDown()) {
     if (native_control->OnKeyDown(static_cast<int>(w_param)))
       return 0;
   } else if (message == WM_DESTROY) {
@@ -371,6 +376,7 @@ LRESULT CALLBACK NativeControl::NativeControlWndProc(HWND window, UINT message,
                             reinterpret_cast<WNDPROC>(original_handler));
     RemoveProp(window, kHandlerKey);
     RemoveProp(window, kNativeControlKey);
+    TRACK_HWND_DESTRUCTION(window);
   }
 
   return CallWindowProc(reinterpret_cast<WNDPROC>(original_handler), window,
diff --git a/chrome/views/text_field.cc b/chrome/views/text_field.cc
index 79ee7b9..7c4e2fd 100644
--- a/chrome/views/text_field.cc
+++ b/chrome/views/text_field.cc
@@ -70,7 +70,9 @@ class TextField::Edit
     MSG_WM_CHAR(OnChar)
     MSG_WM_CONTEXTMENU(OnContextMenu)
     MSG_WM_COPY(OnCopy)
+    MSG_WM_CREATE(OnCreate)
     MSG_WM_CUT(OnCut)
+    MSG_WM_DESTROY(OnDestroy)
     MESSAGE_HANDLER_EX(WM_IME_STARTCOMPOSITION, OnImeStartComposition)
     MESSAGE_HANDLER_EX(WM_IME_COMPOSITION, OnImeComposition)
     MSG_WM_KEYDOWN(OnKeyDown)
@@ -116,7 +118,9 @@ class TextField::Edit
   void OnChar(TCHAR key, UINT repeat_count, UINT flags);
   void OnContextMenu(HWND window, const CPoint& point);
   void OnCopy();
+  LRESULT OnCreate(CREATESTRUCT* create_struct);
   void OnCut();
+  void OnDestroy();
   LRESULT OnImeStartComposition(UINT message, WPARAM wparam, LPARAM lparam);
   LRESULT OnImeComposition(UINT message, WPARAM wparam, LPARAM lparam);
   void OnKeyDown(TCHAR key, UINT repeat_count, UINT flags);
@@ -414,6 +418,11 @@ void TextField::Edit::OnCopy() {
   }
 }
 
+LRESULT TextField::Edit::OnCreate(CREATESTRUCT* create_struct) {
+  TRACK_HWND_CREATION(m_hWnd);
+  return 0;
+}
+
 void TextField::Edit::OnCut() {
   if (parent_->IsReadOnly())
     return;
@@ -425,6 +434,10 @@ void TextField::Edit::OnCut() {
   ReplaceSel(L"", true);
 }
 
+void TextField::Edit::OnDestroy() {
+  TRACK_HWND_DESTRUCTION(m_hWnd);
+}
+
 LRESULT TextField::Edit::OnImeStartComposition(UINT message,
                                                WPARAM wparam,
                                                LPARAM lparam) {
diff --git a/chrome/views/widget_win.cc b/chrome/views/widget_win.cc
index 7268a59..5e60e38 100644
--- a/chrome/views/widget_win.cc
+++ b/chrome/views/widget_win.cc
@@ -162,6 +162,8 @@ void WidgetWin::Init(HWND parent, const gfx::Rect& bounds,
                          window_style_, bounds.x(), bounds.y(), bounds.width(),
                          bounds.height(), parent, NULL, NULL, this);
   DCHECK(hwnd_);
+  TRACK_HWND_CREATION(hwnd_);
+
   // The window procedure should have set the data for us.
   DCHECK(win_util::GetWindowUserData(hwnd_) == this);
 
@@ -932,6 +934,7 @@ LRESULT CALLBACK WidgetWin::WndProc(HWND window, UINT message,
   if (!widget->ProcessWindowMessage(window, message, w_param, l_param, result))
     result = DefWindowProc(window, message, w_param, l_param);
   if (message == WM_NCDESTROY) {
+    TRACK_HWND_DESTRUCTION(window);
     widget->hwnd_ = NULL;
     widget->OnFinalMessage(window);
   }
diff --git a/webkit/glue/plugins/webplugin_delegate_impl.cc b/webkit/glue/plugins/webplugin_delegate_impl.cc
index b1fc5ae..b024bac 100644
--- a/webkit/glue/plugins/webplugin_delegate_impl.cc
+++ b/webkit/glue/plugins/webplugin_delegate_impl.cc
@@ -13,6 +13,7 @@
 #include "base/message_loop.h"
 #include "base/stats_counters.h"
 #include "base/string_util.h"
+#include "base/win_util.h"
 #include "webkit/default_plugin/plugin_impl.h"
 #include "webkit/glue/glue_util.h"
 #include "webkit/glue/webplugin.h"
@@ -201,6 +202,7 @@ WebPluginDelegateImpl::WebPluginDelegateImpl(
 WebPluginDelegateImpl::~WebPluginDelegateImpl() {
   if (::IsWindow(dummy_window_for_activation_)) {
     ::DestroyWindow(dummy_window_for_activation_);
+    TRACK_HWND_DESTRUCTION(dummy_window_for_activation_);
   }
 
   DestroyInstance();
@@ -447,6 +449,7 @@ bool WebPluginDelegateImpl::WindowedCreatePlugin() {
     0,
     GetModuleHandle(NULL),
     0);
+  TRACK_HWND_CREATION(windowed_handle_);
   if (windowed_handle_ == 0)
     return false;
 
@@ -506,6 +509,7 @@ void WebPluginDelegateImpl::WindowedDestroyWindow() {
     }
 
     DestroyWindow(windowed_handle_);
+    TRACK_HWND_DESTRUCTION(windowed_handle_);
     windowed_handle_ = 0;
   }
 }
@@ -658,6 +662,7 @@ bool WebPluginDelegateImpl::CreateDummyWindowForActivation() {
     0,
     GetModuleHandle(NULL),
     0);
+  TRACK_HWND_CREATION(dummy_window_for_activation_);
 
   if (dummy_window_for_activation_ == 0)
     return false;
diff --git a/webkit/tools/test_shell/webwidget_host_win.cc b/webkit/tools/test_shell/webwidget_host_win.cc
index c282699..845abf3 100644
--- a/webkit/tools/test_shell/webwidget_host_win.cc
+++ b/webkit/tools/test_shell/webwidget_host_win.cc
@@ -36,7 +36,7 @@ WebWidgetHost* WebWidgetHost::Create(gfx::NativeWindow parent_window,
                                kWindowClassName, kWindowClassName, WS_POPUP,
                                0, 0, 0, 0,
                                parent_window, NULL, GetModuleHandle(NULL), NULL);
-
+  TRACK_HWND_CREATION(host->view_);
   win_util::SetWindowUserData(host->view_, host);
 
   host->webwidget_ = WebWidget::Create(delegate);
@@ -58,6 +58,9 @@ LRESULT CALLBACK WebWidgetHost::WndProc(HWND hwnd, UINT message, WPARAM wparam,
       case WM_DESTROY:
         delete host;
         break;
+      case WM_NCDESTROY:
+        TRACK_HWND_DESTRUCTION(hwnd);
+        break;
 
       case WM_PAINT: {
         RECT rect;