Combine the WM_CHAR with WM_KEY* for key event flow.

win8/metro_driver/metro_driver_win7.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 "stdafx.h"
 #include <corewindow.h>
 #include <shobjidl.h>
 
 #include "base/logging.h"
 #include "base/profiler/scoped_tracker.h"
@@ skipping 353 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(MouseEvent);
 };
 
 // This class implements the winrt interfaces needed to support keyboard
 // character and system character messages.
 class KeyEvent : public mswr::RuntimeClass<
     winui::Core::IKeyEventArgs,
     winui::Core::ICharacterReceivedEventArgs,
     winui::Core::IAcceleratorKeyEventArgs> {
  public:
-  KeyEvent(const MSG& msg)
-      : msg_(msg) {}
+  KeyEvent(const MSG& msg, bool was_key)
+      : msg_(msg), was_key_(was_key) {}
 
   // IKeyEventArgs implementation.
   HRESULT STDMETHODCALLTYPE
   get_VirtualKey(winsys::VirtualKey* virtual_key) override {
     *virtual_key = static_cast<winsys::VirtualKey>(msg_.wParam);
     return S_OK;
   }
 
   HRESULT STDMETHODCALLTYPE
   get_KeyStatus(winui::Core::CorePhysicalKeyStatus* key_status) override {
     // As per msdn documentation for the keyboard messages.
     key_status->RepeatCount = msg_.lParam & 0x0000FFFF;
     key_status->ScanCode = (msg_.lParam >> 16) & 0x00FF;
     key_status->IsExtendedKey = (msg_.lParam & (1 << 24));
     key_status->IsMenuKeyDown = (msg_.lParam & (1 << 29));
-    key_status->WasKeyDown = (msg_.lParam & (1 << 30));
     key_status->IsKeyReleased = (msg_.lParam & (1 << 31));
+    key_status->WasKeyDown = was_key_;
     return S_OK;
   }
 
   // ICharacterReceivedEventArgs implementation.
   HRESULT STDMETHODCALLTYPE get_KeyCode(uint32* key_code) override {
     *key_code = msg_.wParam;
     return S_OK;
   }
 
   // IAcceleratorKeyEventArgs implementation.
   HRESULT STDMETHODCALLTYPE
   get_EventType(winui::Core::CoreAcceleratorKeyEventType* event_type) override {
     if (msg_.message == WM_SYSKEYDOWN) {
       *event_type = winui::Core::CoreAcceleratorKeyEventType_SystemKeyDown;
     } else if (msg_.message == WM_SYSKEYUP) {
       *event_type = winui::Core::CoreAcceleratorKeyEventType_SystemKeyUp;
     } else if (msg_.message == WM_SYSCHAR) {
       *event_type = winui::Core::CoreAcceleratorKeyEventType_SystemCharacter;
     }
     return S_OK;
   }
 
  private:
   MSG msg_;
+  bool was_key_;
 };
 
 // The following classes are the emulation of the WinRT system as exposed
 // to metro applications. There is one application (ICoreApplication) which
 // contains a series of Views (ICoreApplicationView) each one of them
 // containing a CoreWindow which represents a surface that can drawn to
 // and that receives events.
 //
 // Here is the general dependency hierachy in terms of interfaces:
 //
@@ skipping 28 matching lines @@
 
   HRESULT STDMETHODCALLTYPE
   ProcessEvents(winui::Core::CoreProcessEventsOption options) override {
     // We don't support the other message pump modes. So we basically enter a
     // traditional message loop that we only exit a teardown.
     if (options != winui::Core::CoreProcessEventsOption_ProcessUntilQuit)
       return E_FAIL;
 
     MSG msg = {0};
     while((::GetMessage(&msg, NULL, 0, 0) != 0) && g_window_count > 0) {
+      ::TranslateMessage(&msg);
       ProcessInputMessage(msg);
-      ::TranslateMessage(&msg);
       ::DispatchMessage(&msg);
     }
     // TODO(cpu): figure what to do with msg.WParam which we would normally
     // return here.
     return S_OK;
   }
 
   HRESULT STDMETHODCALLTYPE
   RunAsync(winui::Core::CoreDispatcherPriority priority,
            winui::Core::IDispatchedHandler* agileCallback,
@@ skipping 38 matching lines @@
       } else if ((msg.message >= WM_MOUSEFIRST) &&
                   (msg.message <= WM_MOUSELAST)) {
         ret = input_handler_->HandleMouseMessage(msg);
       }
     }
     return ret;
   }
 
   bool HandleSystemKeys(const MSG& msg) {
     mswr::ComPtr<winui::Core::IAcceleratorKeyEventArgs> event_args;
-    event_args = mswr::Make<KeyEvent>(msg);
+    event_args = mswr::Make<KeyEvent>(msg, true);
     accelerator_key_event_handler_->Invoke(this, event_args.Get());
     return true;
   }
 
   InputHandler* input_handler_;
   AcceleratorKeyEventHandler* accelerator_key_event_handler_;
 };
 
 class CoreWindowEmulation
     : public mswr::RuntimeClass<
@@ skipping 370 matching lines @@
 
   HRESULT STDMETHODCALLTYPE put_MessageHandled(boolean value) override {
     return S_OK;
   }
 
   // InputHandler
   bool HandleKeyboardMessage(const MSG& msg) override {
     switch (msg.message) {
       case WM_KEYDOWN:
       case WM_KEYUP: {
+        // Peek & remove the following messages in the message queue.

[robliao, 2015/08/14 16:42:34]

Similarly, this is clear from the code below. Instead, address why we're doing
this and why it's safe it ignore WM_DEADCHAR.

[Shu Chen, 2015/08/17 04:09:21]

Done.
Chrome is not interested in WM_*DEADCHAR messages and never process them.
Please refer to yukawa@'s comment in patchset #1.

+        //  - WM_CHAR (0x0102)
+        //  - WM_DEADCHAR (0x0103)
+        // And only process WM_CHAR message in browser.
+        MSG char_msg;
+        while (::PeekMessage(&char_msg, msg.hwnd, WM_CHAR, WM_DEADCHAR,
+                             PM_REMOVE)) {
+          if (char_msg.message == WM_DEADCHAR)
+            continue;
+          mswr::ComPtr<winui::Core::ICharacterReceivedEventArgs> char_args;
+          char_args = mswr::Make<KeyEvent>(char_msg, true);
+          character_received_handler_->Invoke(this, char_args.Get());
+        }
         mswr::ComPtr<winui::Core::IKeyEventArgs> event_args;
-        event_args = mswr::Make<KeyEvent>(msg);
+        event_args = mswr::Make<KeyEvent>(msg, true);
         KeyEventHandler* handler = NULL;
         if (msg.message == WM_KEYDOWN) {
           handler = key_down_handler_;
         } else {
           handler = key_up_handler_;
         }
         handler->Invoke(this, event_args.Get());
         break;
       }
 
-      case WM_CHAR:
-      case WM_DEADCHAR:
-      case WM_UNICHAR: {
+      case WM_CHAR: {
         mswr::ComPtr<winui::Core::ICharacterReceivedEventArgs> event_args;

[robliao, 2015/08/14 16:42:34]

Have we confirmed no one sends WM_UNICHAR to us?

[Shu Chen, 2015/08/17 04:09:21]

Some apps may send WM_UNICHAR to us. However, Windows API framework handles it
very well and Chrome doesn't need to handle it.
Please refer to
https://msdn.microsoft.com/en-us/library/windows/desktop/ms646288(v=vs.85).aspx,
the DefWindowProc() call will transfer the WM_UNICHAR message into WM_CHAR
messages.

In facts, the existing code below: "character_received_handler_->Invoke()" on
WM_UNICHAR has potential issues.
Please refer to
https://code.google.com/p/chromium/codesearch#chromium/src/ui/aura/remote_win...,
the uint32 char is casted to base::char16, which is wrong.

-        event_args = mswr::Make<KeyEvent>(msg);
+        event_args = mswr::Make<KeyEvent>(msg, false);
         character_received_handler_->Invoke(this, event_args.Get());
         break;
       }
 
       default:
         return false;
     }
     return true;
   }
 
@@ skipping 275 matching lines @@
 };
 
 
 mswr::ComPtr<winapp::Core::ICoreApplication> InitWindows7() {
   HRESULT hr = ::CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
   if (FAILED(hr))
     CHECK(false);
   return mswr::Make<CoreApplicationWin7Emulation>();
 }