include/vcl/toolkit/menubtn.hxx          |    4 +++-
 vcl/inc/salvtables.hxx                   |    2 +-
 vcl/source/control/managedmenubutton.cxx |    2 +-
 vcl/source/control/menubtn.cxx           |    8 +++++++-
 vcl/source/window/builder.cxx            |    2 +-
 5 files changed, 13 insertions(+), 5 deletions(-)

New commits:
commit 9351d4d6a131b7e4c05a5762ef74789d37054aa3
Author:     Michael Weghorn <m.wegh...@posteo.de>
AuthorDate: Tue Dec 10 08:31:24 2024 +0100
Commit:     Michael Weghorn <m.wegh...@posteo.de>
CommitDate: Tue Dec 10 10:50:30 2024 +0100

    tdf#164072 vcl: Let MenuButton dispose its PopupMenu
    
    The MenuButton usually owns the PopupMenu that
    gets set via MenuButton::SetPopupMenu, but so far
    didn't dispose it, i.e. the PopupMenu was never
    disposed.
    
    As far as I understand, that is a preexisting issue,
    but since
    
        commit 6708246e20ce522e673f539369cd38687d2dd16d
        Author: Michael Weghorn <m.wegh...@posteo.de>
        Date:   Thu Dec 5 11:06:48 2024 +0000
    
            tdf#164093 tdf#157001 a11y: Improve menu window disposal
    
    , not disposing the menu also means not disposing the
    menu's MenuFloatingWindow (which gets created when the
    popup menu shows), resulting in
    
        warn:legacy.osl:563630:563630:vcl/source/window/window.cxx:307: Window 
( 10MenuButton()) with live SystemWindows destroyed:  18MenuFloatingWindow()
        Window ( 10MenuButton()) with live SystemWindows destroyed:  
18MenuFloatingWindow()
    
    and an assert getting triggered as mentioned in
    tdf#164072 comment 16.
    
    Fix this by letting the MenuButton take care of
    disposing the menu if it is the owner.
    
    The only case where the MenuButton is assigned a menu
    without taking ownership is in
    SalInstanceComboBox::set_item_menu (which currently only
    gets called from SvxStyleBox_Base::SetupEntry).
    In that case, the weld::Menu passed to
    SalInstanceComboBox::set_item_menu has the ownership:
    SalInstanceBuilder::weld_menu calls the
    SalInstanceMenu ctor with a bTakeOwnership=true,
    and the SalInstanceMenu dtor disposes the menu.
    
    Add a `bTakeOwnership` bool param to MenuButton::SetPopupMenu
    to distinguish between the cases where the PopupMenu
    is responsible for disposing the menu and where it's not.
    
    Change-Id: I32766d5084e4826056ef394a587b8c2e3124c4da
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/178197
    Tested-by: Jenkins
    Reviewed-by: Michael Weghorn <m.wegh...@posteo.de>
    (cherry picked from commit 90d9b6e12f0aa9a569958586832bd4abe9561197)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/178200

diff --git a/include/vcl/toolkit/menubtn.hxx b/include/vcl/toolkit/menubtn.hxx
index 49056a0084bb..b185fbf1e737 100644
--- a/include/vcl/toolkit/menubtn.hxx
+++ b/include/vcl/toolkit/menubtn.hxx
@@ -41,6 +41,8 @@ private:
     VclPtr<Window>  mpFloatingWindow;
     OUString         msCurItemIdent;
     sal_uInt16      mnCurItemId;
+    // whether the MenuButton is the owner of mpMenu
+    bool mbOwnPopupMenu = false;
     bool            mbDelayMenu;
     bool            mbStartingMenu;
     Link<MenuButton*,void> maActivateHdl;
@@ -80,7 +82,7 @@ public:
     //before being shown
     void            SetDelayMenu(bool bDelay) { mbDelayMenu = bDelay; }
 
-    void            SetPopupMenu(PopupMenu* pNewMenu);
+    void            SetPopupMenu(PopupMenu* pNewMenu, bool bTakeOwnership);
     PopupMenu*      GetPopupMenu() const { return mpMenu; }
 
     void            SetPopover(Window* pWindow);
diff --git a/vcl/inc/salvtables.hxx b/vcl/inc/salvtables.hxx
index 534e158b4196..a511f021940d 100644
--- a/vcl/inc/salvtables.hxx
+++ b/vcl/inc/salvtables.hxx
@@ -911,7 +911,7 @@ public:
             m_xMenuButton
                 = VclPtr<MenuButton>::Create(m_xComboBox, WB_FLATBUTTON | 
WB_NOPOINTERFOCUS);
 
-        m_xMenuButton->SetPopupMenu(pPopup);
+        m_xMenuButton->SetPopupMenu(pPopup, false);
         m_xMenuButton->Show(pPopup != nullptr);
         m_sMenuButtonRow = rIdent;
     }
diff --git a/vcl/source/control/managedmenubutton.cxx 
b/vcl/source/control/managedmenubutton.cxx
index 62be691c1e70..64320690d9ec 100644
--- a/vcl/source/control/managedmenubutton.cxx
+++ b/vcl/source/control/managedmenubutton.cxx
@@ -42,7 +42,7 @@ void ManagedMenuButton::dispose()
 void ManagedMenuButton::PrepareExecute()
 {
     if (!GetPopupMenu())
-        SetPopupMenu(VclPtr<PopupMenu>::Create());
+        SetPopupMenu(VclPtr<PopupMenu>::Create(), true);
 
     MenuButton::PrepareExecute();
 
diff --git a/vcl/source/control/menubtn.cxx b/vcl/source/control/menubtn.cxx
index 28417879888d..1cfd0799fe30 100644
--- a/vcl/source/control/menubtn.cxx
+++ b/vcl/source/control/menubtn.cxx
@@ -166,6 +166,8 @@ void MenuButton::dispose()
 {
     mpMenuTimer.reset();
     mpFloatingWindow.clear();
+    if (mpMenu && mbOwnPopupMenu)
+        mpMenu->dispose();
     mpMenu.clear();
     PushButton::dispose();
 }
@@ -240,12 +242,16 @@ void MenuButton::Select()
     maSelectHdl.Call( this );
 }
 
-void MenuButton::SetPopupMenu(PopupMenu* pNewMenu)
+void MenuButton::SetPopupMenu(PopupMenu* pNewMenu, bool bTakeOwnership)
 {
     if (pNewMenu == mpMenu)
         return;
 
+    if (mpMenu && mbOwnPopupMenu)
+        mpMenu->dispose();
+
     mpMenu = pNewMenu;
+    mbOwnPopupMenu = bTakeOwnership;
 }
 
 void MenuButton::SetPopover(Window* pWindow)
diff --git a/vcl/source/window/builder.cxx b/vcl/source/window/builder.cxx
index e181acf7ed21..3322e34d6626 100644
--- a/vcl/source/window/builder.cxx
+++ b/vcl/source/window/builder.cxx
@@ -760,7 +760,7 @@ VclBuilder::VclBuilder(vcl::Window* pParent, 
std::u16string_view sUIDir, const O
             "vcl", "missing elements of button/menu");
         if (!pTarget || !pMenu)
             continue;
-        pTarget->SetPopupMenu(pMenu);
+        pTarget->SetPopupMenu(pMenu, true);
     }
 
     //Remove ScrollWindow parent widgets whose children in vcl implement 
scrolling

Reply via email to