Commit a54e6c0c authored by ananta's avatar ananta Committed by Commit bot

Fix crash in the HWNDMessageHandler::HandleMouseInputForCaption function.

Crash occurs because we are referencing a member variable left_button_down_on_caption_ after the DefWindowProc call for WM_NCLBUTTONDOWN, which could destroy the window and in
turn the HWNDMessageHandler instance.

Fix is to reset the left_button_down_on_caption_ member before the DefWindowProc call.

BUG=581341
TEST=Covered by views_unittest WidgetTest.DestroyInSysCommandNCLButtonDownOnCaption

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

Cr-Commit-Position: refs/heads/master@{#371892}
parent 03ade5bf
......@@ -3213,7 +3213,8 @@ TEST_F(WidgetTest, CharMessagesAsKeyboardMessagesDoesNotCrash) {
class SubclassWindowHelper {
public:
explicit SubclassWindowHelper(HWND window)
: window_(window) {
: window_(window),
message_to_destroy_on_(0) {
EXPECT_EQ(instance_, nullptr);
instance_ = this;
EXPECT_TRUE(Subclass());
......@@ -3233,6 +3234,10 @@ class SubclassWindowHelper {
messages_.clear();
}
void set_message_to_destroy_on(unsigned int message) {
message_to_destroy_on_ = message;
}
private:
bool Subclass() {
old_proc_ = reinterpret_cast<WNDPROC>(
......@@ -3258,14 +3263,20 @@ class SubclassWindowHelper {
// 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 (message == instance_->message_to_destroy_on_) {
instance_->Unsubclass();
::DestroyWindow(window);
}
return ret;
}
WNDPROC old_proc_;
HWND window_;
static SubclassWindowHelper* instance_;
std::set<unsigned int> messages_;
unsigned int message_to_destroy_on_;
DISALLOW_COPY_AND_ASSIGN(SubclassWindowHelper);
};
......@@ -3347,6 +3358,38 @@ TEST_F(WidgetTest, SysCommandMoveOnNCLButtonDownOnCaptionAndMoveTest) {
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_message_to_destroy_on(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.
......
......@@ -2557,12 +2557,15 @@ bool HWNDMessageHandler::HandleMouseInputForCaption(unsigned int message,
// so we need to call it inside a ScopedRedrawLock. This may cause
// other negative side-effects
// (ex/ stifling non-client mouse releases).
// We may be deleted in the context of DefWindowProc. Don't refer to
// any member variables after the DefWindowProc call.
left_button_down_on_caption_ = false;
if (delegate_->IsUsingCustomFrame()) {
DefWindowProcWithRedrawLock(WM_NCLBUTTONDOWN, HTCAPTION, l_param);
} else {
DefWindowProc(hwnd(), WM_NCLBUTTONDOWN, HTCAPTION, l_param);
}
left_button_down_on_caption_ = false;
}
break;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment