[webkit-reviews] review granted: [Bug 213950] [macOS] Tests that show system menu popups may fail when run concurrently : [Attachment 403505] For EWS

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 213950: [macOS] Tests that show system menu popups may fail when run
concurrently
https://bugs.webkit.org/show_bug.cgi?id=213950

Attachment 403505: For EWS

https://bugs.webkit.org/attachment.cgi?id=403505&action=review




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 403505
  --> https://bugs.webkit.org/attachment.cgi?id=403505
For EWS

View in context: https://bugs.webkit.org/attachment.cgi?id=403505&action=review

> Tools/WebKitTestRunner/mac/TestControllerMac.mm:134
> +    static NeverDestroyed<InstanceMethodSwizzler> cancelTrackingSwizzler {
NSMenu.class, @selector(cancelTracking),
reinterpret_cast<IMP>(swizzledCancelTracking) };
> +    static NeverDestroyed<ClassMethodSwizzler> menuPopUpSwizzler {
NSMenu.class, @selector(popUpContextMenu:withEvent:forView:),
reinterpret_cast<IMP>(swizzledPopUpContextMenu) };
> +    static NeverDestroyed<InstanceMethodSwizzler> carbonMenuPopUpSwizzler {

Do these really need NeverDestroyed? Maybe they would with ARC, but I don’t see
why they need it without ARC. Keep in mind that NeverDestroyed does not make
something stay alive for the duration of execution: being a global makes that
happen. NeverDestroyed simply prevents the destructor from being called, which
is useful for such objects if they have destructors.