From 005229b167c1a9e415fc814520bf1d650adebb22 Mon Sep 17 00:00:00 2001 From: ananta Date: Wed, 28 Jan 2015 13:38:06 -0800 Subject: [PATCH] Fix a crash which occurs due to a CHECK firing in the HWNDMessageHandler::OnKeyEvent code path on Windows. This crash occurs when the LegacyRenderWidgetHostHWND instance sends WM_CHAR/WM_SYSCHAR messages via the WindowEventTarget::HandleKeyboardMessage function. The HWNDMessageHandler class handles these messages via the OnImeMessages handler in the normal case. The CHECK fires in the KeyEvent ctor where we validate that the KeyEvent instance has a valid event type. In this case the WM_CHAR/WM_SYSCHAR messages are processed via the OnKeyEvent handler which leads to the problem. Fix is to add a check for WM_CHAR/WM_SYSCHAR messages in the HWNDMessageHandler::HandleKeyboardMessage function and call the OnImeMessages handler there. The alternative would be to add a new method to the WindowEventTarget interface and change the LegacyRenderWidgetHostHWND class to handle these messages separately and call the new method. I think that change would be a touch big for this case. I also added WM_SYSDEADCHAR to the list of messages processed by the EventTypeFromNative function. We set the type as ET_KEY_RELEASED on the same lines as WM_DEADCHAR. BUG=451797 TEST=Covered by views_unittest WidgetTest.CharMessagesAsKeyboardMessagesDoesNotCrash Review URL: https://codereview.chromium.org/881163002 Cr-Commit-Position: refs/heads/master@{#313591} --- ui/events/win/events_win.cc | 4 ++++ ui/views/widget/widget_unittest.cc | 31 +++++++++++++++++++++++++++++++ ui/views/win/hwnd_message_handler.cc | 6 +++++- 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/ui/events/win/events_win.cc b/ui/events/win/events_win.cc index 58b60500ec51..6960f46e79af 100644 --- a/ui/events/win/events_win.cc +++ b/ui/events/win/events_win.cc @@ -156,6 +156,10 @@ EventType EventTypeFromNative(const base::NativeEvent& native_event) { // sequences. case WM_DEADCHAR: case WM_KEYUP: + // The WM_SYSDEADCHAR message is posted to a window with keyboard focus + // when the WM_SYSKEYDOWN message is translated by the TranslateMessage + // function. It specifies the character code of the system dead key. + case WM_SYSDEADCHAR: case WM_SYSKEYUP: return ET_KEY_RELEASED; case WM_LBUTTONDBLCLK: diff --git a/ui/views/widget/widget_unittest.cc b/ui/views/widget/widget_unittest.cc index 5556db5367fe..4ad12053519e 100644 --- a/ui/views/widget/widget_unittest.cc +++ b/ui/views/widget/widget_unittest.cc @@ -34,6 +34,10 @@ #include "ui/views/window/native_frame_view.h" #if defined(OS_WIN) +#include "ui/aura/window.h" +#include "ui/aura/window_tree_host.h" +#include "ui/base/view_prop.h" +#include "ui/base/win/window_event_target.h" #include "ui/views/win/hwnd_util.h" #endif @@ -3384,5 +3388,32 @@ TEST_F(WidgetTest, NonClientWindowValidAfterInit) { widget->CloseNow(); } +#if defined(OS_WIN) +// This test validates that sending WM_CHAR/WM_SYSCHAR/WM_SYSDEADCHAR +// messages via the WindowEventTarget interface implemented by the +// HWNDMessageHandler class does not cause a crash due to an unprocessed +// event +TEST_F(WidgetTest, CharMessagesAsKeyboardMessagesDoesNotCrash) { + 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.Show(); + + ui::WindowEventTarget* target = + reinterpret_cast(ui::ViewProp::GetValue( + widget.GetNativeWindow()->GetHost()->GetAcceleratedWidget(), + ui::WindowEventTarget::kWin32InputEventTarget)); + EXPECT_NE(nullptr, target); + bool handled = false; + target->HandleKeyboardMessage(WM_CHAR, 0, 0, &handled); + target->HandleKeyboardMessage(WM_SYSCHAR, 0, 0, &handled); + target->HandleKeyboardMessage(WM_SYSDEADCHAR, 0, 0, &handled); + widget.CloseNow(); +} +#endif + } // namespace test } // namespace views diff --git a/ui/views/win/hwnd_message_handler.cc b/ui/views/win/hwnd_message_handler.cc index 8dca09bf7258..995600689f5d 100644 --- a/ui/views/win/hwnd_message_handler.cc +++ b/ui/views/win/hwnd_message_handler.cc @@ -1009,7 +1009,11 @@ LRESULT HWNDMessageHandler::HandleKeyboardMessage(unsigned int message, LPARAM l_param, bool* handled) { base::WeakPtr ref(weak_factory_.GetWeakPtr()); - LRESULT ret = OnKeyEvent(message, w_param, l_param); + LRESULT ret = 0; + if ((message == WM_CHAR) || (message == WM_SYSCHAR)) + ret = OnImeMessages(message, w_param, l_param); + else + ret = OnKeyEvent(message, w_param, l_param); *handled = IsMsgHandled(); return ret; } -- 2.11.4.GIT