From ac5543936490771850fd8563cf255fe18dc6c409 Mon Sep 17 00:00:00 2001 From: John Scipione Date: Tue, 18 Feb 2025 19:46:02 -0500 Subject: [PATCH] Interface Kit: BWindow owns (and deletes) menu sem _Uninstall() before _Hide() in BMenu because the window must be available when we _Uninstall() especially for shortcuts. Remove #define and always assume USE_CACHED_MENUWINDOW. _Uninstall() whenever we _Hide() in BMenu. Change-Id: I5dae85f6edf1f0b4ccf67a6d9d77470576671cee Reviewed-on: https://review.haiku-os.org/c/haiku/+/9012 Tested-by: Commit checker robot Reviewed-by: waddlesplash --- headers/os/interface/MenuBar.h | 3 +-- src/kits/interface/Menu.cpp | 22 ++++++++-------------- src/kits/interface/MenuBar.cpp | 14 +++----------- src/kits/interface/PopUpMenu.cpp | 3 --- src/kits/interface/Window.cpp | 19 +++++++++++++++++-- 5 files changed, 29 insertions(+), 32 deletions(-) diff --git a/headers/os/interface/MenuBar.h b/headers/os/interface/MenuBar.h index b48c91aed9..3b1a78a06e 100644 --- a/headers/os/interface/MenuBar.h +++ b/headers/os/interface/MenuBar.h @@ -106,10 +106,9 @@ private: menu_bar_border fBorder; thread_id fTrackingPID; int32 fPrevFocusToken; - sem_id fMenuSem; BRect* fLastBounds; uint32 fBorders; - uint32 _reserved[1]; + uint32 _reserved[2]; bool fTracking; }; diff --git a/src/kits/interface/Menu.cpp b/src/kits/interface/Menu.cpp index df1322cdba..1af1f35580 100644 --- a/src/kits/interface/Menu.cpp +++ b/src/kits/interface/Menu.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2001-2018 Haiku, Inc. All rights reserved. + * Copyright 2001-2025 Haiku, Inc. All rights reserved. * Distributed under the terms of the MIT license. * * Authors: @@ -51,10 +51,9 @@ #include "utf8_functions.h" -#define USE_CACHED_MENUWINDOW 1 - using BPrivate::gSystemCatalog; + #undef B_TRANSLATION_CONTEXT #define B_TRANSLATION_CONTEXT "Menu" @@ -1392,8 +1391,8 @@ BMenu::Show(bool selectFirst) void BMenu::Hide() { - _Hide(); _Uninstall(); + _Hide(); } @@ -1676,11 +1675,9 @@ BMenu::_Hide() window->DetachMenu(); // we don't want to be deleted when the window is removed -#if USE_CACHED_MENUWINDOW if (fSuper != NULL) window->Unlock(); else -#endif window->Quit(); // it's our window, quit it @@ -2965,13 +2962,12 @@ BMenu::_OverSubmenu(BMenuItem* item, BPoint loc) BMenuWindow* BMenu::_MenuWindow() { -#if USE_CACHED_MENUWINDOW if (fCachedMenuWindow == NULL) { char windowName[64]; snprintf(windowName, 64, "%s cached menu", Name()); fCachedMenuWindow = new (nothrow) BMenuWindow(windowName); } -#endif + return fCachedMenuWindow; } @@ -3060,17 +3056,15 @@ BMenu::_Uninstall() void -BMenu::_SelectItem(BMenuItem* item, bool showSubmenu, bool selectFirstItem, - bool keyDown) +BMenu::_SelectItem(BMenuItem* item, bool showSubmenu, bool selectFirstItem, bool keyDown) { - // Avoid deselecting and then reselecting the same item - // which would cause flickering + // Avoid deselecting and reselecting the same item which would cause flickering. if (item != fSelected) { if (fSelected != NULL) { fSelected->Select(false); BMenu* subMenu = fSelected->Submenu(); if (subMenu != NULL && subMenu->Window() != NULL) - subMenu->_Hide(); + subMenu->Hide(); } fSelected = item; @@ -3458,7 +3452,7 @@ BMenu::_QuitTracking(bool onlyThis) } } - _Hide(); + Hide(); } diff --git a/src/kits/interface/MenuBar.cpp b/src/kits/interface/MenuBar.cpp index 6a07dd23c4..bce90cb66c 100644 --- a/src/kits/interface/MenuBar.cpp +++ b/src/kits/interface/MenuBar.cpp @@ -52,7 +52,6 @@ BMenuBar::BMenuBar(BRect frame, const char* name, uint32 resizingMode, fBorder(B_BORDER_FRAME), fTrackingPID(-1), fPrevFocusToken(-1), - fMenuSem(-1), fLastBounds(NULL), fTracking(false) { @@ -68,7 +67,6 @@ BMenuBar::BMenuBar(const char* name, menu_layout layout, uint32 flags) fBorder(B_BORDER_FRAME), fTrackingPID(-1), fPrevFocusToken(-1), - fMenuSem(-1), fLastBounds(NULL), fTracking(false) { @@ -82,7 +80,6 @@ BMenuBar::BMenuBar(BMessage* archive) fBorder(B_BORDER_FRAME), fTrackingPID(-1), fPrevFocusToken(-1), - fMenuSem(-1), fLastBounds(NULL), fTracking(false) { @@ -492,11 +489,9 @@ BMenuBar::StartMenuBar(int32 menuIndex, bool sticky, bool showMenu, // so let's call MenusBeginning() directly window->MenusBeginning(); - fMenuSem = create_sem(0, "window close sem"); - _set_menu_sem_(window, fMenuSem); - - fTrackingPID = spawn_thread(_TrackTask, "menu_tracking", - B_DISPLAY_PRIORITY, NULL); + sem_id sem = create_sem(0, "window close sem"); + _set_menu_sem_(window, sem); + fTrackingPID = spawn_thread(_TrackTask, "menu_tracking", B_DISPLAY_PRIORITY, NULL); if (fTrackingPID >= 0) { menubar_data data; data.menuBar = this; @@ -512,7 +507,6 @@ BMenuBar::StartMenuBar(int32 menuIndex, bool sticky, bool showMenu, } else { fTracking = false; _set_menu_sem_(window, B_NO_MORE_SEMS); - delete_sem(fMenuSem); } } @@ -540,8 +534,6 @@ BMenuBar::_TrackTask(void* arg) window->PostMessage(_MENUS_DONE_); _set_menu_sem_(window, B_BAD_SEM_ID); - delete_sem(menuBar->fMenuSem); - menuBar->fMenuSem = B_BAD_SEM_ID; return 0; } diff --git a/src/kits/interface/PopUpMenu.cpp b/src/kits/interface/PopUpMenu.cpp index f2a4e24076..9dd818e2c8 100644 --- a/src/kits/interface/PopUpMenu.cpp +++ b/src/kits/interface/PopUpMenu.cpp @@ -375,7 +375,6 @@ BPopUpMenu::_Go(BPoint where, bool autoInvoke, bool startOpened, fTrackThread = spawn_thread(_thread_entry, "popup", B_DISPLAY_PRIORITY, data); if (fTrackThread < B_OK) { // Something went wrong. Cleanup and return NULL - delete_sem(sem); if (async && window != NULL) _set_menu_sem_(window, B_BAD_SEM_ID); delete data; @@ -409,8 +408,6 @@ BPopUpMenu::_thread_entry(void* menuData) if (data->async && data->window) _set_menu_sem_(data->window, B_BAD_SEM_ID); - delete_sem(data->lock); - // Commit suicide if needed if (data->async && menu->fAutoDestruct) { menu->fTrackThread = -1; diff --git a/src/kits/interface/Window.cpp b/src/kits/interface/Window.cpp index a72becb807..f0c102e6f4 100644 --- a/src/kits/interface/Window.cpp +++ b/src/kits/interface/Window.cpp @@ -217,8 +217,23 @@ static value_info sWindowValueInfo[] = { void _set_menu_sem_(BWindow* window, sem_id sem) { - if (window != NULL) - window->fMenuSem = sem; + if (window == NULL) + return; + + // delete semaphore when set to invalid + switch (sem) { + case B_BAD_SEM_ID: + case B_NO_MORE_SEMS: + case -1: + if (window->fMenuSem > 0) + delete_sem(window->fMenuSem); + break; + + default: + break; + } + + window->fMenuSem = sem; }