Skip to content

Commit 8930e89

Browse files
committed
Refactor window management to use WindowRegistry
Introduces a thread-safe ObjectRegistry utility and a WindowRegistry singleton to centralize window instance tracking. Refactors platform-specific WindowManager implementations to use WindowRegistry for storing, retrieving, and enumerating windows, replacing internal unordered_map usage. Updates window_manager.h to remove the internal windows_ map and adds new window_registry.{h,cpp} files for registry logic.
1 parent 62aaba6 commit 8930e89

File tree

7 files changed

+243
-72
lines changed

7 files changed

+243
-72
lines changed

src/foundation/object_registry.h

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/**
2+
* @file object_registry.h
3+
* @brief Thread-safe registry for mapping IDs to shared objects.
4+
*
5+
* This utility provides a minimal, lock-guarded container that stores
6+
* `std::shared_ptr<TObject>` instances keyed by `TId`. It is intended for
7+
* simple, centralized tracking of live objects (e.g., windows, menus) and
8+
* supports lookup, enumeration, removal, and clearing operations.
9+
*
10+
* Thread-safety:
11+
* - All public methods take a mutex to protect internal state.
12+
* - Methods marked `const` still acquire the mutex (via a `mutable` mutex)
13+
* to ensure safe concurrent access while preserving the const interface.
14+
*
15+
* Requirements:
16+
* - `TId` must be hashable and equality comparable (usable as an
17+
* `unordered_map` key).
18+
* - Objects are stored as `std::shared_ptr<TObject>`. The registry does not
19+
* impose ownership semantics beyond holding shared references.
20+
*/
21+
#pragma once
22+
#include <memory>
23+
#include <mutex>
24+
#include <unordered_map>
25+
#include <vector>
26+
27+
namespace nativeapi {
28+
29+
template <typename TObject, typename TId>
30+
class ObjectRegistry {
31+
public:
32+
/**
33+
* @brief Add or replace an object for the given ID.
34+
*
35+
* If an entry for `id` already exists, it is replaced by `object`.
36+
* This operation is O(1) average-case.
37+
*
38+
* @param id The identifier used as key in the registry.
39+
* @param object The object to store; moved into the registry.
40+
*/
41+
void Add(TId id, std::shared_ptr<TObject> object) {
42+
std::lock_guard<std::mutex> lock(mutex_);
43+
objects_[id] = std::move(object);
44+
}
45+
46+
/**
47+
* @brief Get the object associated with an ID.
48+
*
49+
* This operation is O(1) average-case.
50+
*
51+
* @param id The identifier to look up.
52+
* @return std::shared_ptr<TObject> The stored object if present,
53+
* otherwise `nullptr`.
54+
*/
55+
std::shared_ptr<TObject> Get(TId id) const {
56+
std::lock_guard<std::mutex> lock(mutex_);
57+
auto it = objects_.find(id);
58+
return it == objects_.end() ? nullptr : it->second;
59+
}
60+
61+
/**
62+
* @brief Get a snapshot vector of all stored objects.
63+
*
64+
* The returned vector contains strong references to the objects as they
65+
* existed at the moment of the call. Subsequent mutations to the registry
66+
* are not reflected in the returned vector.
67+
*
68+
* Complexity: O(N) to allocate and copy shared pointers.
69+
*
70+
* @return std::vector<std::shared_ptr<TObject>> Snapshot of all objects.
71+
*/
72+
std::vector<std::shared_ptr<TObject>> GetAll() const {
73+
std::lock_guard<std::mutex> lock(mutex_);
74+
std::vector<std::shared_ptr<TObject>> result;
75+
result.reserve(objects_.size());
76+
for (const auto& kv : objects_) {
77+
result.push_back(kv.second);
78+
}
79+
return result;
80+
}
81+
82+
/**
83+
* @brief Remove an object by ID.
84+
*
85+
* This operation is O(1) average-case.
86+
*
87+
* @param id The identifier to remove.
88+
* @return true If an entry was found and removed.
89+
* @return false If no entry existed for the given ID.
90+
*/
91+
bool Remove(TId id) {
92+
std::lock_guard<std::mutex> lock(mutex_);
93+
return objects_.erase(id) > 0;
94+
}
95+
96+
/**
97+
* @brief Remove all entries from the registry.
98+
*
99+
* Complexity: O(N) to destroy or release stored shared pointers.
100+
*/
101+
void Clear() {
102+
std::lock_guard<std::mutex> lock(mutex_);
103+
objects_.clear();
104+
}
105+
106+
private:
107+
// Mutex is mutable to allow locking in logically-const operations.
108+
mutable std::mutex mutex_;
109+
std::unordered_map<TId, std::shared_ptr<TObject>> objects_;
110+
};
111+
112+
} // namespace nativeapi
113+
114+

src/platform/linux/window_manager_linux.cpp

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include "../../window.h"
99
#include "../../window_manager.h"
10+
#include "../../window_registry.h"
1011

1112
// Import GTK headers
1213
#include <gdk/gdk.h>
@@ -255,9 +256,9 @@ void WindowManager::DispatchWindowEvent(const WindowEvent& event) {
255256
}
256257

257258
std::shared_ptr<Window> WindowManager::Get(WindowId id) {
258-
auto it = windows_.find(id);
259-
if (it != windows_.end()) {
260-
return it->second;
259+
auto cached = WindowRegistry::GetInstance().Get(id);
260+
if (cached) {
261+
return cached;
261262
}
262263

263264
// Try to find the window by ID in the current display
@@ -274,7 +275,7 @@ std::shared_ptr<Window> WindowManager::Get(WindowId id) {
274275

275276
if (gdk_window && GetOrCreateWindowId(gdk_window) == id) {
276277
auto window = std::make_shared<Window>((void*)gdk_window);
277-
windows_[id] = window;
278+
WindowRegistry::GetInstance().Add(id, window);
278279
g_list_free(toplevels);
279280
return window;
280281
}
@@ -299,20 +300,16 @@ std::vector<std::shared_ptr<Window>> WindowManager::GetAll() {
299300

300301
if (gdk_window) {
301302
WindowId window_id = GetOrCreateWindowId(gdk_window);
302-
auto it = windows_.find(window_id);
303-
if (it == windows_.end()) {
303+
if (!WindowRegistry::GetInstance().Get(window_id)) {
304304
auto window = std::make_shared<Window>((void*)gdk_window);
305-
windows_[window_id] = window;
305+
WindowRegistry::GetInstance().Add(window_id, window);
306306
}
307307
}
308308
}
309309
g_list_free(toplevels);
310310

311311
// Return all cached windows
312-
for (auto& window : windows_) {
313-
windows.push_back(window.second);
314-
}
315-
return windows;
312+
return WindowRegistry::GetInstance().GetAll();
316313
}
317314

318315
std::shared_ptr<Window> WindowManager::GetCurrent() {
@@ -417,7 +414,7 @@ std::shared_ptr<Window> WindowManager::Create(const WindowOptions& options) {
417414
// Create our Window wrapper and cache before showing
418415
auto window = std::make_shared<Window>((void*)gdk_window);
419416
WindowId window_id = GetOrCreateWindowId(gdk_window);
420-
windows_[window_id] = window;
417+
WindowRegistry::GetInstance().Add(window_id, window);
421418

422419
// Invoke pre-show hook if set
423420
InvokeWillShowHook(window_id);
@@ -431,13 +428,13 @@ std::shared_ptr<Window> WindowManager::Create(const WindowOptions& options) {
431428
}
432429

433430
bool WindowManager::Destroy(WindowId id) {
434-
auto it = windows_.find(id);
435-
if (it != windows_.end()) {
436-
// TODO: Implement proper GTK window destruction
437-
windows_.erase(it);
438-
return true;
431+
auto window = WindowRegistry::GetInstance().Get(id);
432+
if (!window) {
433+
return false;
439434
}
440-
return false;
435+
// TODO: Implement proper GTK window destruction
436+
WindowRegistry::GetInstance().Remove(id);
437+
return true;
441438
}
442439

443440
void WindowManager::SetWillShowHook(std::optional<WindowWillShowHook> hook) {

src/platform/macos/window_manager_macos.mm

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
#include "../../window.h"
88
#include "../../window_manager.h"
9+
#include "../../window_registry.h"
910

1011
// Forward declaration for the delegate
1112
@class NativeAPIWindowManagerDelegate;
@@ -272,7 +273,7 @@ - (void)windowWillClose:(NSNotification*)notification {
272273
[ns_window makeMainWindow];
273274
WindowId window_id = [ns_window windowNumber];
274275
auto window = std::make_shared<Window>((__bridge void*)ns_window);
275-
windows_[window_id] = window;
276+
WindowRegistry::GetInstance().Add(window_id, window);
276277

277278
// Dispatch window created event
278279
WindowCreatedEvent created_event(window_id);
@@ -283,33 +284,32 @@ - (void)windowWillClose:(NSNotification*)notification {
283284

284285
// Destroy a window by its ID. Returns true if window was destroyed.
285286
bool WindowManager::Destroy(WindowId id) {
286-
auto it = windows_.find(id);
287-
if (it != windows_.end()) {
288-
// Get the NSWindow to close it
289-
NSArray* ns_windows = [[NSApplication sharedApplication] windows];
290-
for (NSWindow* ns_window in ns_windows) {
291-
if ([ns_window windowNumber] == id) {
292-
[ns_window close];
293-
windows_.erase(it);
294-
return true;
295-
}
287+
auto window = WindowRegistry::GetInstance().Get(id);
288+
if (!window) {
289+
return false;
290+
}
291+
NSArray* ns_windows = [[NSApplication sharedApplication] windows];
292+
for (NSWindow* ns_window in ns_windows) {
293+
if ([ns_window windowNumber] == id) {
294+
[ns_window close];
295+
WindowRegistry::GetInstance().Remove(id);
296+
return true;
296297
}
297-
// Remove from our map even if we couldn't find the NSWindow
298-
windows_.erase(it);
299298
}
299+
WindowRegistry::GetInstance().Remove(id);
300300
return false;
301301
}
302302

303303
std::shared_ptr<Window> WindowManager::Get(WindowId id) {
304-
auto it = windows_.find(id);
305-
if (it != windows_.end()) {
306-
return it->second;
304+
auto cached = WindowRegistry::GetInstance().Get(id);
305+
if (cached) {
306+
return cached;
307307
}
308308
NSArray* ns_windows = [[NSApplication sharedApplication] windows];
309309
for (NSWindow* ns_window in ns_windows) {
310310
if ([ns_window windowNumber] == id) {
311311
auto window = std::make_shared<Window>((__bridge void*)ns_window);
312-
windows_[id] = window;
312+
WindowRegistry::GetInstance().Add(id, window);
313313
return window;
314314
}
315315
}
@@ -321,14 +321,15 @@ - (void)windowWillClose:(NSNotification*)notification {
321321
NSArray* ns_windows = [[NSApplication sharedApplication] windows];
322322
for (NSWindow* ns_window in ns_windows) {
323323
WindowId window_id = [ns_window windowNumber];
324-
auto it = windows_.find(window_id);
325-
if (it == windows_.end()) {
324+
if (!WindowRegistry::GetInstance().Get(window_id)) {
326325
auto window = std::make_shared<Window>((__bridge void*)ns_window);
327-
windows_[window_id] = window;
326+
WindowRegistry::GetInstance().Add(window_id, window);
328327
}
329328
}
330-
for (auto& window : windows_) {
331-
windows.push_back(window.second);
329+
// Merge cached windows from registry to ensure consistency
330+
auto cached = WindowRegistry::GetInstance().GetAll();
331+
for (const auto& w : cached) {
332+
windows.push_back(w);
332333
}
333334
return windows;
334335
}

src/platform/windows/window_manager_windows.cpp

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <psapi.h>
66
#include "../../window.h"
77
#include "../../window_event.h"
8+
#include "../../window_registry.h"
89
#include "../../window_manager.h"
910
#include "string_utils_windows.h"
1011

@@ -421,7 +422,7 @@ std::shared_ptr<Window> WindowManager::Create(const WindowOptions& options) {
421422

422423
auto window = std::make_shared<Window>(hwnd);
423424
WindowId window_id = window->GetId();
424-
windows_[window_id] = window;
425+
WindowRegistry::GetInstance().Add(window_id, window);
425426

426427
// Dispatch window created event
427428
WindowCreatedEvent created_event(window_id);
@@ -432,45 +433,35 @@ std::shared_ptr<Window> WindowManager::Create(const WindowOptions& options) {
432433

433434
// Destroy a window by its ID. Returns true if window was destroyed.
434435
bool WindowManager::Destroy(WindowId id) {
435-
auto it = windows_.find(id);
436-
if (it != windows_.end()) {
437-
HWND hwnd = static_cast<HWND>(it->second->GetNativeObject());
438-
if (IsWindow(hwnd)) {
439-
DestroyWindow(hwnd);
440-
}
441-
windows_.erase(it);
442-
return true;
436+
auto window = WindowRegistry::GetInstance().Get(id);
437+
if (!window) {
438+
return false;
439+
}
440+
HWND hwnd = static_cast<HWND>(window->GetNativeObject());
441+
if (IsWindow(hwnd)) {
442+
DestroyWindow(hwnd);
443443
}
444-
return false;
444+
WindowRegistry::GetInstance().Remove(id);
445+
return true;
445446
}
446447

447448
std::shared_ptr<Window> WindowManager::Get(WindowId id) {
448-
auto it = windows_.find(id);
449-
if (it != windows_.end()) {
450-
return it->second;
449+
auto cached = WindowRegistry::GetInstance().Get(id);
450+
if (cached) {
451+
return cached;
451452
}
452-
453453
// Check if the window still exists in the system
454454
HWND hwnd = reinterpret_cast<HWND>(id);
455455
if (IsWindow(hwnd)) {
456456
auto window = std::make_shared<Window>(hwnd);
457-
windows_[id] = window;
457+
WindowRegistry::GetInstance().Add(id, window);
458458
return window;
459459
}
460-
461460
return nullptr;
462461
}
463462

464463
std::vector<std::shared_ptr<Window>> WindowManager::GetAll() {
465-
std::vector<std::shared_ptr<Window>> windows;
466-
467-
// Enumerate all windows and add them to our collection
468-
// This is a simplified implementation
469-
for (auto& window_pair : windows_) {
470-
windows.push_back(window_pair.second);
471-
}
472-
473-
return windows;
464+
return WindowRegistry::GetInstance().GetAll();
474465
}
475466

476467
std::shared_ptr<Window> WindowManager::GetCurrent() {

src/window_manager.h

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -205,13 +205,7 @@ class WindowManager : public EventEmitter<WindowEvent> {
205205
class Impl;
206206
std::unique_ptr<Impl> pimpl_;
207207

208-
/**
209-
* @brief Internal registry of all managed windows
210-
*
211-
* Maps window IDs to their corresponding Window instances for fast lookup.
212-
* This container is the authoritative source for all active windows.
213-
*/
214-
std::unordered_map<WindowId, std::shared_ptr<Window>> windows_;
208+
// Window instances are tracked by WindowRegistry (see window_registry.h)
215209

216210
/**
217211
* @brief Set up platform-specific event monitoring

0 commit comments

Comments
 (0)