Fixes use after free in HWNDMessageHandler

IsMsgHandled() shouldn't check weaf_factory_. That defeats its
purposes.

BUG=391050
TEST=none
R=ananta@chromium.org

Review URL: https://codereview.chromium.org/423593005

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

2 files changed, 21 insertions, 15 deletions

diff --git a/ui/views/win/hwnd_message_handler.cc b/ui/views/win/hwnd_message_handler.cc
index 0e513db..ef793e1 100644
--- a/ui/views/win/hwnd_message_handler.cc
+++ b/ui/views/win/hwnd_message_handler.cc
@@ -915,12 +915,13 @@ LRESULT HWNDMessageHandler::OnWndProc(UINT message,
     return 0;
   msg_handled_ = old_msg_handled;
 
-  if (!processed)
+  if (!processed) {
     result = DefWindowProc(window, message, w_param, l_param);
-
-  // DefWindowProc() may have destroyed the window in a nested message loop.
-  if (!::IsWindow(window))
-    return result;
+    // DefWindowProc() may have destroyed the window and/or us in a nested
+    // message loop.
+    if (!ref || !::IsWindow(window))
+      return result;
+  }
 
   if (delegate_) {
     delegate_->PostHandleMSG(message, w_param, l_param);
@@ -939,33 +940,37 @@ LRESULT HWNDMessageHandler::HandleMouseMessage(unsigned int message,
                                                bool* handled) {
   // Don't track forwarded mouse messages. We expect the caller to track the
   // mouse.
+  base::WeakPtr<HWNDMessageHandler> ref(weak_factory_.GetWeakPtr());
   LRESULT ret = HandleMouseEventInternal(message, w_param, l_param, false);
   *handled = IsMsgHandled();
   return ret;
 }
 
-LRESULT HWNDMessageHandler::HandleTouchMessage(unsigned int message,
-                                               WPARAM w_param,
-                                               LPARAM l_param,
-                                               bool* handled) {
-  LRESULT ret = OnTouchEvent(message, w_param, l_param);
-  *handled = IsMsgHandled();
-  return ret;
-}
-
 LRESULT HWNDMessageHandler::HandleKeyboardMessage(unsigned int message,
                                                   WPARAM w_param,
                                                   LPARAM l_param,
                                                   bool* handled) {
+  base::WeakPtr<HWNDMessageHandler> ref(weak_factory_.GetWeakPtr());
   LRESULT ret = OnKeyEvent(message, w_param, l_param);
   *handled = IsMsgHandled();
   return ret;
 }
 
+LRESULT HWNDMessageHandler::HandleTouchMessage(unsigned int message,
+                                               WPARAM w_param,
+                                               LPARAM l_param,
+                                               bool* handled) {
+  base::WeakPtr<HWNDMessageHandler> ref(weak_factory_.GetWeakPtr());
+  LRESULT ret = OnTouchEvent(message, w_param, l_param);
+  *handled = IsMsgHandled();
+  return ret;
+}
+
 LRESULT HWNDMessageHandler::HandleScrollMessage(unsigned int message,
                                                 WPARAM w_param,
                                                 LPARAM l_param,
                                                 bool* handled) {
+  base::WeakPtr<HWNDMessageHandler> ref(weak_factory_.GetWeakPtr());
   LRESULT ret = OnScrollMessage(message, w_param, l_param);
   *handled = IsMsgHandled();
   return ret;
@@ -975,6 +980,7 @@ LRESULT HWNDMessageHandler::HandleNcHitTestMessage(unsigned int message,
                                                    WPARAM w_param,
                                                    LPARAM l_param,
                                                    bool* handled) {
+  base::WeakPtr<HWNDMessageHandler> ref(weak_factory_.GetWeakPtr());
   LRESULT ret = OnNCHitTest(
       gfx::Point(CR_GET_X_LPARAM(l_param), CR_GET_Y_LPARAM(l_param)));
   *handled = IsMsgHandled();
diff --git a/ui/views/win/hwnd_message_handler.h b/ui/views/win/hwnd_message_handler.h
index 972670f..4109599 100644
--- a/ui/views/win/hwnd_message_handler.h
+++ b/ui/views/win/hwnd_message_handler.h
@@ -60,7 +60,7 @@ const int WM_NCUAHDRAWFRAME = 0xAF;
 // IsMsgHandled() from a member function to a define that checks if the weak
 // factory is still valid in addition to the member. Together these allow for
 // |this| to be deleted during dispatch.
-#define IsMsgHandled() !weak_factory_.GetWeakPtr().get() || msg_handled_
+#define IsMsgHandled() !ref.get() || msg_handled_
 
 #define BEGIN_SAFE_MSG_MAP_EX(the_class) \
  private: \