Skip to content

Commit 3fff1a4

Browse files
committed
Refactor menu and tray icon to use shared host window
Menus and tray icons now use a shared hidden host window provided by WindowMessageDispatcher, improving resource management and message routing. Removed redundant window creation and destruction logic from tray icon implementation. Added GetHostWindow method to WindowMessageDispatcher and updated related logic for handler registration and hook management.
1 parent 933dda7 commit 3fff1a4

File tree

4 files changed

+143
-142
lines changed

4 files changed

+143
-142
lines changed

src/platform/windows/menu_windows.cpp

Lines changed: 13 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include "../../menu.h"
99
#include "../../menu_event.h"
1010
#include "string_utils_windows.h"
11+
#include "window_message_dispatcher.h"
1112

1213
namespace nativeapi {
1314

@@ -86,74 +87,6 @@ std::pair<UINT, UINT> ConvertAccelerator(
8687
return std::make_pair(key, modifiers);
8788
}
8889

89-
// Helper function to get the main window for the current thread
90-
HWND GetMainWindowForCurrentThread() {
91-
static HWND main_window = nullptr;
92-
93-
// First try to get the foreground window
94-
HWND hwnd = nullptr;
95-
96-
// Use EnumWindows to find the main window
97-
struct EnumData {
98-
HWND main_window;
99-
DWORD current_thread_id;
100-
};
101-
102-
EnumData data = {nullptr, GetCurrentThreadId()};
103-
104-
EnumWindows(
105-
[](HWND hwnd, LPARAM lParam) -> BOOL {
106-
EnumData* data = reinterpret_cast<EnumData*>(lParam);
107-
108-
// Check if window belongs to current thread
109-
DWORD window_thread_id = GetWindowThreadProcessId(hwnd, nullptr);
110-
if (window_thread_id != data->current_thread_id) {
111-
return TRUE; // Continue enumeration
112-
}
113-
114-
// Check if window is visible and not a tool window
115-
if (!IsWindowVisible(hwnd)) {
116-
return TRUE; // Continue enumeration
117-
}
118-
119-
// Skip tool windows
120-
if (GetWindowLong(hwnd, GWL_EXSTYLE) & WS_EX_TOOLWINDOW) {
121-
return TRUE; // Continue enumeration
122-
}
123-
124-
// Skip message-only windows
125-
if (hwnd == HWND_MESSAGE) {
126-
return TRUE; // Continue enumeration
127-
}
128-
129-
// Check if it's a main window (has a title bar, not a child window)
130-
if (GetParent(hwnd) != nullptr) {
131-
return TRUE; // Continue enumeration
132-
}
133-
134-
// Found a candidate main window
135-
data->main_window = hwnd;
136-
return FALSE; // Stop enumeration
137-
},
138-
reinterpret_cast<LPARAM>(&data));
139-
140-
if (data.main_window) {
141-
return data.main_window;
142-
}
143-
144-
// Fallback: try to find any top-level window
145-
hwnd = FindWindow(nullptr, nullptr);
146-
if (hwnd && IsWindow(hwnd)) {
147-
return hwnd;
148-
}
149-
150-
return nullptr;
151-
}
152-
153-
} // namespace nativeapi
154-
155-
namespace nativeapi {
156-
15790
// MenuItem::Impl implementation
15891
class MenuItem::Impl {
15992
public:
@@ -550,12 +483,20 @@ bool Menu::Open(double x, double y) {
550483
pimpl_->visible_ = true;
551484

552485
POINT pt = {static_cast<int>(x), static_cast<int>(y)};
553-
// Use the helper function to get the main window for current thread
554-
HWND hwnd = GetMainWindowForCurrentThread();
555486

556-
// Show the context menu
487+
// Get the host window for menus
488+
HWND host_window = WindowMessageDispatcher::GetInstance().GetHostWindow();
489+
if (!host_window) {
490+
pimpl_->visible_ = false;
491+
return false;
492+
}
493+
494+
// Set the host window as foreground to ensure menu can be displayed
495+
SetForegroundWindow(host_window);
496+
497+
// Show the context menu using the host window
557498
TrackPopupMenu(pimpl_->hmenu_, TPM_BOTTOMALIGN | TPM_LEFTALIGN, pt.x, pt.y, 0,
558-
hwnd, nullptr);
499+
host_window, nullptr);
559500

560501
pimpl_->visible_ = false;
561502
return true;

src/platform/windows/tray_icon_windows.cpp

Lines changed: 5 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -33,18 +33,16 @@ class TrayIcon::Impl {
3333
using RightClickedCallback = std::function<void(TrayIconId)>;
3434
using DoubleClickedCallback = std::function<void(TrayIconId)>;
3535

36-
Impl() : hwnd_(nullptr), icon_handle_(nullptr), owns_window_(false) {
36+
Impl() : hwnd_(nullptr), icon_handle_(nullptr) {
3737
tray_icon_id_ = IdAllocator::Allocate<TrayIcon>();
3838
}
3939

4040
Impl(HWND hwnd,
41-
bool owns_window,
4241
ClickedCallback clicked_callback,
4342
RightClickedCallback right_clicked_callback,
4443
DoubleClickedCallback double_clicked_callback)
4544
: hwnd_(hwnd),
4645
icon_handle_(nullptr),
47-
owns_window_(owns_window),
4846
clicked_callback_(std::move(clicked_callback)),
4947
right_clicked_callback_(std::move(right_clicked_callback)),
5048
double_clicked_callback_(std::move(double_clicked_callback)) {
@@ -71,10 +69,7 @@ class TrayIcon::Impl {
7169

7270
if (hwnd_) {
7371
Shell_NotifyIconW(NIM_DELETE, &nid_);
74-
// Destroy the window if we own it
75-
if (owns_window_) {
76-
DestroyWindow(hwnd_);
77-
}
72+
// Note: We don't destroy the shared host window
7873
}
7974
if (icon_handle_) {
8075
DestroyIcon(icon_handle_);
@@ -121,7 +116,6 @@ class TrayIcon::Impl {
121116
std::shared_ptr<Menu> context_menu_;
122117
HICON icon_handle_;
123118
TrayIconId tray_icon_id_;
124-
bool owns_window_; // Whether we created the window and should destroy it
125119

126120
// Callback functions for event emission
127121
ClickedCallback clicked_callback_;
@@ -133,45 +127,10 @@ TrayIcon::TrayIcon() : TrayIcon(nullptr) {}
133127

134128
TrayIcon::TrayIcon(void* native_tray_icon) {
135129
HWND hwnd = nullptr;
136-
bool owns_window = false;
137130

138131
if (native_tray_icon == nullptr) {
139-
// Create a new tray icon - need a window handle for receiving messages
140-
HINSTANCE hInstance = GetModuleHandle(nullptr);
141-
142-
// Register window class for message-only window (once per process)
143-
static bool class_registered = false;
144-
static std::wstring class_name = L"NativeAPITrayIconWindow";
145-
146-
if (!class_registered) {
147-
WNDCLASSW wc = {};
148-
// Use WindowMessageDispatcher to route messages to registered handlers
149-
wc.lpfnWndProc = WindowMessageDispatcher::DispatchWindowProc;
150-
wc.hInstance = hInstance;
151-
wc.lpszClassName = class_name.c_str();
152-
153-
if (RegisterClassW(&wc)) {
154-
class_registered = true;
155-
}
156-
}
157-
158-
// Create a message-only window (HWND_MESSAGE as parent)
159-
if (class_registered) {
160-
hwnd = CreateWindowExW(
161-
0, // Extended styles
162-
class_name.c_str(), // Window class
163-
L"", // Window title (empty for message-only window)
164-
0, // Window style
165-
0, 0, 0, 0, // Position and size (ignored for message-only)
166-
HWND_MESSAGE, // Parent: message-only window
167-
nullptr, // Menu
168-
hInstance, // Instance
169-
nullptr); // Additional data
170-
171-
if (hwnd) {
172-
owns_window = true;
173-
}
174-
}
132+
// Use the shared host window from WindowMessageDispatcher
133+
hwnd = WindowMessageDispatcher::GetInstance().GetHostWindow();
175134
} else {
176135
// Wrap existing native tray icon
177136
// In a real implementation, you'd extract HWND from the tray parameter
@@ -196,7 +155,7 @@ TrayIcon::TrayIcon(void* native_tray_icon) {
196155
};
197156

198157
pimpl_ = std::make_unique<Impl>(
199-
hwnd, owns_window, std::move(clicked_callback),
158+
hwnd, std::move(clicked_callback),
200159
std::move(right_clicked_callback), std::move(double_clicked_callback));
201160
} else {
202161
// Failed to create window, create uninitialized Impl

src/platform/windows/window_message_dispatcher.cpp

Lines changed: 113 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ WindowMessageDispatcher& WindowMessageDispatcher::GetInstance() {
1212
WindowMessageDispatcher::~WindowMessageDispatcher() {
1313
std::lock_guard<std::mutex> lock(mutex_);
1414

15+
// Destroy host window if it exists
16+
if (host_window_) {
17+
DestroyWindow(host_window_);
18+
host_window_ = nullptr;
19+
}
20+
1521
// Uninstall all hooks before destruction
1622
for (const auto& [hwnd, _] : original_procs_) {
1723
UninstallHook(hwnd);
@@ -29,42 +35,57 @@ int WindowMessageDispatcher::RegisterHandler(WindowMessageHandler handler) {
2935

3036
int WindowMessageDispatcher::RegisterHandler(HWND hwnd,
3137
WindowMessageHandler handler) {
32-
std::lock_guard<std::mutex> lock(mutex_);
33-
34-
int id = next_id_++;
35-
handlers_[id] = {std::move(handler), hwnd};
38+
// Check if hook needs to be installed (outside of lock to avoid deadlock)
39+
bool needs_hook = false;
40+
{
41+
std::lock_guard<std::mutex> lock(mutex_);
42+
needs_hook = (original_procs_.find(hwnd) == original_procs_.end());
43+
}
3644

37-
// Install hook for this window if not already installed
38-
if (original_procs_.find(hwnd) == original_procs_.end()) {
45+
// Install hook if needed (outside of lock)
46+
if (needs_hook) {
3947
InstallHook(hwnd);
4048
}
4149

50+
// Register the handler (inside lock)
51+
std::lock_guard<std::mutex> lock(mutex_);
52+
int id = next_id_++;
53+
handlers_[id] = {std::move(handler), hwnd};
54+
4255
return id;
4356
}
4457

4558
bool WindowMessageDispatcher::UnregisterHandler(int id) {
46-
std::lock_guard<std::mutex> lock(mutex_);
59+
HWND target_hwnd = HWND(0);
60+
bool should_uninstall = false;
4761

48-
auto it = handlers_.find(id);
49-
if (it == handlers_.end()) {
50-
return false;
51-
}
62+
{
63+
std::lock_guard<std::mutex> lock(mutex_);
64+
65+
auto it = handlers_.find(id);
66+
if (it == handlers_.end()) {
67+
return false;
68+
}
5269

53-
HWND target_hwnd = it->second.target_hwnd;
54-
handlers_.erase(it);
70+
target_hwnd = it->second.target_hwnd;
71+
handlers_.erase(it);
5572

56-
// Check if this was the last handler for this window
57-
if (target_hwnd != HWND(0)) {
58-
bool has_other_handlers = std::any_of(
59-
handlers_.begin(), handlers_.end(), [target_hwnd](const auto& pair) {
60-
return pair.second.target_hwnd == target_hwnd;
61-
});
73+
// Check if this was the last handler for this window
74+
if (target_hwnd != HWND(0)) {
75+
bool has_other_handlers = std::any_of(
76+
handlers_.begin(), handlers_.end(), [target_hwnd](const auto& pair) {
77+
return pair.second.target_hwnd == target_hwnd;
78+
});
6279

63-
if (!has_other_handlers) {
64-
UninstallHook(target_hwnd);
80+
should_uninstall = !has_other_handlers;
6581
}
6682
}
6783

84+
// Uninstall hook if needed (outside of lock to avoid deadlock)
85+
if (should_uninstall) {
86+
UninstallHook(target_hwnd);
87+
}
88+
6889
return true;
6990
}
7091

@@ -125,6 +146,11 @@ bool WindowMessageDispatcher::InstallHook(HWND hwnd) {
125146
return false;
126147
}
127148

149+
// If the window already has DispatchWindowProc, don't install it again
150+
if (current_proc == DispatchWindowProc) {
151+
return true; // Already installed
152+
}
153+
128154
// Store original procedure
129155
original_procs_[hwnd] = current_proc;
130156

@@ -143,12 +169,75 @@ void WindowMessageDispatcher::UninstallHook(HWND hwnd) {
143169

144170
WNDPROC original_proc = it->second;
145171

146-
// Restore original window procedure
147-
SetWindowLongPtr(hwnd, GWLP_WNDPROC,
148-
reinterpret_cast<LONG_PTR>(original_proc));
172+
// Don't restore window procedure for host window - it should keep DispatchWindowProc
173+
if (hwnd != host_window_) {
174+
// Restore original window procedure
175+
SetWindowLongPtr(hwnd, GWLP_WNDPROC,
176+
reinterpret_cast<LONG_PTR>(original_proc));
177+
}
149178

150179
// Remove from our tracking
151180
original_procs_.erase(it);
152181
}
153182

183+
HWND WindowMessageDispatcher::GetHostWindow() {
184+
// Check if host window already exists (without holding lock to avoid deadlock)
185+
if (host_window_ && IsWindow(host_window_)) {
186+
return host_window_;
187+
}
188+
189+
// Create a hidden window class for hosting
190+
static const wchar_t* class_name = L"NativeApiHostWindow";
191+
192+
WNDCLASSW wc = {};
193+
wc.lpfnWndProc = DispatchWindowProc; // Use dispatcher for host window
194+
wc.hInstance = GetModuleHandle(nullptr);
195+
wc.lpszClassName = class_name;
196+
197+
// Register the window class (only once)
198+
static bool class_registered = false;
199+
if (!class_registered) {
200+
if (RegisterClassW(&wc)) {
201+
class_registered = true;
202+
} else {
203+
return nullptr;
204+
}
205+
}
206+
207+
// Create the hidden host window (outside of lock to avoid deadlock)
208+
HWND new_host_window = CreateWindowExW(
209+
WS_EX_TOOLWINDOW, // Extended style: tool window
210+
class_name, // Window class
211+
L"NativeApi Host", // Window title
212+
WS_OVERLAPPED, // Window style: overlapped window
213+
0, 0, // Position
214+
1, 1, // Size (minimal)
215+
HWND_MESSAGE, // Parent: message-only window
216+
nullptr, // Menu
217+
GetModuleHandle(nullptr), // Instance
218+
nullptr // Additional data
219+
);
220+
221+
if (new_host_window) {
222+
// Ensure the window is hidden
223+
ShowWindow(new_host_window, SW_HIDE);
224+
225+
// Now acquire lock to register the window
226+
std::lock_guard<std::mutex> lock(mutex_);
227+
228+
// Double-check if another thread created the window while we were creating it
229+
if (host_window_ && IsWindow(host_window_)) {
230+
// Another thread won, destroy our window and use theirs
231+
DestroyWindow(new_host_window);
232+
return host_window_;
233+
}
234+
235+
// Register the host window in original_procs_ with DefWindowProc as fallback
236+
host_window_ = new_host_window;
237+
original_procs_[host_window_] = DefWindowProcW;
238+
}
239+
240+
return host_window_;
241+
}
242+
154243
} // namespace nativeapi

0 commit comments

Comments
 (0)