[PATCH] user32: Don't destroy in-use menus

[Andrew Eikum]

---
 dlls/user32/menu.c       | 58 ++++++++++++++++++++++++++++++++++++++++--------
 dlls/user32/tests/menu.c |  6 ++---
 2 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/dlls/user32/menu.c b/dlls/user32/menu.c
index ab3f73b..1966aac 100644
--- a/dlls/user32/menu.c
+++ b/dlls/user32/menu.c
@@ -102,6 +102,11 @@ typedef struct {
     BOOL        bScrolling;   /* Scroll arrows are active */
     UINT        nScrollPos;   /* Current scroll position */
     UINT        nTotalHeight; /* Total height of menu items inside menu */
+
+    /* two different kinds of "reference" counting. in_use is for internal
+     * refcounting for e.g. submenus. user_destroyed indicates whether
+     * DestroyMenu has been invoked on a handle returned by CreateMenu. */
+    BOOL        in_use, user_destroyed;
     /* ------------ MENUINFO members ------ */
     DWORD	dwStyle;	/* Extended menu style */
     UINT	cyMax;		/* max height of the whole menu, 0 is screen height */
@@ -3956,6 +3961,11 @@ BOOL WINAPI RemoveMenu( HMENU hMenu, UINT nPos, UINT wFlags )
 
       /* Remove item */
 
+    if (item->fType & MF_POPUP) {
+        POPUPMENU *sub = MENU_GetMenu(item->hSubMenu);
+        InterlockedDecrement(&sub->in_use);
+    }
+
     MENU_FreeItemData( item );
 
     if (--menu->nItems == 0)
@@ -4106,16 +4116,19 @@ HMENU WINAPI CreateMenu(void)
     return hMenu;
 }
 
-
-/**********************************************************************
- *         DestroyMenu    (USER32.@)
- */
-BOOL WINAPI DestroyMenu( HMENU hMenu )
+static BOOL MENU_DestroyMenu( HMENU hMenu )
 {
     LPPOPUPMENU lppop;
 
     TRACE("(%p)\n", hMenu);
 
+    if (!(lppop = MENU_GetMenu(hMenu)))
+        return FALSE;
+
+    TRACE("user_destroyed: %u, in_use: %u\n", lppop->user_destroyed, lppop->in_use);
+    if (lppop->in_use)
+        return TRUE;
+
     if (!(lppop = free_user_handle( hMenu, USER_MENU ))) return FALSE;
     if (lppop == OBJ_OTHER_PROCESS) return FALSE;
 
@@ -4132,7 +4145,11 @@ BOOL WINAPI DestroyMenu( HMENU hMenu )
         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) {
+                POPUPMENU *sub = MENU_GetMenu(item->hSubMenu);
+                InterlockedDecrement(&sub->in_use);
+                MENU_DestroyMenu(item->hSubMenu);
+            }
             MENU_FreeItemData( item );
         }
         HeapFree( GetProcessHeap(), 0, lppop->items );
@@ -4141,6 +4158,20 @@ BOOL WINAPI DestroyMenu( HMENU hMenu )
     return TRUE;
 }
 
+/**********************************************************************
+ *         DestroyMenu    (USER32.@)
+ */
+BOOL WINAPI DestroyMenu( HMENU hMenu )
+{
+    POPUPMENU *lppop;
+
+    if (!(lppop = MENU_GetMenu(hMenu)))
+        return FALSE;
+
+    lppop->user_destroyed = TRUE;
+
+    return MENU_DestroyMenu(hMenu);
+}
 
 /**********************************************************************
  *         GetSystemMenu    (USER32.@)
@@ -4546,7 +4577,7 @@ BOOL WINAPI IsMenu(HMENU hmenu)
 {
     LPPOPUPMENU menu = MENU_GetMenu(hmenu);
 
-    if (!menu)
+    if (!menu || menu->user_destroyed)
     {
         SetLastError(ERROR_INVALID_MENU_HANDLE);
         return FALSE;
@@ -4787,6 +4818,13 @@ static BOOL SetMenuItemInfo_common(MENUITEM * menu,
 	menu->wID = lpmii->wID;
 
     if (lpmii->fMask & MIIM_SUBMENU) {
+        if (menu->fType & MF_POPUP) {
+            /* release internal reference for replaced menu item */
+            POPUPMENU *subMenu = MENU_GetMenu(menu->hSubMenu);
+            if (subMenu)
+                InterlockedDecrement(&subMenu->in_use);
+        }
+
 	menu->hSubMenu = lpmii->hSubMenu;
 	if (menu->hSubMenu) {
 	    POPUPMENU *subMenu = MENU_GetMenu(menu->hSubMenu);
@@ -4798,6 +4836,7 @@ static BOOL SetMenuItemInfo_common(MENUITEM * menu,
                 }
 		subMenu->wFlags |= MF_POPUP;
 		menu->fType |= MF_POPUP;
+                InterlockedIncrement(&subMenu->in_use);
 	    } else {
                 SetLastError( ERROR_INVALID_PARAMETER);
                 return FALSE;
@@ -5179,11 +5218,12 @@ BOOL WINAPI SetMenuInfo (HMENU hMenu, LPCMENUINFO lpmi)
  *
  */
 BOOL WINAPI GetMenuInfo (HMENU hMenu, LPMENUINFO lpmi)
-{   POPUPMENU *menu;
+{
+    POPUPMENU *menu;
 
     TRACE("(%p %p)\n", hMenu, lpmi);
 
-    if (lpmi && (lpmi->cbSize == sizeof( MENUINFO)) && (menu = MENU_GetMenu(hMenu)))
+    if (lpmi && (lpmi->cbSize == sizeof( MENUINFO)) && (menu = MENU_GetMenu(hMenu)) && !menu->user_destroyed)
     {
 
 	if (lpmi->fMask & MIM_BACKGROUND)
diff --git a/dlls/user32/tests/menu.c b/dlls/user32/tests/menu.c
index 593292d..7290fb8 100644
--- a/dlls/user32/tests/menu.c
+++ b/dlls/user32/tests/menu.c
@@ -439,7 +439,7 @@ static void test_menu_locked_by_window(void)
     DestroyWindow(hwnd);
 }
 
-/* demonstrates that subpopup's are locked
+/* demonstrates that subpopups are locked
  * even after a client calls DestroyMenu on it */
 static LRESULT WINAPI subpopuplocked_wnd_proc(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam)
 {
@@ -526,9 +526,7 @@ static void test_subpopup_locked_by_menu(void)
         SetLastError( 0xdeadbeef);
         ret = TrackPopupMenu( hmenu, TPM_RETURNCMD, 100,100, 0, hwnd, NULL);
         gle = GetLastError();
-        todo_wine {
-            ok( ret == itemid , "TrackPopupMenu returned %d error is %d\n", ret, gle);
-        }
+        ok( ret == itemid , "TrackPopupMenu returned %d error is %d\n", ret, gle);
         ok( gle == 0 ||
             broken(gle == 0xdeadbeef) || /* wow64 */
             broken(gle == ERROR_INVALID_PARAMETER), /* win2k0 */
-- 
1.9.2