API proposal for chrome.app.window to intercept all keys.

ui/views/widget/desktop_aura/desktop_keyboard_capture_win.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 "ui/views/widget/desktop_aura/desktop_keyboard_capture_win.h"
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/logging.h"
+#include "base/synchronization/lock.h"
+#include "base/task_runner.h"
+
+namespace views {
+
+base::Lock DesktopKeyboardCaptureWin::registration_lock_;
+
+base::ScopedPtrHashMap<HWND,
+                       DesktopKeyboardCaptureWin::KeyboardInterceptRegistration>
+    DesktopKeyboardCaptureWin::registrations_;
+
+class VIEWS_EXPORT DesktopKeyboardCaptureWin::KeyboardInterceptRegistration {

[sky, 2014/07/29 23:07:25]

You don't need VIEWS_EXPORT in a .cc file.

[Sriram, 2014/07/31 00:21:13]

Done.

+ public:
+  KeyboardInterceptRegistration() : hook_handle_(NULL) {}
+
+  ~KeyboardInterceptRegistration() {
+    if (hook_handle_ != NULL) {

[sky, 2014/07/29 23:07:25]

no {}

[Sriram, 2014/07/31 00:21:13]

Done.

+      Unhook();
+    }
+  }
+
+  bool Hook(HOOKPROC callback_function) {
+    DCHECK(hook_handle_ == NULL) << "Keyboard hook already registered";
+    hook_handle_ = SetWindowsHookEx(WH_KEYBOARD_LL, callback_function, NULL, 0);

[cpu_(ooo_6.6-7.5), 2014/07/31 00:14:48]

check that this is a thread with a UI style message pump.

[Sriram, 2014/07/31 00:55:42]

I am not sure as how I could check this. Could you please give me pointers?
Thanks!

On 2014/07/31 00:14:48, cpu wrote:

[cpu_(ooo_6.6-7.5), 2014/07/31 21:45:21]

DCHECK(base::MessageLoopForUI::IsCurrent())

+    if (hook_handle_ == NULL) {
+      DLOG(ERROR) << "Error calling SetWindowsHookEx() - GLE = "

[sky, 2014/07/29 23:07:25]

DVLOG

[Sriram, 2014/07/31 00:21:13]

Done.

+                  << GetLastError();
+      return false;
+    }
+    return true;
+  }
+
+  bool Unhook() {
+    DCHECK(hook_handle_ != NULL) << "Unhook called without registring hooks";
+    BOOL result = UnhookWindowsHookEx(hook_handle_);
+    if (!result) {
+      DLOG(ERROR) << "Error calling UnhookWindowsHookEx() - GLE = "
+                  << GetLastError();
+      return false;
+    }
+    hook_handle_ = NULL;
+    return true;
+  }
+
+ private:
+  // Hook returned when it was installed
+  HHOOK hook_handle_;
+
+  DISALLOW_COPY_AND_ASSIGN(KeyboardInterceptRegistration);
+};
+
+// static
+void DesktopKeyboardCaptureWin::Capture(HWND window_handle,
+                                        base::TaskRunner* task_runner) {
+  if (AddToRegistrationList(window_handle)) {
+    // Task to register from thread which should be active for the duration
+    // of the registration.
+    task_runner->PostTask(

[sky, 2014/07/29 23:07:25]

Why do you PostTask here?

[Sriram, 2014/07/31 00:21:13]

Hook callbacks happen in the thread that calls SetWindowsHookEx() and so posting
to the thread that is supposed to be active for the lifetime (i.e. the UI
thread).

On 2014/07/29 23:07:25, sky wrote:

[sky, 2014/07/31 15:21:22]

I'm still confused by your use of threads. At the current time there is only
ever one thread using views. That may change, but currently its the case.

[Sriram, 2014/07/31 19:40:05]

Done.

+        FROM_HERE,
+        base::Bind(&DesktopKeyboardCaptureWin::RegisterExistingHandle,
+                   window_handle));
+  }
+}
+
+// static
+void DesktopKeyboardCaptureWin::RegisterExistingHandle(HWND window_handle) {
+  base::AutoLock lock(registration_lock_);

[sky, 2014/07/29 23:07:25]

Why are you using a lock?

[Sriram, 2014/07/31 00:21:13]

The registration is accessed from UI thread (as part of handling
callbacks/registration) and any thread on which Release() could get called from.
So using a lock to protect it.

On 2014/07/29 23:07:25, sky wrote:

+  KeyboardInterceptRegistration* registration =
+      registrations_.get(window_handle);
+  if (registration != NULL) {
+    VLOG(1) << "Registered keyboard hook for handle = " << window_handle;
+    registration->Hook(KeyboardHook);
+  }
+}
+
+// static
+void DesktopKeyboardCaptureWin::Release(HWND window_handle) {
+  base::AutoLock lock(registration_lock_);
+  registrations_.erase(window_handle);
+  VLOG(1) << "Keyboard hook unregistered for handle = " << window_handle;
+}
+
+// static
+bool DesktopKeyboardCaptureWin::AddToRegistrationList(HWND handle) {
+  base::AutoLock lock(registration_lock_);
+  if (registrations_.contains(handle)) {

[sky, 2014/07/29 23:07:25]

no {}

[Sriram, 2014/07/31 00:21:14]

Done.

+    return false;
+  }
+  scoped_ptr<KeyboardInterceptRegistration> registration(
+      new KeyboardInterceptRegistration());
+
+  return registrations_.add(handle, registration.Pass()).second;
+}
+
+// static
+bool DesktopKeyboardCaptureWin::IsRegistered(HWND handle) {
+  base::AutoLock lock(registration_lock_);
+  return registrations_.contains(handle);
+}
+
+// static
+bool DesktopKeyboardCaptureWin::IsKeyUp(WPARAM w_param) {

[sky, 2014/07/29 23:07:25]

Move functions like this that don't need any state from
DesktopKeyboardCaptureWin into an anonymous namespace.

[Sriram, 2014/07/31 00:21:13]

Done.

+  return (w_param == WM_KEYUP) || (w_param == WM_SYSKEYUP);
+}
+
+// static
+bool DesktopKeyboardCaptureWin::IsBitSet(ULONG value, ULONG mask) {
+  return ((value & mask) != 0);
+}
+
+// static
+LPARAM DesktopKeyboardCaptureWin::GetLParamFromHookStruct(
+    WPARAM w_param,
+    KBDLLHOOKSTRUCT hook_struct) {
+  ULONG key_state = 0;
+  // There is no way to get repeat count so always set it to 1.
+  key_state = 1;
+
+  // Scan code
+  key_state |= (hook_struct.scanCode & 0xFF) << 16;
+
+  // Extended key when the event is received as part window event and so skip it

[cpu_(ooo_6.6-7.5), 2014/07/31 00:14:48]

comments need periods at the end.

[Sriram, 2014/07/31 00:49:59]

Done.

+
+  // Context code
+  key_state |= IsBitSet(hook_struct.flags, LLKHF_ALTDOWN) << 29;
+
+  // Previous key state - set to 1 for KEYUP events
+  key_state |= IsKeyUp(w_param) << 30;
+
+  // Transition state
+  key_state |= IsBitSet(hook_struct.flags, LLKHF_UP) << 31;
+  return static_cast<LPARAM>(key_state);
+}
+
+// static
+DWORD DesktopKeyboardCaptureWin::RemoveLocationOnKeycode(DWORD vk_code) {
+  // Virtual keycode from low level hook include location while window messages
+  // does not. So convert them to be without location.
+  switch (vk_code) {
+    case VK_LSHIFT:
+    case VK_RSHIFT:
+      return VK_SHIFT;
+    case VK_LCONTROL:
+    case VK_RCONTROL:
+      return VK_CONTROL;
+    case VK_LMENU:
+    case VK_RMENU:
+      return VK_MENU;
+  }
+  return vk_code;
+}
+
+// static
+bool DesktopKeyboardCaptureWin::UpdateThreadKeyboardState() {
+  const int kKeyboardStateLength = 256;

[cpu_(ooo_6.6-7.5), 2014/07/31 00:14:48]

what is this function trying to do?

A full fledged comment about the general operation would be nice as well.

[Sriram, 2014/07/31 00:55:42]

Done.

+  BYTE keyboard_state[kKeyboardStateLength];
+  if (!GetKeyboardState(keyboard_state)) {
+    PLOG(ERROR) << "Error getting keyboard state";
+    return false;
+  }
+
+  int keys_to_update[] = {VK_SHIFT, VK_CONTROL, VK_MENU};
+  for (int index = 0; index < _countof(keys_to_update); index++) {

[cpu_(ooo_6.6-7.5), 2014/07/31 00:14:48]

dont use _countof()

[Sriram, 2014/07/31 00:50:00]

Done.

+    int key = keys_to_update[index];
+    SHORT key_state = GetAsyncKeyState(key);
+    keyboard_state[key] = (IsBitSet(key_state, 0x8000) ? 0x80 : 0) |
+                          (IsBitSet(key_state, 0x1) ? 1 : 0);
+  }
+
+  if (!SetKeyboardState(keyboard_state)) {
+    PLOG(ERROR) << "Error setting keyboard state";
+    return false;
+  }
+
+  return true;
+}
+
+// static
+LRESULT CALLBACK DesktopKeyboardCaptureWin::KeyboardHook(int code,
+                                                         WPARAM w_param,
+                                                         LPARAM l_param) {
+  HWND current_active_window = GetActiveWindow();

[sky, 2014/07/29 23:07:25]

I think we should uninstall the hook when we don't have an active window any
longer. Less errors then and less calling into chrome from other apps.

[Sriram, 2014/07/31 00:21:13]

We do uninstall the hook as soon as the window losses focus. This is to handle
race when the keyboard event is already in flight.

On 2014/07/29 23:07:25, sky wrote:

+  if ((code >= 0) && IsRegistered(current_active_window)) {
+    UpdateThreadKeyboardState();
+
+    KBDLLHOOKSTRUCT hook_struct = *reinterpret_cast<KBDLLHOOKSTRUCT*>(l_param);
+    PostMessage(current_active_window,
+                w_param,
+                RemoveLocationOnKeycode(hook_struct.vkCode),
+                GetLParamFromHookStruct(w_param, hook_struct));
+
+    return 1;
+  }
+
+  return CallNextHookEx(NULL, code, w_param, l_param);
+}
+
+}  // namespace views