[PATCH 1/3] user32: Store pointer to submenu struct instead of HMENU
Andrew Eikum
aeikum at codeweavers.com
Fri Sep 12 10:44:26 CDT 2014
This patch also adds internal reference counting to menu structs, as we
already have tests which depend on the struct remaining valid after the
HMENU is destroyed.
I know they're large, but I'm sending these first three patches
together to give the gist of what the entire patch series does. First
we add reference counting to the existing code, then we change our
internal uses of HMENU to use the pointer directly.
The ultimate goal is to allow submenus to be selected even after
they're Destroyed by the client. A later patch will remove the
todo_wine in <dlls/user32/tests/menu.c:test_subpopup_locked_by_menu>.
---
dlls/user32/menu.c | 159 +++++++++++++++++++++++++++++------------------
dlls/user32/tests/menu.c | 6 +-
2 files changed, 104 insertions(+), 61 deletions(-)
diff --git a/dlls/user32/menu.c b/dlls/user32/menu.c
index 9e05dd4..ae366cb 100644
--- a/dlls/user32/menu.c
+++ b/dlls/user32/menu.c
@@ -62,13 +62,15 @@
WINE_DEFAULT_DEBUG_CHANNEL(menu);
WINE_DECLARE_DEBUG_CHANNEL(accel);
+typedef struct _POPUPMENU POPUPMENU, *LPPOPUPMENU;
+
/* Menu item structure */
typedef struct {
/* ----------- MENUITEMINFO Stuff ----------- */
UINT fType; /* Item type. */
UINT fState; /* Item state. */
UINT_PTR wID; /* Item id. */
- HMENU hSubMenu; /* Pop-up menu. */
+ POPUPMENU *submenu; /* Pop-up menu. */
HBITMAP hCheckBit; /* Bitmap when checked. */
HBITMAP hUnCheckBit; /* Bitmap when unchecked. */
LPWSTR text; /* Item text. */
@@ -83,7 +85,7 @@ typedef struct {
} MENUITEM;
/* Popup menu structure */
-typedef struct {
+struct _POPUPMENU {
struct user_object obj;
WORD wFlags; /* Menu flags (MF_POPUP, MF_SYSMENU) */
WORD Width; /* Width of the whole menu */
@@ -97,6 +99,7 @@ typedef struct {
BOOL bScrolling; /* Scroll arrows are active */
UINT nScrollPos; /* Current scroll position */
UINT nTotalHeight; /* Total height of menu items inside menu */
+ LONG refcount;
/* ------------ MENUINFO members ------ */
DWORD dwStyle; /* Extended menu style */
UINT cyMax; /* max height of the whole menu, 0 is screen height */
@@ -105,7 +108,7 @@ typedef struct {
DWORD dwMenuData; /* application defined value */
HMENU hSysMenuOwner; /* Handle to the dummy sys menu holder */
WORD textOffset; /* Offset of text when items have both bitmaps and text */
-} POPUPMENU, *LPPOPUPMENU;
+};
/* internal flags for menu tracking */
@@ -179,6 +182,7 @@ static BOOL fEndMenu = FALSE;
DWORD WINAPI DrawMenuBarTemp(HWND hwnd, HDC hDC, LPRECT lprect, HMENU hMenu, HFONT hFont);
static BOOL SetMenuItemInfo_common( MENUITEM *, const MENUITEMINFOW *, BOOL);
+static void MENU_ReleaseMenu( POPUPMENU * );
/*********************************************************************
* menu class descriptor
@@ -193,6 +197,10 @@ const struct builtin_class_descr MENU_builtin_class =
(HBRUSH)(COLOR_MENU+1) /* brush */
};
+static HMENU MENU_GetHandle(const POPUPMENU *menu)
+{
+ return menu ? menu->obj.handle : NULL;
+}
/***********************************************************************
* debug_print_menuitem
@@ -222,8 +230,8 @@ static void do_debug_print_menuitem(const char *prefix, const MENUITEM *mp,
if (mp) {
UINT flags = mp->fType;
TRACE( "{ ID=0x%lx", mp->wID);
- if ( mp->hSubMenu)
- TRACE( ", Sub=%p", mp->hSubMenu);
+ if ( mp->submenu)
+ TRACE( ", Sub=%p", MENU_GetHandle(mp->submenu));
if (flags) {
int count = 0;
TRACE( ", fType=");
@@ -620,7 +628,7 @@ static MENUITEM *MENU_FindItem( HMENU *hmenu, UINT *nPos, UINT wFlags )
{
if (item->fType & MF_POPUP)
{
- HMENU hsubmenu = item->hSubMenu;
+ HMENU hsubmenu = MENU_GetHandle(item->submenu);
MENUITEM *subitem = MENU_FindItem( &hsubmenu, nPos, wFlags );
if (subitem)
{
@@ -666,11 +674,11 @@ static UINT MENU_FindSubMenu( HMENU *hmenu, HMENU hSubTarget )
item = menu->items;
for (i = 0; i < menu->nItems; i++, item++) {
if(!(item->fType & MF_POPUP)) continue;
- if (item->hSubMenu == hSubTarget) {
+ if (MENU_GetHandle(item->submenu) == hSubTarget) {
return i;
}
else {
- HMENU hsubmenu = item->hSubMenu;
+ HMENU hsubmenu = MENU_GetHandle(item->submenu);
UINT pos = MENU_FindSubMenu( &hsubmenu, hSubTarget );
if (pos != NO_SELECTED_ITEM) {
*hmenu = hsubmenu;
@@ -2252,7 +2260,7 @@ static HMENU MENU_GetSubPopup( HMENU hmenu )
item = &menu->items[menu->FocusedItem];
if ((item->fType & MF_POPUP) && (item->fState & MF_MOUSESELECT))
- return item->hSubMenu;
+ return MENU_GetHandle(item->submenu);
return 0;
}
@@ -2281,10 +2289,11 @@ static void MENU_HideSubPopups( HWND hwndOwner, HMENU hmenu,
if (!(item->fType & MF_POPUP) ||
!(item->fState & MF_MOUSESELECT)) return;
item->fState &= ~MF_MOUSESELECT;
- hsubmenu = item->hSubMenu;
+ submenu = item->submenu;
+ hsubmenu = MENU_GetHandle(item->submenu);
} else return;
- if (!(submenu = MENU_GetMenu( hsubmenu ))) return;
+ if (!submenu) return;
MENU_HideSubPopups( hwndOwner, hsubmenu, FALSE, wFlags );
MENU_SelectItem( hwndOwner, hsubmenu, NO_SELECTED_ITEM, sendMenuSelect, 0 );
DestroyWindow( submenu->hWnd );
@@ -2326,7 +2335,7 @@ static HMENU MENU_ShowSubPopup( HWND hwndOwner, HMENU hmenu,
/* Send WM_INITMENUPOPUP message only if TPM_NONOTIFY flag is not specified */
if (!(wFlags & TPM_NONOTIFY))
- SendMessageW( hwndOwner, WM_INITMENUPOPUP, (WPARAM)item->hSubMenu,
+ SendMessageW( hwndOwner, WM_INITMENUPOPUP, (WPARAM)MENU_GetHandle(item->submenu),
MAKELPARAM( menu->FocusedItem, IS_SYSTEM_MENU(menu) ));
item = &menu->items[menu->FocusedItem];
@@ -2351,7 +2360,7 @@ static HMENU MENU_ShowSubPopup( HWND hwndOwner, HMENU hmenu,
if (IS_SYSTEM_MENU(menu))
{
- MENU_InitSysMenuPopup(item->hSubMenu,
+ MENU_InitSysMenuPopup(MENU_GetHandle(item->submenu),
GetWindowLongW( menu->hWnd, GWL_STYLE ),
GetClassLongW( menu->hWnd, GCL_STYLE));
@@ -2396,13 +2405,13 @@ static HMENU MENU_ShowSubPopup( HWND hwndOwner, HMENU hmenu,
/* use default alignment for submenus */
wFlags &= ~(TPM_CENTERALIGN | TPM_RIGHTALIGN | TPM_VCENTERALIGN | TPM_BOTTOMALIGN);
- MENU_InitPopup( hwndOwner, item->hSubMenu, wFlags );
+ MENU_InitPopup( hwndOwner, MENU_GetHandle(item->submenu), wFlags );
- MENU_ShowPopup( hwndOwner, item->hSubMenu, menu->FocusedItem, wFlags,
+ MENU_ShowPopup( hwndOwner, MENU_GetHandle(item->submenu), menu->FocusedItem, wFlags,
rect.left, rect.top, rect.right, rect.bottom );
if (selectFirst)
- MENU_MoveSelection( hwndOwner, item->hSubMenu, ITEM_NEXT );
- return item->hSubMenu;
+ MENU_MoveSelection( hwndOwner, MENU_GetHandle(item->submenu), ITEM_NEXT );
+ return MENU_GetHandle(item->submenu);
}
@@ -2444,7 +2453,7 @@ static HMENU MENU_PtMenu( HMENU hMenu, POINT pt )
ret = (item != NO_SELECTED_ITEM &&
(menu->items[item].fType & MF_POPUP) &&
(menu->items[item].fState & MF_MOUSESELECT))
- ? MENU_PtMenu(menu->items[item].hSubMenu, pt) : 0;
+ ? MENU_PtMenu(MENU_GetHandle(menu->items[item].submenu), pt) : 0;
if (!ret) /* check the current window (avoiding WM_HITTEST) */
{
@@ -2483,7 +2492,7 @@ static INT MENU_ExecFocusedItem( MTRACKER* pmt, HMENU hMenu, UINT wFlags )
item = &menu->items[menu->FocusedItem];
- TRACE("hMenu %p wID %08lx hSubMenu %p fType %04x\n", hMenu, item->wID, item->hSubMenu, item->fType);
+ TRACE("hMenu %p wID %08lx hSubMenu %p fType %04x\n", hMenu, item->wID, MENU_GetHandle(item->submenu), item->fType);
if (!(item->fType & MF_POPUP))
{
@@ -3776,9 +3785,8 @@ UINT WINAPI GetMenuState( HMENU hMenu, UINT wItemID, UINT wFlags )
debug_print_menuitem (" item: ", item, "");
if (item->fType & MF_POPUP)
{
- POPUPMENU *menu = MENU_GetMenu( item->hSubMenu );
- if (!menu) return -1;
- else return (menu->nItems << 8) | ((item->fState|item->fType) & 0xff);
+ if (!item->submenu) return -1;
+ else return (item->submenu->nItems << 8) | ((item->fState|item->fType) & 0xff);
}
else
{
@@ -3944,6 +3952,8 @@ BOOL WINAPI RemoveMenu( HMENU hMenu, UINT nPos, UINT wFlags )
if (!(menu = MENU_GetMenu(hMenu))) return FALSE;
/* Remove item */
+ if ((item->fType & MF_POPUP) && item->submenu)
+ MENU_ReleaseMenu(item->submenu);
MENU_FreeItemData( item );
@@ -3974,7 +3984,7 @@ BOOL WINAPI DeleteMenu( HMENU hMenu, UINT nPos, UINT wFlags )
{
MENUITEM *item = MENU_FindItem( &hMenu, &nPos, wFlags );
if (!item) return FALSE;
- if (item->fType & MF_POPUP) DestroyMenu( item->hSubMenu );
+ if (item->fType & MF_POPUP) DestroyMenu( MENU_GetHandle(item->submenu) );
/* nPos is now the position of the item */
RemoveMenu( hMenu, nPos, wFlags | MF_BYPOSITION );
return TRUE;
@@ -4087,6 +4097,7 @@ HMENU WINAPI CreateMenu(void)
if (!(menu = HeapAlloc( GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*menu) ))) return 0;
menu->FocusedItem = NO_SELECTED_ITEM;
menu->bTimeToHide = FALSE;
+ menu->refcount = 1;
if (!(hMenu = alloc_user_handle( &menu->obj, USER_MENU ))) HeapFree( GetProcessHeap(), 0, menu );
@@ -4095,18 +4106,16 @@ HMENU WINAPI CreateMenu(void)
return hMenu;
}
-
-/**********************************************************************
- * DestroyMenu (USER32.@)
- */
-BOOL WINAPI DestroyMenu( HMENU hMenu )
+static void MENU_ReleaseMenu( POPUPMENU *lppop )
{
- LPPOPUPMENU lppop;
+ LONG ref;
- TRACE("(%p)\n", hMenu);
+ if (!lppop)
+ return;
- if (!(lppop = free_user_handle( hMenu, USER_MENU ))) return FALSE;
- if (lppop == OBJ_OTHER_PROCESS) return FALSE;
+ ref = InterlockedDecrement(&lppop->refcount);
+ if (ref)
+ return;
/* DestroyMenu should not destroy system menu popup owner */
if ((lppop->wFlags & (MF_POPUP | MF_SYSMENU)) == MF_POPUP && lppop->hWnd)
@@ -4115,18 +4124,46 @@ BOOL WINAPI DestroyMenu( HMENU hMenu )
lppop->hWnd = 0;
}
- if (lppop->items) /* recursively destroy submenus */
+ if (lppop->items)
{
int i;
MENUITEM *item = lppop->items;
for (i = lppop->nItems; i > 0; i--, item++)
{
- if (item->fType & MF_POPUP) DestroyMenu(item->hSubMenu);
+ if (item->fType & MF_POPUP)
+ MENU_ReleaseMenu(item->submenu);
MENU_FreeItemData( item );
}
HeapFree( GetProcessHeap(), 0, lppop->items );
}
HeapFree( GetProcessHeap(), 0, lppop );
+}
+
+/**********************************************************************
+ * DestroyMenu (USER32.@)
+ */
+BOOL WINAPI DestroyMenu( HMENU hMenu )
+{
+ POPUPMENU *lppop;
+
+ TRACE("(%p)\n", hMenu);
+
+ if (!(lppop = free_user_handle( hMenu, USER_MENU ))) return FALSE;
+ if (lppop == OBJ_OTHER_PROCESS) return FALSE;
+
+ if (lppop->items) /* recursively destroy submenu handles */
+ {
+ int i;
+ MENUITEM *item = lppop->items;
+ for (i = lppop->nItems; i > 0; i--, item++)
+ {
+ if (item->fType & MF_POPUP)
+ DestroyMenu(MENU_GetHandle(item->submenu));
+ }
+ }
+
+ MENU_ReleaseMenu(lppop);
+
return TRUE;
}
@@ -4274,7 +4311,7 @@ BOOL WINAPI GetMenuBarInfo( HWND hwnd, LONG idObject, LONG idItem, PMENUBARINFO
pmbi->fFocused = menu->FocusedItem == idItem - 1;
if (pmbi->fFocused && (menu->items[idItem - 1].fType & MF_POPUP))
{
- menu = MENU_GetMenu(menu->items[idItem - 1].hSubMenu);
+ menu = menu->items[idItem - 1].submenu;
if (menu)
pmbi->hwndMenu = menu->hWnd;
}
@@ -4346,7 +4383,7 @@ HMENU WINAPI GetSubMenu( HMENU hMenu, INT nPos )
if (!(lpmi = MENU_FindItem(&hMenu,(UINT*)&nPos,MF_BYPOSITION))) return 0;
if (!(lpmi->fType & MF_POPUP)) return 0;
- return lpmi->hSubMenu;
+ return MENU_GetHandle(lpmi->submenu);
}
@@ -4633,7 +4670,7 @@ static BOOL GetMenuItemInfo_common ( HMENU hmenu, UINT item, BOOL bypos,
lpmii->wID = menu->wID;
if (lpmii->fMask & MIIM_SUBMENU)
- lpmii->hSubMenu = menu->hSubMenu;
+ lpmii->hSubMenu = MENU_GetHandle(menu->submenu);
else {
/* hSubMenu is always cleared
* (not on Win9x/ME ) */
@@ -4730,13 +4767,12 @@ static int MENU_depth( POPUPMENU *pmenu, int depth)
item = pmenu->items;
subdepth = depth;
for( i = 0; i < pmenu->nItems && subdepth <= MAXMENUDEPTH; i++, item++){
- POPUPMENU *psubmenu = item->hSubMenu ? MENU_GetMenu( item->hSubMenu) : NULL;
- if( psubmenu){
- int bdepth = MENU_depth( psubmenu, depth);
+ if( item->submenu){
+ int bdepth = MENU_depth( item->submenu, depth);
if( bdepth > subdepth) subdepth = bdepth;
}
if( subdepth > MAXMENUDEPTH)
- TRACE("<- hmenu %p\n", item->hSubMenu);
+ TRACE("<- hmenu %p\n", MENU_GetHandle(item->submenu));
}
return subdepth;
}
@@ -4776,24 +4812,28 @@ static BOOL SetMenuItemInfo_common(MENUITEM * menu,
menu->wID = lpmii->wID;
if (lpmii->fMask & MIIM_SUBMENU) {
- menu->hSubMenu = lpmii->hSubMenu;
- if (menu->hSubMenu) {
- POPUPMENU *subMenu = MENU_GetMenu(menu->hSubMenu);
- if (subMenu) {
- if( MENU_depth( subMenu, 0) > MAXMENUDEPTH) {
+ if ((menu->fType & MF_POPUP) && menu->submenu)
+ MENU_ReleaseMenu(menu->submenu);
+ if (lpmii->hSubMenu) {
+ menu->submenu = MENU_GetMenu(lpmii->hSubMenu);
+ if (menu->submenu) {
+ if( MENU_depth( menu->submenu, 0) > MAXMENUDEPTH) {
ERR( "Loop detected in menu hierarchy or maximum menu depth exceeded!\n");
- menu->hSubMenu = 0;
+ menu->submenu = NULL;
return FALSE;
}
- subMenu->wFlags |= MF_POPUP;
- menu->fType |= MF_POPUP;
- } else {
+ menu->submenu->wFlags |= MF_POPUP;
+ menu->fType |= MF_POPUP;
+ InterlockedIncrement(&menu->submenu->refcount);
+ } else {
SetLastError( ERROR_INVALID_PARAMETER);
return FALSE;
}
- }
- else
- menu->fType &= ~MF_POPUP;
+ }
+ else{
+ menu->fType &= ~MF_POPUP;
+ menu->submenu = NULL;
+ }
}
if (lpmii->fMask & MIIM_CHECKMARKS)
@@ -4970,7 +5010,7 @@ UINT WINAPI GetMenuDefaultItem(HMENU hmenu, UINT bypos, UINT flags)
if ( (item->fType & MF_POPUP) && (flags & GMDI_GOINTOPOPUPS) )
{
UINT ret;
- ret = GetMenuDefaultItem( item->hSubMenu, bypos, flags );
+ ret = GetMenuDefaultItem( MENU_GetHandle(item->submenu), bypos, flags );
if ( -1 != ret ) return ret;
/* when item not found in submenu, return the popup item */
@@ -5115,10 +5155,9 @@ BOOL WINAPI GetMenuItemRect (HWND hwnd, HMENU hMenu, UINT uItem,
* actually use the items to draw the menu
* (recalculate and/or redraw)
*/
-static BOOL menu_SetMenuInfo( HMENU hMenu, LPCMENUINFO lpmi)
+static BOOL menu_SetMenuInfo( POPUPMENU *menu, LPCMENUINFO lpmi)
{
- POPUPMENU *menu;
- if( !(menu = MENU_GetMenu(hMenu))) return FALSE;
+ if( !menu ) return FALSE;
if (lpmi->fMask & MIM_BACKGROUND)
menu->hbrBack = lpmi->hbrBack;
@@ -5140,15 +5179,17 @@ static BOOL menu_SetMenuInfo( HMENU hMenu, LPCMENUINFO lpmi)
MENUITEM *item = menu->items;
for( i = menu->nItems; i; i--, item++)
if( item->fType & MF_POPUP)
- menu_SetMenuInfo( item->hSubMenu, lpmi);
+ menu_SetMenuInfo( item->submenu, lpmi);
}
return TRUE;
}
BOOL WINAPI SetMenuInfo (HMENU hMenu, LPCMENUINFO lpmi)
{
+ POPUPMENU *menu;
TRACE("(%p %p)\n", hMenu, lpmi);
- if( lpmi && (lpmi->cbSize == sizeof( MENUINFO)) && (menu_SetMenuInfo( hMenu, lpmi))) {
+ menu = MENU_GetMenu(hMenu);
+ if( lpmi && (lpmi->cbSize == sizeof( MENUINFO)) && (menu_SetMenuInfo( menu, lpmi))) {
if( lpmi->fMask & MIM_STYLE) {
if (lpmi->dwStyle & MNS_AUTODISMISS) FIXME("MNS_AUTODISMISS unimplemented\n");
if (lpmi->dwStyle & MNS_DRAGDROP) FIXME("MNS_DRAGDROP unimplemented\n");
diff --git a/dlls/user32/tests/menu.c b/dlls/user32/tests/menu.c
index 575d786..00bb63a 100644
--- a/dlls/user32/tests/menu.c
+++ b/dlls/user32/tests/menu.c
@@ -1859,8 +1859,10 @@ static void test_menu_search_bycommand( void )
ok (info.wID == (UINT_PTR)hmenuSub, "IDs differ for the popup menu\n");
ok (!strcmp(info.dwTypeData, "Case 2 MenuItem 2"), "Returned item has wrong label (%s)\n", info.dwTypeData);
- DestroyMenu( hmenu );
- DestroyMenu( hmenuSub );
+ rc = DestroyMenu( hmenu );
+ ok(rc == TRUE, "main menu destroy should have succeeded\n");
+ rc = DestroyMenu( hmenuSub );
+ ok(rc == FALSE, "submenu should have already been destroyed\n");
/*
Case 3: Menu containing a popup menu which in turn
--
2.1.0
More information about the wine-patches
mailing list