Skip to content

Commit 9b58825

Browse files
committed
Refactor macOS menu and tray icon target classes
Renamed Objective-C target and delegate classes for menu and tray icon handling to improve clarity and consistency. Added exception safety and event listener management for menu item submenus. Updated usage throughout implementation to match new class names and improved resource cleanup.
1 parent 70cd711 commit 9b58825

File tree

2 files changed

+195
-89
lines changed

2 files changed

+195
-89
lines changed

src/platform/macos/menu_macos.mm

Lines changed: 149 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,12 @@
2222
typedef void (^MenuOpenedBlock)(nativeapi::MenuID menu_id);
2323
typedef void (^MenuClosedBlock)(nativeapi::MenuID menu_id);
2424

25-
@interface MenuItemTarget : NSObject
25+
@interface NSMenuItemTarget : NSObject
2626
@property(nonatomic, copy) MenuItemClickedBlock clickedBlock;
2727
- (void)menuItemClicked:(id)sender;
2828
@end
2929

30-
@interface MenuDelegate : NSObject <NSMenuDelegate>
30+
@interface NSMenuDelegateImpl : NSObject <NSMenuDelegate>
3131
@property(nonatomic, copy) MenuOpenedBlock openedBlock;
3232
@property(nonatomic, copy) MenuClosedBlock closedBlock;
3333
@end
@@ -118,45 +118,71 @@ @interface MenuDelegate : NSObject <NSMenuDelegate>
118118

119119
} // namespace nativeapi
120120

121-
// Implementation of MenuItemTarget - moved to global scope
122-
@implementation MenuItemTarget
121+
// Implementation of NSMenuItemTarget - moved to global scope
122+
@implementation NSMenuItemTarget
123123
- (void)menuItemClicked:(id)sender {
124-
NSMenuItem* menuItem = (NSMenuItem*)sender;
124+
@try {
125+
NSMenuItem* menuItem = (NSMenuItem*)sender;
126+
if (!menuItem)
127+
return;
125128

126-
// Call the block if it exists
127-
if (_clickedBlock) {
128-
std::string itemText = [[menuItem title] UTF8String];
129-
// Get the MenuItemID from the menu item's associated object
130-
NSNumber* itemIdObj = objc_getAssociatedObject(menuItem, kMenuItemIdKey);
131-
if (itemIdObj) {
132-
nativeapi::MenuItemID itemId = [itemIdObj longValue];
133-
_clickedBlock(itemId, itemText);
129+
// Call the block if it exists
130+
if (_clickedBlock) {
131+
NSString* title = [menuItem title];
132+
std::string itemText = title ? [title UTF8String] : "";
133+
134+
// Get the MenuItemID from the menu item's associated object
135+
NSNumber* itemIdObj = objc_getAssociatedObject(menuItem, kMenuItemIdKey);
136+
if (itemIdObj) {
137+
nativeapi::MenuItemID itemId = [itemIdObj longValue];
138+
_clickedBlock(itemId, itemText);
139+
}
134140
}
141+
} @catch (NSException* exception) {
142+
// Log the exception but don't crash
143+
NSLog(@"Exception in menuItemClicked: %@", [exception reason]);
135144
}
136145
}
146+
137147
@end
138148

139-
// Implementation of MenuDelegate - moved to global scope
140-
@implementation MenuDelegate
149+
// Implementation of NSMenuDelegateImpl - moved to global scope
150+
@implementation NSMenuDelegateImpl
141151
- (void)menuWillOpen:(NSMenu*)menu {
142-
if (_openedBlock) {
143-
// Get the MenuID from the menu's associated object
144-
NSNumber* menuIdObj = objc_getAssociatedObject(menu, kMenuIdKey);
145-
if (menuIdObj) {
146-
nativeapi::MenuID menuId = [menuIdObj longValue];
147-
_openedBlock(menuId);
152+
@try {
153+
if (!menu)
154+
return;
155+
156+
if (_openedBlock) {
157+
// Get the MenuID from the menu's associated object
158+
NSNumber* menuIdObj = objc_getAssociatedObject(menu, kMenuIdKey);
159+
if (menuIdObj) {
160+
nativeapi::MenuID menuId = [menuIdObj longValue];
161+
_openedBlock(menuId);
162+
}
148163
}
164+
} @catch (NSException* exception) {
165+
// Log the exception but don't crash
166+
NSLog(@"Exception in menuWillOpen: %@", [exception reason]);
149167
}
150168
}
151169

152170
- (void)menuDidClose:(NSMenu*)menu {
153-
if (_closedBlock) {
154-
// Get the MenuID from the menu's associated object
155-
NSNumber* menuIdObj = objc_getAssociatedObject(menu, kMenuIdKey);
156-
if (menuIdObj) {
157-
nativeapi::MenuID menuId = [menuIdObj longValue];
158-
_closedBlock(menuId);
171+
@try {
172+
if (!menu)
173+
return;
174+
175+
if (_closedBlock) {
176+
// Get the MenuID from the menu's associated object
177+
NSNumber* menuIdObj = objc_getAssociatedObject(menu, kMenuIdKey);
178+
if (menuIdObj) {
179+
nativeapi::MenuID menuId = [menuIdObj longValue];
180+
_closedBlock(menuId);
181+
}
159182
}
183+
} @catch (NSException* exception) {
184+
// Log the exception but don't crash
185+
NSLog(@"Exception in menuDidClose: %@", [exception reason]);
160186
}
161187
}
162188
@end
@@ -167,7 +193,7 @@ - (void)menuDidClose:(NSMenu*)menu {
167193
class MenuItem::Impl {
168194
public:
169195
NSMenuItem* ns_menu_item_;
170-
MenuItemTarget* target_;
196+
NSMenuItemTarget* ns_menu_item_target_;
171197
MenuItemType type_;
172198
std::optional<std::string> text_;
173199
std::optional<std::string> icon_;
@@ -179,35 +205,51 @@ - (void)menuDidClose:(NSMenu*)menu {
179205
MenuItemState state_;
180206
int radio_group_;
181207
std::shared_ptr<Menu> submenu_;
208+
size_t submenu_opened_listener_id_;
209+
size_t submenu_closed_listener_id_;
210+
MenuItemID menu_item_id_;
182211

183212
Impl(NSMenuItem* menu_item, MenuItemType type)
184213
: ns_menu_item_(menu_item),
185-
target_([[MenuItemTarget alloc] init]),
214+
ns_menu_item_target_([[NSMenuItemTarget alloc] init]),
186215
type_(type),
187216
accelerator_("", KeyboardAccelerator::None),
188217
has_accelerator_(false),
189218
enabled_(true),
190219
visible_(true),
191220
state_(MenuItemState::Unchecked),
192-
radio_group_(-1) {
193-
[ns_menu_item_ setTarget:target_];
221+
radio_group_(-1),
222+
submenu_opened_listener_id_(0),
223+
submenu_closed_listener_id_(0),
224+
menu_item_id_(-1) {
225+
[ns_menu_item_ setTarget:ns_menu_item_target_];
194226
[ns_menu_item_ setAction:@selector(menuItemClicked:)];
195227
}
196228

197229
~Impl() {
198-
// First, clean up submenu reference
230+
// First, remove submenu listeners before cleaning up submenu reference
231+
if (submenu_ && submenu_opened_listener_id_ != 0) {
232+
submenu_->RemoveListener(submenu_opened_listener_id_);
233+
submenu_opened_listener_id_ = 0;
234+
}
235+
if (submenu_ && submenu_closed_listener_id_ != 0) {
236+
submenu_->RemoveListener(submenu_closed_listener_id_);
237+
submenu_closed_listener_id_ = 0;
238+
}
239+
240+
// Then clean up submenu reference
199241
if (submenu_) {
200242
submenu_.reset();
201243
}
202244

203-
if (target_) {
245+
if (ns_menu_item_target_) {
204246
// Clean up blocks first
205-
target_.clickedBlock = nil;
247+
ns_menu_item_target_.clickedBlock = nil;
206248

207249
// Remove target and action to prevent callbacks after destruction
208250
[ns_menu_item_ setTarget:nil];
209251
[ns_menu_item_ setAction:nil];
210-
target_ = nil;
252+
ns_menu_item_target_ = nil;
211253
}
212254
}
213255
};
@@ -232,12 +274,13 @@ - (void)menuDidClose:(NSMenu*)menu {
232274
}
233275

234276
pimpl_ = std::make_unique<Impl>(nsItem, type);
277+
pimpl_->menu_item_id_ = id;
235278
objc_setAssociatedObject(nsItem, kMenuItemIdKey, [NSNumber numberWithLong:id],
236279
OBJC_ASSOCIATION_RETAIN_NONATOMIC);
237280
pimpl_->text_ = text.empty() ? std::nullopt : std::optional<std::string>(text);
238281

239282
// 设置默认的 Block 处理器,直接发送事件
240-
pimpl_->target_.clickedBlock = ^(MenuItemID item_id, const std::string& item_text) {
283+
pimpl_->ns_menu_item_target_.clickedBlock = ^(MenuItemID item_id, const std::string& item_text) {
241284
try {
242285
EmitSync<MenuItemClickedEvent>(item_id, item_text);
243286
} catch (...) {
@@ -250,11 +293,12 @@ - (void)menuDidClose:(NSMenu*)menu {
250293
: id(g_next_menu_item_id++),
251294
pimpl_(std::make_unique<Impl>((__bridge NSMenuItem*)native_item, MenuItemType::Normal)) {
252295
NSMenuItem* nsItem = (__bridge NSMenuItem*)native_item;
296+
pimpl_->menu_item_id_ = id;
253297
objc_setAssociatedObject(nsItem, kMenuItemIdKey, [NSNumber numberWithLong:id],
254298
OBJC_ASSOCIATION_RETAIN_NONATOMIC);
255299

256300
// 设置默认的 Block 处理器,直接发送事件
257-
pimpl_->target_.clickedBlock = ^(MenuItemID item_id, const std::string& item_text) {
301+
pimpl_->ns_menu_item_target_.clickedBlock = ^(MenuItemID item_id, const std::string& item_text) {
258302
try {
259303
EmitSync<MenuItemClickedEvent>(item_id, item_text);
260304
} catch (...) {
@@ -422,7 +466,7 @@ - (void)menuDidClose:(NSMenu*)menu {
422466
if (sibling == pimpl_->ns_menu_item_)
423467
continue;
424468
NSObject* targetObj = [sibling target];
425-
if ([targetObj isKindOfClass:[MenuItemTarget class]]) {
469+
if ([targetObj isKindOfClass:[NSMenuItemTarget class]]) {
426470
// Get the MenuItemID from the associated object
427471
NSNumber* siblingIdObj = objc_getAssociatedObject(sibling, kMenuItemIdKey);
428472
if (siblingIdObj) {
@@ -452,11 +496,59 @@ - (void)menuDidClose:(NSMenu*)menu {
452496
}
453497

454498
void MenuItem::SetSubmenu(std::shared_ptr<Menu> submenu) {
455-
pimpl_->submenu_ = submenu;
456-
if (submenu) {
457-
[pimpl_->ns_menu_item_ setSubmenu:(__bridge NSMenu*)submenu->GetNativeObject()];
458-
} else {
459-
[pimpl_->ns_menu_item_ setSubmenu:nil];
499+
try {
500+
pimpl_->submenu_ = submenu;
501+
if (submenu) {
502+
NSMenu* nsSubmenu = (__bridge NSMenu*)submenu->GetNativeObject();
503+
if (nsSubmenu) {
504+
[pimpl_->ns_menu_item_ setSubmenu:nsSubmenu];
505+
506+
// Remove previous submenu listeners if they exist
507+
if (pimpl_->submenu_opened_listener_id_ != 0) {
508+
submenu->RemoveListener(pimpl_->submenu_opened_listener_id_);
509+
pimpl_->submenu_opened_listener_id_ = 0;
510+
}
511+
if (pimpl_->submenu_closed_listener_id_ != 0) {
512+
submenu->RemoveListener(pimpl_->submenu_closed_listener_id_);
513+
pimpl_->submenu_closed_listener_id_ = 0;
514+
}
515+
516+
// Add event listeners to forward submenu events
517+
MenuItemID menu_item_id = id;
518+
MenuItem* self = this;
519+
pimpl_->submenu_opened_listener_id_ = submenu->AddListener<MenuOpenedEvent>(
520+
[self, menu_item_id](const MenuOpenedEvent& event) {
521+
try {
522+
self->EmitSync<MenuItemSubmenuOpenedEvent>(menu_item_id);
523+
} catch (...) {
524+
// Protect against event emission exceptions
525+
}
526+
});
527+
528+
pimpl_->submenu_closed_listener_id_ = submenu->AddListener<MenuClosedEvent>(
529+
[self, menu_item_id](const MenuClosedEvent& event) {
530+
try {
531+
self->EmitSync<MenuItemSubmenuClosedEvent>(menu_item_id);
532+
} catch (...) {
533+
// Protect against event emission exceptions
534+
}
535+
});
536+
}
537+
} else {
538+
// Remove listeners when submenu is cleared
539+
if (pimpl_->submenu_ && pimpl_->submenu_opened_listener_id_ != 0) {
540+
pimpl_->submenu_->RemoveListener(pimpl_->submenu_opened_listener_id_);
541+
pimpl_->submenu_opened_listener_id_ = 0;
542+
}
543+
if (pimpl_->submenu_ && pimpl_->submenu_closed_listener_id_ != 0) {
544+
pimpl_->submenu_->RemoveListener(pimpl_->submenu_closed_listener_id_);
545+
pimpl_->submenu_closed_listener_id_ = 0;
546+
}
547+
[pimpl_->ns_menu_item_ setSubmenu:nil];
548+
}
549+
} catch (...) {
550+
// Handle C++ exceptions
551+
NSLog(@"Exception in SetSubmenu");
460552
}
461553
}
462554

@@ -465,6 +557,16 @@ - (void)menuDidClose:(NSMenu*)menu {
465557
}
466558

467559
void MenuItem::RemoveSubmenu() {
560+
// Remove listeners before clearing submenu reference
561+
if (pimpl_->submenu_ && pimpl_->submenu_opened_listener_id_ != 0) {
562+
pimpl_->submenu_->RemoveListener(pimpl_->submenu_opened_listener_id_);
563+
pimpl_->submenu_opened_listener_id_ = 0;
564+
}
565+
if (pimpl_->submenu_ && pimpl_->submenu_closed_listener_id_ != 0) {
566+
pimpl_->submenu_->RemoveListener(pimpl_->submenu_closed_listener_id_);
567+
pimpl_->submenu_closed_listener_id_ = 0;
568+
}
569+
468570
pimpl_->submenu_.reset();
469571
[pimpl_->ns_menu_item_ setSubmenu:nil];
470572
}
@@ -474,9 +576,9 @@ - (void)menuDidClose:(NSMenu*)menu {
474576
return false;
475577

476578
// Call the block directly instead of going through target-action
477-
if (pimpl_->target_.clickedBlock) {
579+
if (pimpl_->ns_menu_item_target_.clickedBlock) {
478580
std::string itemText = pimpl_->text_.value_or("");
479-
pimpl_->target_.clickedBlock(id, itemText);
581+
pimpl_->ns_menu_item_target_.clickedBlock(id, itemText);
480582
}
481583
return true;
482584
}
@@ -489,13 +591,13 @@ - (void)menuDidClose:(NSMenu*)menu {
489591
class Menu::Impl {
490592
public:
491593
NSMenu* ns_menu_;
492-
MenuDelegate* delegate_;
594+
NSMenuDelegateImpl* delegate_;
493595
std::vector<std::shared_ptr<MenuItem>> items_;
494596
bool enabled_;
495597
bool visible_;
496598

497599
Impl(NSMenu* menu)
498-
: ns_menu_(menu), delegate_([[MenuDelegate alloc] init]), enabled_(true), visible_(false) {
600+
: ns_menu_(menu), delegate_([[NSMenuDelegateImpl alloc] init]), enabled_(true), visible_(false) {
499601
[ns_menu_ setDelegate:delegate_];
500602
}
501603

0 commit comments

Comments
 (0)