Fix crash in the HWNDMessageHandler::HandleMouseInputForCaption function.

ui/views/widget/widget_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <algorithm>
 #include <set>
 
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 3195 matching lines @@
   target->HandleKeyboardMessage(WM_SYSCHAR, 0, 0, &handled);
   target->HandleKeyboardMessage(WM_SYSDEADCHAR, 0, 0, &handled);
   widget.CloseNow();
 }
 
 // Provides functionality to subclass a window and keep track of messages
 // received.
 class SubclassWindowHelper {
  public:
   explicit SubclassWindowHelper(HWND window)
-      : window_(window) {
+      : window_(window),
+        destroy_on_message_(0) {
     EXPECT_EQ(instance_, nullptr);
     instance_ = this;
     EXPECT_TRUE(Subclass());
   }
 
   ~SubclassWindowHelper() {
     Unsubclass();
     instance_ = nullptr;
   }
 
   // Returns true if the |message| passed in was received.
   bool received_message(unsigned int message) {
     return (messages_.find(message) != messages_.end());
   }
 
   void Clear() {
     messages_.clear();
   }
 
+  void set_destroy_on_message(unsigned int message) {
+    destroy_on_message_ = message;
+  }
+
  private:
   bool Subclass() {
     old_proc_ = reinterpret_cast<WNDPROC>(
         ::SetWindowLongPtr(window_,
                            GWLP_WNDPROC,
                            reinterpret_cast<LONG_PTR>(WndProc)));
     return old_proc_ != nullptr;
   }
 
   void Unsubclass() {
     ::SetWindowLongPtr(window_,
                        GWLP_WNDPROC,
                        reinterpret_cast<LONG_PTR>(old_proc_));
   }
 
   static LRESULT CALLBACK WndProc(HWND window,
                                   unsigned int message,
                                   WPARAM w_param,
                                   LPARAM l_param) {
     EXPECT_NE(instance_, nullptr);
     EXPECT_EQ(window, instance_->window_);
 
     // Keep track of messags received for this window.
     instance_->messages_.insert(message);
 
-    return ::CallWindowProc(instance_->old_proc_, window, message, w_param,
-                            l_param);
+    LRESULT ret = ::CallWindowProc(instance_->old_proc_, window, message,
+                                   w_param, l_param);
+    if (instance_->destroy_on_message_ &&

[sky, 2016/01/27 03:14:30]

Is there a reason for this check? Doesn't the next line cover what you want?

[ananta, 2016/01/27 21:23:31]

Yeah. Wrote that thinking about WM_NULL. Not needed. 

+        message == instance_->destroy_on_message_) {
+      instance_->Unsubclass();
+      ::DestroyWindow(window);
+    }
+    return ret;
   }
 
   WNDPROC old_proc_;
   HWND window_;
   static SubclassWindowHelper* instance_;
   std::set<unsigned int> messages_;
+  unsigned int destroy_on_message_;

[sky, 2016/01/27 03:14:30]

nit: I first took this to mean destroy on any message, but it means destroy on a
specific message. So, how about message_to_destroy_on_?

[ananta, 2016/01/27 21:23:31]

Done.

 
   DISALLOW_COPY_AND_ASSIGN(SubclassWindowHelper);
 };
 
 SubclassWindowHelper* SubclassWindowHelper::instance_ = nullptr;
 
 // This test validates whether the WM_SYSCOMMAND message for SC_MOVE is
 // received when we post a WM_NCLBUTTONDOWN message for the caption in the
 // following scenarios:-
 // 1. Posting a WM_NCMOUSEMOVE message for a different location.
@@ skipping 61 matching lines @@
   ::PostMessage(window, WM_NCLBUTTONDOWN, HTCAPTION, MAKELPARAM(100, 100));
   ::PostMessage(window, WM_MOUSEMOVE, HTCLIENT, MAKELPARAM(110, 110));
   RunPendingMessages();
 
   EXPECT_TRUE(subclass_helper.received_message(WM_NCLBUTTONDOWN));
   EXPECT_TRUE(subclass_helper.received_message(WM_MOUSEMOVE));
   EXPECT_TRUE(subclass_helper.received_message(WM_SYSCOMMAND));
 
   widget.CloseNow();
 }
+
+// This test validates that destroying the window in the context of the
+// WM_SYSCOMMAND message with SC_MOVE does not crash.
+TEST_F(WidgetTest, DestroyInSysCommandNCLButtonDownOnCaption) {
+  Widget widget;
+  Widget::InitParams params =
+      CreateParams(Widget::InitParams::TYPE_WINDOW);
+  params.native_widget = new PlatformDesktopNativeWidget(&widget);
+  params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  widget.Init(params);
+  widget.SetBounds(gfx::Rect(0, 0, 200, 200));
+  widget.Show();
+  ::SetCursorPos(500, 500);
+
+  HWND window = widget.GetNativeWindow()->GetHost()->GetAcceleratedWidget();
+
+  SubclassWindowHelper subclass_helper(window);
+
+  // Destroying the window in the context of the WM_SYSCOMMAND message
+  // should not crash.
+  subclass_helper.set_destroy_on_message(WM_SYSCOMMAND);
+
+  ::PostMessage(window, WM_NCLBUTTONDOWN, HTCAPTION, MAKELPARAM(100, 100));
+  ::PostMessage(window, WM_NCMOUSEMOVE, HTCAPTION, MAKELPARAM(110, 110));
+  RunPendingMessages();
+
+  EXPECT_TRUE(subclass_helper.received_message(WM_NCLBUTTONDOWN));
+  EXPECT_TRUE(subclass_helper.received_message(WM_SYSCOMMAND));
+
+  widget.CloseNow();
+}
+
 #endif
 
 // Test that SetAlwaysOnTop and IsAlwaysOnTop are consistent.
 TEST_F(WidgetTest, AlwaysOnTop) {
   WidgetAutoclosePtr widget(CreateTopLevelNativeWidget());
   EXPECT_FALSE(widget->IsAlwaysOnTop());
   widget->SetAlwaysOnTop(true);
   EXPECT_TRUE(widget->IsAlwaysOnTop());
   widget->SetAlwaysOnTop(false);
   EXPECT_FALSE(widget->IsAlwaysOnTop());
@@ skipping 36 matching lines @@
   EXPECT_EQ(scale_factor, view->last_scale_factor());
 
   // Changes should be propagated.
   scale_factor *= 2.0f;
   widget->GetLayer()->OnDeviceScaleFactorChanged(scale_factor);
   EXPECT_EQ(scale_factor, view->last_scale_factor());
 }
 
 }  // namespace test
 }  // namespace views