Skip to content

Commit 7e377a9

Browse files
lijy91lijy91
committed
Refactor window ID management and registration on Windows
Window IDs are now stored as a custom property on HWNDs, ensuring consistent retrieval and avoiding duplicate allocations. Window registration in the WindowRegistry is now handled dynamically via enumeration in WindowManager::GetAll(), and window destruction properly removes IDs and properties. This improves reliability of window tracking, event handling, and registry consistency across the Windows platform code.
1 parent 033cfb1 commit 7e377a9

File tree

2 files changed

+192
-66
lines changed

2 files changed

+192
-66
lines changed

‎src/platform/windows/window_manager_windows.cpp‎

Lines changed: 103 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,39 @@
1313

1414
namespace nativeapi {
1515

16+
// Property name for storing window ID in HWND (must match window_windows.cpp)
17+
static const wchar_t* kWindowIdProperty = L"NativeAPIWindowId";
18+
19+
// Helper function to get window ID from HWND
20+
// First tries to read from custom property, then creates Window object if needed
21+
static WindowId GetWindowIdFromHwnd(HWND hwnd) {
22+
if (!hwnd) {
23+
return IdAllocator::kInvalidId;
24+
}
25+
26+
// First, try to get window ID from HWND's custom property
27+
HANDLE prop_handle = GetProp(hwnd, kWindowIdProperty);
28+
if (prop_handle) {
29+
WindowId window_id = static_cast<WindowId>(reinterpret_cast<uintptr_t>(prop_handle));
30+
if (window_id != IdAllocator::kInvalidId && window_id != 0) {
31+
return window_id;
32+
}
33+
}
34+
35+
// If property doesn't exist, create a new Window object and register it
36+
// Use shared_ptr so it can be properly registered in WindowRegistry
37+
auto window = std::make_shared<Window>(hwnd);
38+
WindowId window_id = window->GetId();
39+
40+
// Register the window manually since constructor's shared_from_this() fails
41+
// during construction (shared_ptr control block not fully initialized yet)
42+
if (window_id != IdAllocator::kInvalidId) {
43+
WindowRegistry::GetInstance().Add(window_id, window);
44+
}
45+
46+
return window_id;
47+
}
48+
1649
namespace {
1750

1851
using PFN_ShowWindow = BOOL(WINAPI*)(HWND, int);
@@ -37,9 +70,12 @@ static bool IsShowCommandVisible(int cmd) {
3770
}
3871

3972
static void InvokePreShowHideHooks(HWND hwnd, int cmd) {
73+
WindowId window_id = GetWindowIdFromHwnd(hwnd);
74+
if (window_id == IdAllocator::kInvalidId) {
75+
return;
76+
}
77+
4078
auto& manager = WindowManager::GetInstance();
41-
Window temp_window(hwnd);
42-
WindowId window_id = temp_window.GetId();
4379
if (cmd == SW_HIDE) {
4480
manager.InvokeWillHideHook(window_id);
4581
} else if (IsShowCommandVisible(cmd)) {
@@ -59,12 +95,15 @@ static BOOL WINAPI HookedShowWindow(HWND hwnd, int nCmdShow) {
5995
}
6096

6197
static BOOL WINAPI HookedShowWindowAsync(HWND hwnd, int nCmdShow) {
98+
std::cout << "HookedShowWindowAsync called for hwnd: " << hwnd << " with nCmdShow: " << nCmdShow << std::endl;
6299
InvokePreShowHideHooks(hwnd, nCmdShow);
63100
if (g_original_show_window_async) {
101+
std::cout << "Calling original ShowWindowAsync" << std::endl;
64102
return g_original_show_window_async(hwnd, nCmdShow);
65103
}
66104
auto p = reinterpret_cast<PFN_ShowWindowAsync>(
67105
GetProcAddress(GetModuleHandleW(L"user32.dll"), "ShowWindowAsync"));
106+
std::cout << "Calling fallback ShowWindowAsync" << std::endl;
68107
return p ? p(hwnd, nCmdShow) : FALSE;
69108
}
70109

@@ -267,9 +306,11 @@ class WindowManager::Impl {
267306
}
268307

269308
void OnWindowEvent(HWND hwnd, const std::string& event_type) {
270-
// Create a temporary Window object to get the proper WindowId
271-
Window temp_window(hwnd);
272-
WindowId window_id = temp_window.GetId();
309+
// Get window ID, first trying custom property, then creating Window if needed
310+
WindowId window_id = GetWindowIdFromHwnd(hwnd);
311+
if (window_id == IdAllocator::kInvalidId) {
312+
return;
313+
}
273314

274315
if (event_type == "focused") {
275316
WindowFocusedEvent event(window_id);
@@ -333,30 +374,72 @@ bool WindowManager::Destroy(WindowId id) {
333374
}
334375

335376
std::shared_ptr<Window> WindowManager::Get(WindowId id) {
336-
auto cached = WindowRegistry::GetInstance().Get(id);
337-
if (cached) {
338-
return cached;
339-
}
340-
// Check if the window still exists in the system
341-
HWND hwnd = reinterpret_cast<HWND>(id);
342-
if (IsWindow(hwnd)) {
343-
auto window = std::make_shared<Window>(hwnd);
344-
WindowRegistry::GetInstance().Add(id, window);
377+
// First try to get from registry
378+
auto window = WindowRegistry::GetInstance().Get(id);
379+
if (window) {
345380
return window;
346381
}
347-
return nullptr;
382+
383+
// If not in registry, enumerate all windows to find it
384+
// This will create and register the window if it exists
385+
GetAll();
386+
387+
// Try again after enumeration
388+
return WindowRegistry::GetInstance().Get(id);
389+
}
390+
391+
// Callback for EnumWindows to collect all top-level windows
392+
static BOOL CALLBACK EnumWindowsCallback(HWND hwnd, LPARAM lParam) {
393+
auto* windows = reinterpret_cast<std::vector<HWND>*>(lParam);
394+
395+
// Only include visible windows that are not minimized to taskbar
396+
// and have a title (filters out many background windows)
397+
if (IsWindowVisible(hwnd)) {
398+
int length = GetWindowTextLengthW(hwnd);
399+
if (length > 0) {
400+
// Check if it's a normal window (not tool window, etc.)
401+
LONG exStyle = GetWindowLong(hwnd, GWL_EXSTYLE);
402+
if (!(exStyle & WS_EX_TOOLWINDOW)) {
403+
windows->push_back(hwnd);
404+
}
405+
}
406+
}
407+
408+
return TRUE; // Continue enumeration
348409
}
349410

350411
std::vector<std::shared_ptr<Window>> WindowManager::GetAll() {
351-
return WindowRegistry::GetInstance().GetAll();
412+
std::vector<HWND> hwnds;
413+
414+
// Enumerate all top-level windows
415+
EnumWindows(EnumWindowsCallback, reinterpret_cast<LPARAM>(&hwnds));
416+
417+
std::vector<std::shared_ptr<Window>> windows;
418+
windows.reserve(hwnds.size());
419+
420+
for (HWND hwnd : hwnds) {
421+
// Get window ID from HWND, creating Window object if needed
422+
WindowId window_id = GetWindowIdFromHwnd(hwnd);
423+
424+
if (window_id != IdAllocator::kInvalidId) {
425+
// Try to get existing window from registry
426+
auto window = WindowRegistry::GetInstance().Get(window_id);
427+
if (window) {
428+
windows.push_back(window);
429+
}
430+
}
431+
}
432+
433+
return windows;
352434
}
353435

354436
std::shared_ptr<Window> WindowManager::GetCurrent() {
355-
HWND hwnd = GetForegroundWindow();
437+
HWND hwnd = GetActiveWindow();
356438
if (hwnd) {
357-
Window temp_window(hwnd);
358-
WindowId window_id = temp_window.GetId();
359-
return Get(window_id);
439+
WindowId window_id = GetWindowIdFromHwnd(hwnd);
440+
if (window_id != IdAllocator::kInvalidId) {
441+
return Get(window_id);
442+
}
360443
}
361444
return nullptr;
362445
}

‎src/platform/windows/window_windows.cpp‎

Lines changed: 89 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
#include <windows.h>
22
#include <iostream>
3-
#include <mutex>
4-
#include <unordered_map>
53
#include "../../foundation/id_allocator.h"
64
#include "../../window.h"
75
#include "../../window_manager.h"
@@ -11,14 +9,18 @@
119

1210
namespace nativeapi {
1311

12+
// Property name for storing window ID in HWND
13+
static const wchar_t* kWindowIdProperty = L"NativeAPIWindowId";
14+
1415
// Forward declaration
1516
static LRESULT CALLBACK WindowProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam);
1617

1718
// Private implementation class
1819
class Window::Impl {
1920
public:
20-
Impl(HWND hwnd) : hwnd_(hwnd) {}
21+
Impl(HWND hwnd, WindowId id) : hwnd_(hwnd), window_id_(id) {}
2122
HWND hwnd_;
23+
WindowId window_id_;
2224
};
2325

2426
// Custom window procedure to handle window messages
@@ -28,14 +30,19 @@ static LRESULT CALLBACK WindowProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM l
2830
// Intercept visibility changes BEFORE they happen (pre-show/hide "swizzle")
2931
WINDOWPOS* pos = reinterpret_cast<WINDOWPOS*>(lParam);
3032
if (pos) {
31-
auto& manager = WindowManager::GetInstance();
32-
Window temp_window(hwnd);
33-
WindowId window_id = temp_window.GetId();
34-
if (pos->flags & SWP_SHOWWINDOW) {
35-
manager.InvokeWillShowHook(window_id);
36-
}
37-
if (pos->flags & SWP_HIDEWINDOW) {
38-
manager.InvokeWillHideHook(window_id);
33+
// Get window ID from window's custom property (stored during window creation)
34+
HANDLE prop_handle = GetProp(hwnd, kWindowIdProperty);
35+
if (prop_handle) {
36+
WindowId window_id = static_cast<WindowId>(reinterpret_cast<uintptr_t>(prop_handle));
37+
if (window_id != IdAllocator::kInvalidId) {
38+
auto& manager = WindowManager::GetInstance();
39+
if (pos->flags & SWP_SHOWWINDOW) {
40+
manager.InvokeWillShowHook(window_id);
41+
}
42+
if (pos->flags & SWP_HIDEWINDOW) {
43+
manager.InvokeWillHideHook(window_id);
44+
}
45+
}
3946
}
4047
}
4148
return DefWindowProc(hwnd, uMsg, wParam, lParam);
@@ -75,7 +82,9 @@ Window::Window() {
7582
DWORD error = GetLastError();
7683
if (error != ERROR_CLASS_ALREADY_EXISTS) {
7784
std::cerr << "Failed to register window class. Error: " << error << std::endl;
78-
pimpl_ = std::make_unique<Impl>(nullptr);
85+
// Allocate ID even for failed window creation to maintain consistency
86+
WindowId id = IdAllocator::Allocate<Window>();
87+
pimpl_ = std::make_unique<Impl>(nullptr, id);
7988
return;
8089
}
8190
class_registered = true;
@@ -91,18 +100,78 @@ Window::Window() {
91100

92101
if (!hwnd) {
93102
std::cerr << "Failed to create window. Error: " << GetLastError() << std::endl;
94-
pimpl_ = std::make_unique<Impl>(nullptr);
103+
// Allocate ID even for failed window creation to maintain consistency
104+
WindowId id = IdAllocator::Allocate<Window>();
105+
pimpl_ = std::make_unique<Impl>(nullptr, id);
95106
return;
96107
}
97108

98-
// Only create the instance, don't show the window
99-
pimpl_ = std::make_unique<Impl>(hwnd);
109+
// Allocate window ID using IdAllocator
110+
WindowId id = IdAllocator::Allocate<Window>();
111+
if (id == IdAllocator::kInvalidId) {
112+
std::cerr << "Failed to allocate window ID" << std::endl;
113+
DestroyWindow(hwnd);
114+
pimpl_ = std::make_unique<Impl>(nullptr, IdAllocator::kInvalidId);
115+
return;
116+
}
117+
118+
// Store window ID as a custom property in HWND for easy retrieval in WindowProc
119+
SetProp(hwnd, kWindowIdProperty, reinterpret_cast<HANDLE>(static_cast<uintptr_t>(id)));
120+
121+
// Create the instance with allocated ID
122+
pimpl_ = std::make_unique<Impl>(hwnd, id);
123+
124+
// Note: Window registration in WindowRegistry is now handled by WindowManager::GetAll()
125+
// which uses EnumWindows to discover and register all windows dynamically
100126
}
101127

102-
Window::Window(void* native_window)
103-
: pimpl_(std::make_unique<Impl>(static_cast<HWND>(native_window))) {}
128+
Window::Window(void* native_window) {
129+
HWND hwnd = static_cast<HWND>(native_window);
130+
131+
if (!hwnd) {
132+
// Allocate ID even for null window to maintain consistency
133+
WindowId id = IdAllocator::Allocate<Window>();
134+
pimpl_ = std::make_unique<Impl>(nullptr, id);
135+
return;
136+
}
137+
138+
// Check if window already has an ID stored as a custom property
139+
HANDLE prop_handle = GetProp(hwnd, kWindowIdProperty);
140+
WindowId id = IdAllocator::kInvalidId;
141+
142+
if (prop_handle) {
143+
id = static_cast<WindowId>(reinterpret_cast<uintptr_t>(prop_handle));
144+
}
145+
146+
if (id == IdAllocator::kInvalidId || id == 0) {
147+
// Allocate new ID if window doesn't have one
148+
id = IdAllocator::Allocate<Window>();
149+
if (id == IdAllocator::kInvalidId) {
150+
std::cerr << "Failed to allocate window ID" << std::endl;
151+
pimpl_ = std::make_unique<Impl>(nullptr, IdAllocator::kInvalidId);
152+
return;
153+
}
154+
// Store the ID as a custom property in HWND
155+
SetProp(hwnd, kWindowIdProperty, reinterpret_cast<HANDLE>(static_cast<uintptr_t>(id)));
156+
}
157+
158+
pimpl_ = std::make_unique<Impl>(hwnd, id);
104159

105-
Window::~Window() {}
160+
// Note: Window registration in WindowRegistry is now handled by WindowManager::GetAll()
161+
// which uses EnumWindows to discover and register all windows dynamically
162+
}
163+
164+
Window::~Window() {
165+
// Remove window from registry on destruction
166+
if (pimpl_ && pimpl_->window_id_ != IdAllocator::kInvalidId) {
167+
WindowRegistry::GetInstance().Remove(pimpl_->window_id_);
168+
169+
// Remove the custom property from HWND if window is still valid
170+
if (pimpl_->hwnd_) {
171+
RemoveProp(pimpl_->hwnd_, kWindowIdProperty);
172+
}
173+
}
174+
}
106175

107176
void Window::Focus() {
108177
if (pimpl_->hwnd_) {
@@ -654,36 +723,10 @@ void Window::StartResizing() {
654723
}
655724

656725
WindowId Window::GetId() const {
657-
// Use IdAllocator to generate unique IDs instead of casting pointers
658-
if (!pimpl_->hwnd_) {
726+
if (!pimpl_) {
659727
return IdAllocator::kInvalidId;
660728
}
661-
662-
// Store the allocated ID in a static map to ensure consistency
663-
static std::unordered_map<HWND, WindowId> window_id_map;
664-
static std::mutex map_mutex;
665-
666-
std::lock_guard<std::mutex> lock(map_mutex);
667-
auto it = window_id_map.find(pimpl_->hwnd_);
668-
if (it != window_id_map.end()) {
669-
return it->second;
670-
}
671-
672-
// Allocate new ID using the IdAllocator
673-
WindowId new_id = IdAllocator::Allocate<Window>();
674-
if (new_id != IdAllocator::kInvalidId) {
675-
window_id_map[pimpl_->hwnd_] = new_id;
676-
677-
// Register window in registry (delayed registration)
678-
// This requires the Window to be managed by shared_ptr
679-
try {
680-
WindowRegistry::GetInstance().Add(new_id, const_cast<Window*>(this)->shared_from_this());
681-
} catch (const std::bad_weak_ptr&) {
682-
// Window not yet managed by shared_ptr, skip registration
683-
// Registration will happen when window is properly managed by shared_ptr
684-
}
685-
}
686-
return new_id;
729+
return pimpl_->window_id_;
687730
}
688731

689732
void* Window::GetNativeObjectInternal() const {

0 commit comments

Comments
 (0)