Stop watching for window destroy notifications

govert · govert · commit 381161cf0828 · 2020-10-26T11:05:42.000+02:00
diff --git a/Source/ExcelDna.IntelliSense/UIMonitor/ExcelToolTipWatcher.cs b/Source/ExcelDna.IntelliSense/UIMonitor/ExcelToolTipWatcher.cs
@@ -1,5 +1,6 @@
 ﻿using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Threading;
 
 namespace ExcelDna.IntelliSense
@@ -57,10 +58,13 @@ void _windowWatcher_ExcelToolTipWindowChanged(object sender, WindowWatcher.Windo
                     }
                     break;
                 case WindowWatcher.WindowChangedEventArgs.ChangeType.Hide:
-                case WindowWatcher.WindowChangedEventArgs.ChangeType.Destroy:
                     if (_toolTips.Remove(e.WindowHandle))
                         ToolTipChanged?.Invoke(this, new ToolTipChangeEventArgs(ToolTipChangeType.Hide, e.WindowHandle));
                     break;
+                case WindowWatcher.WindowChangedEventArgs.ChangeType.Destroy:
+                    // Not expecting this anymore - Destroy is no longer routed from the WinEvents.
+                    Debug.Fail("Unexpected ChangeType");
+                    break;
                 case WindowWatcher.WindowChangedEventArgs.ChangeType.Create:
                 case WindowWatcher.WindowChangedEventArgs.ChangeType.Focus:
                 case WindowWatcher.WindowChangedEventArgs.ChangeType.Unfocus:
diff --git a/Source/ExcelDna.IntelliSense/UIMonitor/FormulaEditWatcher.cs b/Source/ExcelDna.IntelliSense/UIMonitor/FormulaEditWatcher.cs
@@ -115,12 +115,8 @@ void _windowWatcher_FormulaBarWindowChanged(object sender, WindowWatcher.WindowC
                     }
                     break;
                 case WindowWatcher.WindowChangedEventArgs.ChangeType.Destroy:
-                    // We expect this for every text change, but ignore since we react to the Create event
-                    //if (_formulaEditFocus == FormulaEditFocus.FormulaBar)
-                    //{
-                    //    _formulaEditFocus = FormulaEditFocus.None;
-                    //    UpdateEditState();
-                    //}
+                    // Not expecting this anymore - Destroy is no longer routed from the WinEvents.
+                    Debug.Fail("Unexpected ChangeType");
                     break;
                 case WindowWatcher.WindowChangedEventArgs.ChangeType.Focus:
                     if (_formulaEditFocus != FormulaEditFocus.FormulaBar)
@@ -193,12 +189,8 @@ void _windowWatcher_InCellEditWindowChanged(object sender, WindowWatcher.WindowC
                     }
                     break;
                 case WindowWatcher.WindowChangedEventArgs.ChangeType.Destroy:
-                    // We expect this for every text change, but ignore since we react to the Create event
-                    //if (_formulaEditFocus == FormulaEditFocus.FormulaBar)
-                    //{
-                    //    _formulaEditFocus = FormulaEditFocus.None;
-                    //    UpdateEditState();
-                    //}
+                    // Not expecting this anymore - Destroy is no longer routed from the WinEvents.
+                    Debug.Fail("Unexpected ChangeType");
                     break;
                 case WindowWatcher.WindowChangedEventArgs.ChangeType.Focus:
                     if (_formulaEditFocus != FormulaEditFocus.InCellEdit)
diff --git a/Source/ExcelDna.IntelliSense/UIMonitor/PopupListWatcher.cs b/Source/ExcelDna.IntelliSense/UIMonitor/PopupListWatcher.cs
@@ -53,8 +53,8 @@ void _windowWatcher_PopupListWindowChanged(object sender, WindowWatcher.WindowCh
                     _hwndPopupList = e.WindowHandle;
                     break;
                 case WindowWatcher.WindowChangedEventArgs.ChangeType.Destroy:
-                    // We expect this only when shutting down
-                    Logger.WindowWatcher.Info($"PopupList window destroyed: {e.WindowHandle}");
+                    // Not expecting this anymore - Destroy is no longer routed from the WinEvents.
+                    Debug.Fail("Unexpected ChangeType");
                     break;
                 case WindowWatcher.WindowChangedEventArgs.ChangeType.Show:
                     Logger.WindowWatcher.Verbose($"PopupList window show");
diff --git a/Source/ExcelDna.IntelliSense/UIMonitor/WinEvents.cs b/Source/ExcelDna.IntelliSense/UIMonitor/WinEvents.cs
@@ -17,15 +17,17 @@ public class WinEventArgs : EventArgs
         {
             public WinEvent EventType;
             public IntPtr WindowHandle;
+            public string WindowClassName;
             public WinEventObjectId ObjectId;
             public int ChildId;
             public uint EventThreadId;
             public uint EventTimeMs;
 
-            public WinEventArgs(WinEvent eventType, IntPtr hWnd, WinEventObjectId idObject, int idChild, uint dwEventThread, uint dwmsEventTime)
+            public WinEventArgs(WinEvent eventType, IntPtr hWnd, string windowClassName, WinEventObjectId idObject, int idChild, uint dwEventThread, uint dwmsEventTime)
             {
                 EventType = eventType;
                 WindowHandle = hWnd;
+                WindowClassName = windowClassName;
                 ObjectId = idObject;
                 ChildId = idChild;
                 EventThreadId = dwEventThread;
@@ -187,11 +189,14 @@ void HandleWinEvent(IntPtr hWinEventHook, WinEvent eventType, IntPtr hWnd,
                 if (_hWndFilterOrZero != IntPtr.Zero && hWnd != _hWndFilterOrZero)
                     return;
 
-                if (!IsSupportedWinEvent(eventType))
+                if (!IsSupportedWinEvent(eventType) || idObject == WinEventObjectId.OBJID_CURSOR)
                     return;
 
+                // Moving the GetClassName call here where the main thread is running.
+                var windowClassName = Win32Helper.GetClassName(hWnd);
+                    
                 // CONSIDER: We might add some filtering here... maybe only interested in some of the window / event combinations
-                _syncContextAuto.Post(OnWinEventReceived, new WinEventArgs(eventType, hWnd, idObject, idChild, dwEventThread, dwmsEventTime));
+                _syncContextAuto.Post(OnWinEventReceived, new WinEventArgs(eventType, hWnd, windowClassName, idObject, idChild, dwEventThread, dwmsEventTime));
             }
             catch (Exception ex)
             {
@@ -203,7 +208,7 @@ void HandleWinEvent(IntPtr hWinEventHook, WinEvent eventType, IntPtr hWnd,
         bool IsSupportedWinEvent(WinEvent winEvent)
         {
             return winEvent == WinEvent.EVENT_OBJECT_CREATE ||
-                   winEvent == WinEvent.EVENT_OBJECT_DESTROY ||
+                   // winEvent == WinEvent.EVENT_OBJECT_DESTROY ||  // Stopped watching for this, because we can't route using the ClassName and don't really need anymore
                    winEvent == WinEvent.EVENT_OBJECT_SHOW ||
                    winEvent == WinEvent.EVENT_OBJECT_HIDE ||
                    winEvent == WinEvent.EVENT_OBJECT_FOCUS ||
@@ -217,9 +222,10 @@ bool IsSupportedWinEvent(WinEvent winEvent)
         void OnWinEventReceived(object winEventArgsObj)
         {
             var winEventArgs = (WinEventArgs)winEventArgsObj;
+            if (winEventArgs.ObjectId == WinEventObjectId.OBJID_CURSOR)
+                return;
 #if DEBUG
-            if (winEventArgs.ObjectId != WinEventObjectId.OBJID_CURSOR)
-                Logger.WinEvents.Verbose($"{winEventArgs.EventType} - Window {winEventArgs.WindowHandle:X} ({Win32Helper.GetClassName(winEventArgs.WindowHandle)} - Object/Child {winEventArgs.ObjectId} / {winEventArgs.ChildId} - Thread {winEventArgs.EventThreadId} at {winEventArgs.EventTimeMs}");
+            Logger.WinEvents.Verbose($"{winEventArgs.EventType} - Window {winEventArgs.WindowHandle:X} {(winEventArgs.WindowHandle != IntPtr.Zero ? Win32Helper.GetClassName(winEventArgs.WindowHandle) : "")} - Object/Child {winEventArgs.ObjectId} / {winEventArgs.ChildId} - Thread {winEventArgs.EventThreadId} at {winEventArgs.EventTimeMs}");
 #endif
             WinEventReceived?.Invoke(this, winEventArgs);
         }
diff --git a/Source/ExcelDna.IntelliSense/UIMonitor/WindowWatcher.cs b/Source/ExcelDna.IntelliSense/UIMonitor/WindowWatcher.cs
@@ -153,12 +153,11 @@ public void TryInitialize()
 
         bool UpdateFocus(IntPtr windowHandle, string windowClassName)
         {
-            if (windowHandle == _focusedWindowHandle)
-            {
-                Debug.Assert(_focusedWindowClassName == windowClassName);   // I've seen this, with _focusedWindowClassName == "" and windowClassName == "EXCEL7".
-                return false;
-            }
+            if (windowHandle == _focusedWindowHandle && _focusedWindowClassName == windowClassName)
+                    return false;
 
+            // We see a change in the WindowClassName often - handle that as a focus change too
+ 
             Debug.Assert(_focusedWindowClassName != _excelToolTipClass); // We don't expect the ToolTip to ever get the focus
             Logger.WindowWatcher.Verbose($"Focus lost by {_focusedWindowHandle} ({_focusedWindowClassName})");
             // It has changed - raise an event for the old window
@@ -191,21 +190,25 @@ bool UpdateFocus(IntPtr windowHandle, string windowClassName)
         // CONSIDER: We would be able to run all the watcher updates from WinEvents, including Location and Selection changes,
         //           but since WinEvents have no hwnd filter, UIAutomation events might be more efficient.
         // CONSIDER: Performance optimisation would keep a list of window handles we know about, preventing the class name check every time
+        // NOTE: We are not getting OBJID_CURSOR events here - that means we expect to have a valid WindowHandle except when destroyed
         void _windowStateChangeHook_WinEventReceived(object sender, WinEventHook.WinEventArgs e)
         {
-            var className = Win32Helper.GetClassName(e.WindowHandle);
+            if (e.WindowHandle == IntPtr.Zero)
+            {
+                Debug.Fail("WinEvent with window 0!?");
+            }
             if (e.EventType == WinEventHook.WinEvent.EVENT_OBJECT_FOCUS)
             {
                 // Might raise change event for Unfocus
-                if (!UpdateFocus(e.WindowHandle, className))
+                if (!UpdateFocus(e.WindowHandle, e.WindowClassName))
                 {
                     // We already have the right focus
                     return;
                 }
             }
 
             // Debug.Print("### Thread receiving WindowStateChange: " + Thread.CurrentThread.ManagedThreadId);
-            switch (className)
+            switch (e.WindowClassName)
             {
                 //case _sheetWindowClass:
                 //    if (e.EventType == WinEventHook.WinEvent.EVENT_OBJECT_SHOW)
diff --git a/Source/ExcelDna.IntelliSense/Win32Helper.cs b/Source/ExcelDna.IntelliSense/Win32Helper.cs
@@ -29,7 +29,7 @@ enum WM : uint
         public static extern IntPtr GetModuleHandle(string lpModuleName);
         [DllImport("kernel32.dll")]
         static extern uint GetCurrentProcessId();
-        [DllImport("user32.dll")]
+        [DllImport("user32.dll", SetLastError = true)]
         static extern int GetClassNameW(IntPtr hwnd, [MarshalAs(UnmanagedType.LPWStr)] StringBuilder buf, int nMaxCount);
 
         [DllImport("user32.dll", CharSet = CharSet.Auto, SetLastError = true)]
@@ -228,7 +228,13 @@ public static uint GetExcelProcessId()
         public static string GetClassName(IntPtr hWnd)
         {
             _buffer.Length = 0;
-            GetClassNameW(hWnd, _buffer, _buffer.Capacity);
+            int result = GetClassNameW(hWnd, _buffer, _buffer.Capacity);
+            if (result == 0)
+            {
+                // It failed!?
+                int error = Marshal.GetLastWin32Error();
+                Debug.Print($"GetClassName failed on {hWnd}(0x{hWnd:x}) - Error {error}");
+            }
             return _buffer.ToString();
         }
 
