clean up the hook management code

src/lib/platform/MSWindowsDesks.cpp

@@ -547,7 +547,7 @@ MSWindowsDesks::deskEnter(Desk* desk)
     AttachThreadInput(thatThread, thisThread, TRUE);
     SetForegroundWindow(desk->m_foregroundWindow);
     AttachThreadInput(thatThread, thisThread, FALSE);
-    EnableWindow(desk->m_window, desk->m_lowLevel ? FALSE : TRUE);
+    EnableWindow(desk->m_window, FALSE);
     desk->m_foregroundWindow = NULL;
 }
 
@@ -560,35 +560,16 @@ MSWindowsDesks::deskLeave(Desk* desk, HKL keyLayout)
         // layout we choose rather than the keyboard layout of the last
         // active window.
         int x, y, w, h;
-        if (desk->m_lowLevel) {
         // with a low level hook the cursor will never budge so
         // just a 1x1 window is sufficient.
         x = m_xCenter;
         y = m_yCenter;
         w = 1;
         h = 1;
-        }
-        else {
-            // with regular hooks the cursor will jitter as it's moved
-            // by the user then back to the center by us.  to be sure
-            // we never lose it, cover all the monitors with the window.
-            x = m_x;
-            y = m_y;
-            w = m_w;
-            h = m_h;
-        }
         SetWindowPos(desk->m_window, HWND_TOP, x, y, w, h,
                             SWP_NOACTIVATE | SWP_SHOWWINDOW);
 
-        // if not using low-level hooks we have to also activate the
+        // since we're using low-level hooks, disable the foreground window
-        // window to ensure we don't lose keyboard focus.
-        // FIXME -- see if this can be avoided.  if so then always
-        // disable the window (see handling of BARRIER_MSG_SWITCH).
-        if (!desk->m_lowLevel) {
-            SetActiveWindow(desk->m_window);
-        }
-
-        // if using low-level hooks then disable the foreground window
         // so it can't mess up any of our keyboard events.  the console
         // program, for example, will cause characters to be reported as
         // unshifted, regardless of the shift key state.  interestingly
@@ -596,7 +577,6 @@ MSWindowsDesks::deskLeave(Desk* desk, HKL keyLayout)
         //
         // note that we must enable the window to activate it and we
         // need to disable the window on deskEnter.
-        else {
         desk->m_foregroundWindow = getForegroundWindow();
         if (desk->m_foregroundWindow != NULL) {
             EnableWindow(desk->m_window, TRUE);
@@ -609,7 +589,6 @@ MSWindowsDesks::deskLeave(Desk* desk, HKL keyLayout)
             SetForegroundWindow(desk->m_window);
             AttachThreadInput(thatThread, thisThread, FALSE);
         }
-        }
 
         // switch to requested keyboard layout
         ActivateKeyboardLayout(keyLayout, 0);
@@ -683,25 +662,14 @@ MSWindowsDesks::deskThread(void* vdesk)
                 auto list = immune_keys_list();
                 LOG((CLOG_DEBUG "Found %u immune keys", list.size()));
                 //m_setImmuneKeys(list.data(), list.size());
-                switch (MSWindowsHook::install()) {
+                if (!MSWindowsHook::install()) {
-                case kHOOK_FAILED:
                     // we won't work on this desk
-                    desk->m_lowLevel = false;
+                    LOG((CLOG_DEBUG "Cannot hook on this desk"));
-                    break;
-
-                case kHOOK_OKAY:
-                    desk->m_lowLevel = false;
-                    break;
-
-                case kHOOK_OKAY_LL:
-                    desk->m_lowLevel = true;
-                    break;
                 }
-
                 // a window on the primary screen with low-level hooks
                 // should never activate.
                 if (desk->m_window)
-                    EnableWindow(desk->m_window, desk->m_lowLevel ? FALSE : TRUE);
+                    EnableWindow(desk->m_window, FALSE);
             }
             break;
 

src/lib/platform/MSWindowsHook.cpp

@@ -18,6 +18,7 @@
  */
 
 #include "platform/MSWindowsHook.h"
+#include "platform/MSWindowsHookResource.h"
 #include "barrier/protocol_types.h"
 #include "barrier/XScreen.h"
 #include "base/Log.h"
@@ -30,14 +31,11 @@
  //
 #define NO_GRAB_KEYBOARD 0
 
-static const char* g_name = "synwinhk";
+static const DWORD      g_threadID = GetCurrentThreadId();
 
-static DWORD            g_processID = 0;
+static WindowsHookResource  g_hkMessage;
-static DWORD            g_threadID = 0;
+static WindowsHookResource  g_hkKeyboard;
-static HHOOK            g_getMessage = NULL;
+static WindowsHookResource  g_hkMouse;
-static HHOOK            g_keyboardLL = NULL;
-static HHOOK            g_mouseLL = NULL;
-static bool                g_screenSaver = false;
 static EHookMode        g_mode = kHOOK_DISABLE;
 static UInt32            g_zoneSides = 0;
 static SInt32            g_zoneSize = 0;
@@ -50,90 +48,7 @@ static WPARAM            g_deadRelease = 0;
 static LPARAM            g_deadLParam = 0;
 static BYTE                g_deadKeyState[256] = { 0 };
 static BYTE                g_keyState[256] = { 0 };
-static DWORD            g_hookThread = 0;
 static bool                g_fakeServerInput = false;
-static BOOL                g_isPrimary = TRUE;
-
-MSWindowsHook::MSWindowsHook()
-{
-}
-
-MSWindowsHook::~MSWindowsHook()
-{
-    cleanup();
-
-    if (g_processID == GetCurrentProcessId()) {
-        uninstall();
-        uninstallScreenSaver();
-        g_processID = 0;
-    }
-}
-
-void
-MSWindowsHook::loadLibrary()
-{
-    if (g_processID == 0) {
-        g_processID = GetCurrentProcessId();
-    }
-    if (init(GetCurrentThreadId()) == 0) {
-        LOG((CLOG_ERR "failed to init hooks handler"));
-        throw XScreenOpenFailure();
-    }
-}
-
-int
-MSWindowsHook::init(DWORD threadID)
-{
-    // try to open process that last called init() to see if it's
-    // still running or if it died without cleaning up.
-    if (g_processID != 0 && g_processID != GetCurrentProcessId()) {
-        HANDLE process = OpenProcess(STANDARD_RIGHTS_REQUIRED,
-            FALSE, g_processID);
-        if (process != NULL) {
-            // old process (probably) still exists so refuse to
-            // reinitialize this DLL (and thus steal it from the
-            // old process).
-            int result = CloseHandle(process);
-            if (result == false) {
-                return 0;
-            }
-        }
-
-        // clean up after old process.  the system should've already
-        // removed the hooks so we just need to reset our state.
-        g_processID = GetCurrentProcessId();
-        g_threadID = 0;
-        g_getMessage = NULL;
-        g_keyboardLL = NULL;
-        g_mouseLL = NULL;
-        g_screenSaver = false;
-    }
-
-    // save thread id.  we'll post messages to this thread's
-    // message queue.
-    g_threadID = threadID;
-
-    // set defaults
-    g_mode = kHOOK_DISABLE;
-    g_zoneSides = 0;
-    g_zoneSize = 0;
-    g_xScreen = 0;
-    g_yScreen = 0;
-    g_wScreen = 0;
-    g_hScreen = 0;
-
-    return 1;
-}
-
-int
-MSWindowsHook::cleanup()
-{
-    if (g_processID == GetCurrentProcessId()) {
-        g_threadID = 0;
-    }
-
-    return 1;
-}
 
 void
 MSWindowsHook::setSides(UInt32 sides)
@@ -445,14 +360,9 @@ keyboardHookHandler(WPARAM wParam, LPARAM lParam)
             // pass event on.  we want to let these through to
             // the window proc because otherwise the keyboard
             // lights may not stay synchronized.
-            break;
-
         case VK_HANGUL:
-            // pass these modifiers if using a low level hook, discard
+            // pass event on because we're using a low level hook
-            // them if not.
+
-            if (g_hookThread == 0) {
-                return true;
-            }
             break;
 
         default:
@@ -493,7 +403,7 @@ keyboardLLHook(int code, WPARAM wParam, LPARAM lParam)
         }
     }
 
-    return CallNextHookEx(g_keyboardLL, code, wParam, lParam);
+    return CallNextHookEx(g_hkKeyboard, code, wParam, lParam);
 }
 #endif // !NO_GRAB_KEYBOARD
 
@@ -613,19 +523,12 @@ mouseLLHook(int code, WPARAM wParam, LPARAM lParam)
         }
     }
 
-    return CallNextHookEx(g_mouseLL, code, wParam, lParam);
+    return CallNextHookEx(g_hkMouse, code, wParam, lParam);
 }
 
-EHookResult
+bool
 MSWindowsHook::install()
 {
-    assert(g_getMessage == NULL || g_screenSaver);
-
-    // must be initialized
-    if (g_threadID == 0) {
-        return kHOOK_FAILED;
-    }
-
     // discard old dead keys
     g_deadVirtKey = 0;
     g_deadLParam = 0;
@@ -633,68 +536,34 @@ MSWindowsHook::install()
     // reset fake input flag
     g_fakeServerInput = false;
 
-    // install low-level hooks.  we require that they both get installed.
+#if NO_GRAB_KEYBOARD
-    g_mouseLL = SetWindowsHookEx(WH_MOUSE_LL,
+    // we only need the mouse hook
-        &mouseLLHook,
+    if (!g_hkMouse.set(WH_MOUSE_LL, &mouseLLHook, NULL, 0))
-        NULL,
+        return false;
-        0);
+#else
-#if !NO_GRAB_KEYBOARD
+    // we need both hooks. if either fails, discard the other
-    g_keyboardLL = SetWindowsHookEx(WH_KEYBOARD_LL,
+    if (!g_hkMouse.set(WH_MOUSE_LL, &mouseLLHook, NULL, 0) ||
-        &keyboardLLHook,
+        !g_hkKeyboard.set(WH_KEYBOARD_LL, &keyboardLLHook, NULL, 0)) {
-        NULL,
+        g_hkMouse.unset();
-        0);
+        g_hkKeyboard.unset();
-    if (g_mouseLL == NULL || g_keyboardLL == NULL) {
+        return false;
-        if (g_keyboardLL != NULL) {
-            UnhookWindowsHookEx(g_keyboardLL);
-            g_keyboardLL = NULL;
-        }
-        if (g_mouseLL != NULL) {
-            UnhookWindowsHookEx(g_mouseLL);
-            g_mouseLL = NULL;
-        }
     }
 #endif
 
-    // check that we got all the hooks we wanted
+    return true;
-    if ((g_mouseLL == NULL)
-#if !NO_GRAB_KEYBOARD
-       || (g_keyboardLL == NULL)
-#endif
-        ) {
-        uninstall();
-        return kHOOK_FAILED;
 }
 
-    if (g_keyboardLL != NULL || g_mouseLL != NULL) {
+void
-        g_hookThread = GetCurrentThreadId();
-        return kHOOK_OKAY_LL;
-    }
-
-    return kHOOK_OKAY;
-}
-
-int
 MSWindowsHook::uninstall()
 {
     // discard old dead keys
     g_deadVirtKey = 0;
     g_deadLParam = 0;
 
-    // uninstall hooks
+    g_hkMouse.unset();
-    if (g_keyboardLL != NULL) {
+    g_hkKeyboard.unset();
-        UnhookWindowsHookEx(g_keyboardLL);
-        g_keyboardLL = NULL;
-    }
-    if (g_mouseLL != NULL) {
-        UnhookWindowsHookEx(g_mouseLL);
-        g_mouseLL = NULL;
-    }
-    if (g_getMessage != NULL && !g_screenSaver) {
-        UnhookWindowsHookEx(g_getMessage);
-        g_getMessage = NULL;
-    }
 
-    return 1;
+    uninstallScreenSaver();
 }
 
 static
@@ -702,7 +571,6 @@ LRESULT CALLBACK
 getMessageHook(int code, WPARAM wParam, LPARAM lParam)
 {
     if (code >= 0) {
-        if (g_screenSaver) {
         MSG* msg = reinterpret_cast<MSG*>(lParam);
         if (msg->message == WM_SYSCOMMAND &&
             msg->wParam == SC_SCREENSAVE) {
@@ -711,44 +579,21 @@ getMessageHook(int code, WPARAM wParam, LPARAM lParam)
                 BARRIER_MSG_SCREEN_SAVER, TRUE, 0);
         }
     }
+
+    return CallNextHookEx(g_hkMessage, code, wParam, lParam);
 }
 
-    return CallNextHookEx(g_getMessage, code, wParam, lParam);
+bool
-}
-
-int
 MSWindowsHook::installScreenSaver()
 {
-    // must be initialized
-    if (g_threadID == 0) {
-        return 0;
-    }
-
-    // generate screen saver messages
-    g_screenSaver = true;
-
     // install hook unless it's already installed
-    if (g_getMessage == NULL) {
+    if (g_hkMessage.is_set())
-        g_getMessage = SetWindowsHookEx(WH_GETMESSAGE,
+        return true;
-            &getMessageHook,
+    return g_hkMessage.set(WH_GETMESSAGE, &getMessageHook, NULL, 0);
-            NULL,
-            0);
 }
 
-    return (g_getMessage != NULL) ? 1 : 0;
+void
-}
-
-int
 MSWindowsHook::uninstallScreenSaver()
 {
-    // uninstall hook unless the mouse wheel hook is installed
+    g_hkMessage.unset();
-    if (g_getMessage != NULL) {
-        UnhookWindowsHookEx(g_getMessage);
-        g_getMessage = NULL;
-    }
-
-    // screen saver hook is no longer installed
-    g_screenSaver = false;
-
-    return 1;
 }

src/lib/platform/MSWindowsHook.h

@@ -28,18 +28,12 @@
 class MSWindowsHook
 {
 public:
-    MSWindowsHook();
-    virtual ~MSWindowsHook();
-
-    void                loadLibrary();
-    int                    init(DWORD threadID);
-    int                    cleanup();
     void                setSides(UInt32 sides);
     void                setZone(SInt32 x, SInt32 y, SInt32 w, SInt32 h, SInt32 jumpZoneSize);
     void                setMode(EHookMode mode);
 
-    static EHookResult install();
+    static bool install();
-    static int uninstall();
+    static void uninstall();
-    static int installScreenSaver();
+    static bool installScreenSaver();
-    static int uninstallScreenSaver();
+    static void uninstallScreenSaver();
 };

src/lib/platform/MSWindowsHookResource.cpp

@@ -0,0 +1,33 @@
+#include "MSWindowsHookResource.h"
+
+WindowsHookResource::WindowsHookResource() :
+    _hook(NULL)
+{
+}
+
+WindowsHookResource::~WindowsHookResource()
+{
+    unset();
+}
+
+bool WindowsHookResource::set(int idHook, HOOKPROC lpfn, HINSTANCE hmod, DWORD dwThreadId)
+{
+    if (is_set())
+        return false;
+    _hook = SetWindowsHookEx(idHook, lpfn, hmod, dwThreadId);
+    return is_set();
+}
+
+bool WindowsHookResource::unset()
+{
+    if (is_set()) {
+        if (UnhookWindowsHookEx(_hook) == 0) {
+            return false;
+        }
+        _hook = NULL;
+    }
+    return true;
+}
+
+bool WindowsHookResource::is_set() const { return _hook != NULL; }
+WindowsHookResource::operator HHOOK() const { return _hook; }

src/lib/platform/MSWindowsHookResource.h

@@ -0,0 +1,20 @@
+#pragma once
+
+#define WIN32_LEAN_AND_MEAN
+#include <Windows.h>
+
+class WindowsHookResource
+{
+public:
+    explicit WindowsHookResource();
+    ~WindowsHookResource();
+
+    bool set(int idHook, HOOKPROC lpfn, HINSTANCE hmod, DWORD dwThreadId);
+    bool unset();
+
+    bool is_set() const;
+    operator HHOOK() const;
+
+private:
+    HHOOK _hook;
+};

src/lib/platform/MSWindowsScreen.cpp

@@ -129,10 +129,6 @@ MSWindowsScreen::MSWindowsScreen(
 
     s_screen = this;
     try {
-        if (m_isPrimary && !m_noHooks) {
-            m_hook.loadLibrary();
-        }
-
         m_screensaver = new MSWindowsScreenSaver();
         m_desks       = new MSWindowsDesks(
                             m_isPrimary,

src/lib/platform/synwinhk.h

@@ -48,12 +48,6 @@
 
 extern "C" {
 
-enum EHookResult {
-    kHOOK_FAILED,
-    kHOOK_OKAY,
-    kHOOK_OKAY_LL
-};
-
 enum EHookMode {
     kHOOK_DISABLE,
     kHOOK_WATCH_JUMP_ZONE,