From 2db98715cc393eda65256b6fb55a46d4e390574b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ivan=20Sto=C5=A1i=C4=87?= <ivan100sic@gmail.com>
Date: Thu, 30 Apr 2020 11:05:18 +0200
Subject: [PATCH] "Unhooked" the Shortcut guide module from the PT event
 dispatcher (#2318)

* "Unhooked" the Shortcut guide module from the PT event dispatcher

* Fixup: warning/undefined behavior/terrible bug

* SetWindowsHookEx and UnhookWindowsHookEx now fail silently

* Updated a comment in shortcut_guide.h

* Renamed a method, added an error message
---
 src/modules/shortcut_guide/shortcut_guide.cpp | 69 +++++++++++++++----
 src/modules/shortcut_guide/shortcut_guide.h   |  6 ++
 2 files changed, 60 insertions(+), 15 deletions(-)

diff --git a/src/modules/shortcut_guide/shortcut_guide.cpp b/src/modules/shortcut_guide/shortcut_guide.cpp
index 0c70a7e5ae..708d4f43c5 100644
--- a/src/modules/shortcut_guide/shortcut_guide.cpp
+++ b/src/modules/shortcut_guide/shortcut_guide.cpp
@@ -11,6 +11,24 @@ extern "C" IMAGE_DOS_HEADER __ImageBase;
 
 OverlayWindow* instance = nullptr;
 
+namespace
+{
+    LRESULT CALLBACK LowLevelKeyboardProc(int nCode, WPARAM wParam, LPARAM lParam)
+    {
+        LowlevelKeyboardEvent event;
+        if (nCode == HC_ACTION)
+        {
+            event.lParam = reinterpret_cast<KBDLLHOOKSTRUCT*>(lParam);
+            event.wParam = wParam;
+            if (instance->signal_event(&event) != 0)
+            {
+                return 1;
+            }
+        }
+        return CallNextHookEx(NULL, nCode, wParam, lParam);
+    }
+}
+
 OverlayWindow::OverlayWindow()
 {
     app_name = GET_RESOURCE_STRING(IDS_SHORTCUT_GUIDE);
@@ -24,8 +42,7 @@ const wchar_t* OverlayWindow::get_name()
 
 const wchar_t** OverlayWindow::get_events()
 {
-    static const wchar_t* events[2] = { ll_keyboard, 0 };
-    return events;
+    return nullptr;
 }
 
 bool OverlayWindow::get_config(wchar_t* buffer, int* buffer_size)
@@ -117,6 +134,11 @@ void OverlayWindow::enable()
         winkey_popup->set_theme(theme.value);
         target_state = std::make_unique<TargetState>(pressTime.value);
         winkey_popup->initialize();
+        hook_handle = SetWindowsHookEx(WH_KEYBOARD_LL, LowLevelKeyboardProc, GetModuleHandle(NULL), NULL);
+        if (!hook_handle)
+        {
+            MessageBoxW(NULL, L"Cannot install keyboard listener.", L"PowerToys - Shortcut Guide", MB_OK | MB_ICONERROR);
+        }
     }
     _enabled = true;
 }
@@ -134,6 +156,14 @@ void OverlayWindow::disable(bool trace_event)
         target_state->exit();
         target_state.reset();
         winkey_popup.reset();
+        if (hook_handle)
+        {
+            bool success = UnhookWindowsHookEx(hook_handle);
+            if (success)
+            {
+                hook_handle = nullptr;
+            }
+        }
     }
 }
 
@@ -149,22 +179,31 @@ bool OverlayWindow::is_enabled()
 
 intptr_t OverlayWindow::signal_event(const wchar_t* name, intptr_t data)
 {
-    if (_enabled && wcscmp(name, ll_keyboard) == 0)
-    {
-        auto& event = *(reinterpret_cast<LowlevelKeyboardEvent*>(data));
-        if (event.wParam == WM_KEYDOWN ||
-            event.wParam == WM_SYSKEYDOWN ||
-            event.wParam == WM_KEYUP ||
-            event.wParam == WM_SYSKEYUP)
-        {
-            bool supress = target_state->signal_event(event.lParam->vkCode,
-                                                      event.wParam == WM_KEYDOWN || event.wParam == WM_SYSKEYDOWN);
-            return supress ? 1 : 0;
-        }
-    }
     return 0;
 }
 
+intptr_t OverlayWindow::signal_event(LowlevelKeyboardEvent* event)
+{
+    if (!_enabled)
+    {
+        return 0;
+    }
+
+    if (event->wParam == WM_KEYDOWN ||
+        event->wParam == WM_SYSKEYDOWN ||
+        event->wParam == WM_KEYUP ||
+        event->wParam == WM_SYSKEYUP)
+    {
+        bool suppress = target_state->signal_event(event->lParam->vkCode,
+            event->wParam == WM_KEYDOWN || event->wParam == WM_SYSKEYDOWN);
+        return suppress ? 1 : 0;
+    }
+    else
+    {
+        return 0;
+    }
+}
+
 void OverlayWindow::on_held()
 {
     auto filter = get_shortcutguide_filtered_window();
diff --git a/src/modules/shortcut_guide/shortcut_guide.h b/src/modules/shortcut_guide/shortcut_guide.h
index 61d0691e5b..c9f69679dc 100644
--- a/src/modules/shortcut_guide/shortcut_guide.h
+++ b/src/modules/shortcut_guide/shortcut_guide.h
@@ -22,6 +22,8 @@ public:
     virtual void enable() override;
     virtual void disable() override;
     virtual bool is_enabled() override;
+
+    // PowerToys interface method, not used
     virtual intptr_t signal_event(const wchar_t* name, intptr_t data) override;
 
     virtual void register_system_menu_helper(PowertoySystemMenuIface* helper) override {}
@@ -32,6 +34,9 @@ public:
     void quick_hide();
     void was_hidden();
 
+    // Method called from LowLevelKeyboardProc
+    intptr_t signal_event(LowlevelKeyboardEvent* event);
+
     virtual void destroy() override;
 
 private:
@@ -39,6 +44,7 @@ private:
     std::unique_ptr<TargetState> target_state;
     std::unique_ptr<D2DOverlayWindow> winkey_popup;
     bool _enabled = false;
+    HHOOK hook_handle;
 
     void init_settings();
     void disable(bool trace_event);