fix leak from framework::AddonMenuManager::BuildMenu

provide a callback when a menu item gets deleted

Change-Id: I5b5f1a181fb10f53f6b1fe7b5637d385e1517530

framework/inc/uielement/newmenucontroller.hxx

@@ -81,9 +81,6 @@ namespace framework
 
         private:
             virtual void impl_setPopupMenu() SAL_OVERRIDE;
-            typedef MenuConfiguration::Attributes AddInfo;
-
-            typedef std::unordered_map< int, std::unique_ptr<AddInfo> > AddInfoForId;
 
             void fillPopupMenu( com::sun::star::uno::Reference< com::sun::star::awt::XPopupMenu >& rPopupMenu );
             void retrieveShortcutsFromConfiguration( const ::com::sun::star::uno::Reference< ::com::sun::star::ui::XAcceleratorConfiguration >& rAccelCfg,
@@ -99,7 +96,6 @@ namespace framework
                                 m_bNewMenu    : 1,
                                 m_bModuleIdentified : 1,
                                 m_bAcceleratorCfg : 1;
-            AddInfoForId        m_aAddInfoForItem;
             OUString       m_aTargetFrame;
             OUString       m_aModuleIdentifier;
             OUString       m_aEmptyDocURL;

framework/source/fwe/classes/addonmenu.cxx

@@ -58,10 +58,7 @@ AddonMenu::~AddonMenu()
     {
         if ( GetItemType( i ) != MenuItemType::SEPARATOR )
         {
-            // delete user attributes created with new!
             sal_uInt16 nId = GetItemId( i );
-            MenuConfiguration::Attributes* pUserAttributes = reinterpret_cast<MenuConfiguration::Attributes*>(GetUserValue( nId ));
-            delete pUserAttributes;
             delete GetPopupMenu( nId );
         }
     }
@@ -333,7 +330,8 @@ void AddonMenuManager::BuildMenu( PopupMenu*                            pCurrent
 
             // Store values from configuration to the New and Wizard menu entries to enable
             // sfx2 based code to support high contrast mode correctly!
-            pCurrentMenu->SetUserValue( nId, sal_uIntPtr( new MenuConfiguration::Attributes( aTarget, aImageId )) );
+            sal_uIntPtr nAttributePtr = MenuConfiguration::Attributes::CreateAttribute(aTarget, aImageId);
+            pCurrentMenu->SetUserValue(nId, nAttributePtr, MenuConfiguration::Attributes::ReleaseAttribute);
             pCurrentMenu->SetItemCommand( nId, aURL );
 
             if ( pSubMenu )

framework/source/fwe/classes/bmkmenu.cxx

@@ -164,8 +164,8 @@ void BmkMenu::Initialize()
             else
                 InsertItem( nId, aTitle );
 
-            MenuConfiguration::Attributes* pUserAttributes = new MenuConfiguration::Attributes( aTargetFrame, aImageId );
-            SetUserValue( nId, reinterpret_cast<sal_uIntPtr>(pUserAttributes) );
+            sal_uIntPtr nAttributePtr = MenuConfiguration::Attributes::CreateAttribute(aTargetFrame, aImageId);
+            SetUserValue(nId, nAttributePtr, MenuConfiguration::Attributes::ReleaseAttribute);
 
             SetItemCommand( nId, aURL );
         }

framework/source/fwe/xml/menuconfiguration.cxx

@@ -139,6 +139,28 @@ void MenuConfiguration::StoreMenuBarConfigurationToXML(
     }
 }
 
+sal_uIntPtr MenuConfiguration::Attributes::CreateAttribute(const OUString& rFrame, const OUString& rImageIdStr)
+{
+    Attributes* pAttributes = new Attributes(rFrame, rImageIdStr);
+    pAttributes->acquire();
+    return reinterpret_cast<sal_uIntPtr>(pAttributes);
+}
+
+sal_uIntPtr MenuConfiguration::Attributes::CreateAttribute(const css::uno::WeakReference<css::frame::XDispatchProvider>& rDispatchProvider)
+{
+    Attributes* pAttributes = new Attributes(rDispatchProvider);
+    pAttributes->acquire();
+    return reinterpret_cast<sal_uIntPtr>(pAttributes);
+}
+
+void MenuConfiguration::Attributes::ReleaseAttribute(sal_uIntPtr nAttributePtr)
+{
+    if (!nAttributePtr)
+        return;
+    Attributes* pAttributes = reinterpret_cast<Attributes*>(nAttributePtr);
+    pAttributes->release();
+}
+
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */

framework/source/uielement/menubarmanager.cxx

@@ -1687,9 +1687,8 @@ void MenuBarManager::FillMenu(
                         if ( xDispatchProvider.is() )
                         {
                             // Use attributes struct to transport special dispatch provider
-                            MenuConfiguration::Attributes* pAttributes = new MenuConfiguration::Attributes;
-                            pAttributes->xDispatchProvider = xDispatchProvider;
-                            pMenu->SetUserValue( nId, reinterpret_cast<sal_uIntPtr>( pAttributes ));
+                            sal_uIntPtr nAttributePtr = MenuConfiguration::Attributes::CreateAttribute(xDispatchProvider);
+                            pMenu->SetUserValue(nId, nAttributePtr, MenuConfiguration::Attributes::ReleaseAttribute);
                         }
 
                         // Use help command to transport module identifier

framework/source/uielement/newmenucontroller.cxx

@@ -83,9 +83,10 @@ void NewMenuController::setMenuImages( PopupMenu* pPopupMenu, bool bSetImages )
                 bool        bImageSet( false );
                 OUString aImageId;
 
-                AddInfoForId::const_iterator pInfo = m_aAddInfoForItem.find( nItemId );
-                if ( pInfo != m_aAddInfoForItem.end() )
-                    aImageId = pInfo->second->aImageId; // Retrieve image id for menu item
+                sal_uIntPtr nAttributePtr = pPopupMenu->GetUserValue(sal::static_int_cast<sal_uInt16>(i));
+                MenuConfiguration::Attributes* pAttributes = reinterpret_cast<MenuConfiguration::Attributes *>(nAttributePtr);
+                if (pAttributes)
+                    aImageId = pAttributes->aImageId;
 
                 if ( !aImageId.isEmpty() )
                 {
@@ -345,13 +346,12 @@ void NewMenuController::fillPopupMenu( Reference< css::awt::XPopupMenu >& rPopup
             if (( nItemId != 0 ) &&
                 ( pSubMenu->GetItemType( nItemId ) != MenuItemType::SEPARATOR ))
             {
-                MenuConfiguration::Attributes* pBmkAttributes = reinterpret_cast<MenuConfiguration::Attributes *>(pSubMenu->GetUserValue( nItemId ));
-                if ( pBmkAttributes != 0 )
+                sal_uIntPtr nAttributePtr = pSubMenu->GetUserValue(nItemId);
+                if (nAttributePtr)
                 {
-                    auto result = m_aAddInfoForItem.insert(
-                        std::make_pair(nItemId, std::unique_ptr<AddInfo>(new AddInfo(*pBmkAttributes))));
-                    MenuConfiguration::Attributes *pNewInfo = result.first->second.get();
-                    pVCLPopupMenu->SetUserValue(nItemId, reinterpret_cast<sal_uIntPtr>(pNewInfo));
+                    MenuConfiguration::Attributes* pAttributes = reinterpret_cast<MenuConfiguration::Attributes *>(nAttributePtr);
+                    pAttributes->acquire();
+                    pVCLPopupMenu->SetUserValue(nItemId, nAttributePtr, MenuConfiguration::Attributes::ReleaseAttribute);
                 }
             }
         }
@@ -405,10 +405,16 @@ void SAL_CALL NewMenuController::itemSelected( const css::awt::MenuEvent& rEvent
         VCLXPopupMenu* pPopupMenu = static_cast<VCLXPopupMenu *>(VCLXPopupMenu::GetImplementation( xPopupMenu ));
         if ( pPopupMenu )
         {
+            OUString aTargetFrame( m_aTargetFrame );
+
             {
                 SolarMutexGuard aSolarMutexGuard;
                 PopupMenu* pVCLPopupMenu = static_cast<PopupMenu *>(pPopupMenu->GetMenu());
-                aTargetURL.Complete = pVCLPopupMenu->GetItemCommand( rEvent.MenuId );
+                aTargetURL.Complete = pVCLPopupMenu->GetItemCommand(rEvent.MenuId);
+                sal_uIntPtr nAttributePtr = pVCLPopupMenu->GetUserValue(rEvent.MenuId);
+                MenuConfiguration::Attributes* pAttributes = reinterpret_cast<MenuConfiguration::Attributes *>(nAttributePtr);
+                if (pAttributes)
+                    aTargetFrame = pAttributes->aTargetFrame;
             }
 
             xURLTransformer->parseStrict( aTargetURL );
@@ -416,11 +422,6 @@ void SAL_CALL NewMenuController::itemSelected( const css::awt::MenuEvent& rEvent
             aArgsList[0].Name = "Referer";
             aArgsList[0].Value = makeAny( OUString( "private:user" ));
 
-            OUString aTargetFrame( m_aTargetFrame );
-            AddInfoForId::const_iterator pItem = m_aAddInfoForItem.find( rEvent.MenuId );
-            if ( pItem != m_aAddInfoForItem.end() )
-                aTargetFrame = pItem->second->aTargetFrame;
-
             xDispatch = xDispatchProvider->queryDispatch( aTargetURL, aTargetFrame, 0 );
         }
     }

include/framework/menuconfiguration.hxx

@@ -61,22 +61,46 @@ class FWE_DLLPUBLIC MenuConfiguration
     public:
         struct Attributes
         {
-            Attributes()
-                : nStyle(0)
+        private:
+            oslInterlockedCount refCount;
+
+            Attributes(const OUString& rFrame, const OUString& rImageIdStr)
+                : refCount(0)
+                , aTargetFrame(rFrame)
+                , aImageId(rImageIdStr)
+                , nStyle(0)
             {
             }
 
-            Attributes( const OUString& aFrame, const OUString& aImageIdStr )
-                : aTargetFrame(aFrame)
-                , aImageId(aImageIdStr)
+            Attributes(const css::uno::WeakReference<css::frame::XDispatchProvider>& rDispatchProvider)
+                : refCount(0)
+                , xDispatchProvider(rDispatchProvider)
                 , nStyle(0)
             {
             }
 
+            Attributes(const Attributes&);  //not-implemented
+
+        public:
             OUString aTargetFrame;
             OUString aImageId;
-            ::com::sun::star::uno::WeakReference< ::com::sun::star::frame::XDispatchProvider > xDispatchProvider;
+            css::uno::WeakReference<css::frame::XDispatchProvider> xDispatchProvider;
             sal_Int16 nStyle;
+
+            static sal_uIntPtr CreateAttribute(const OUString& rFrame, const OUString& rImageIdStr);
+            static sal_uIntPtr CreateAttribute(const css::uno::WeakReference<css::frame::XDispatchProvider>& rDispatchProvider);
+            static void ReleaseAttribute(sal_uIntPtr nAttributePtr);
+
+            void acquire()
+            {
+                osl_atomic_increment(&refCount);
+            }
+
+            void release()
+            {
+                if (!osl_atomic_decrement(&refCount))
+                    delete this;
+            }
         };
 
         MenuConfiguration(

include/vcl/menu.hxx

@@ -108,6 +108,8 @@ struct MenuLogo
     Color       aEndColor;
 };
 
+typedef void (*MenuUserDataReleaseFunction)(sal_uLong);
+
 class VCL_DLLPUBLIC Menu : public Resource
 {
     friend class MenuBar;
@@ -263,8 +265,8 @@ public:
     void                SetItemBits( sal_uInt16 nItemId, MenuItemBits nBits );
     MenuItemBits        GetItemBits( sal_uInt16 nItemId ) const;
 
-    void                SetUserValue( sal_uInt16 nItemId, sal_uLong nValue );
-    sal_uLong           GetUserValue( sal_uInt16 nItemId ) const;
+    void                SetUserValue(sal_uInt16 nItemId, sal_uLong nValue, MenuUserDataReleaseFunction aFunc=0);
+    sal_uLong           GetUserValue(sal_uInt16 nItemId) const;
 
     void                SetPopupMenu( sal_uInt16 nItemId, PopupMenu* pMenu );
     PopupMenu*          GetPopupMenu( sal_uInt16 nItemId ) const;

vcl/source/window/menu.cxx

@@ -758,11 +758,16 @@ MenuItemBits Menu::GetItemBits( sal_uInt16 nItemId ) const
     return nBits;
 }
 
-void Menu::SetUserValue( sal_uInt16 nItemId, sal_uLong nValue )
+void Menu::SetUserValue(sal_uInt16 nItemId, sal_uLong nValue, MenuUserDataReleaseFunction aFunc)
 {
-    MenuItemData* pData = pItemList->GetData( nItemId );
-    if ( pData )
+    MenuItemData* pData = pItemList->GetData(nItemId);
+    if (pData)
+    {
+        if (pData->aUserValueReleaseFunc)
+            pData->aUserValueReleaseFunc(pData->nUserValue);
+        pData->aUserValueReleaseFunc = aFunc;
         pData->nUserValue = nValue;
+    }
 }
 
 sal_uLong Menu::GetUserValue( sal_uInt16 nItemId ) const

vcl/source/window/menuitemlist.cxx

@@ -32,6 +32,8 @@ using namespace vcl;
 
 MenuItemData::~MenuItemData()
 {
+    if (aUserValueReleaseFunc)
+        aUserValueReleaseFunc(nUserValue);
     if( pAutoSubMenu )
     {
         static_cast<PopupMenu*>(pAutoSubMenu)->pRefAutoSubMenu = NULL;

vcl/source/window/menuitemlist.hxx

@@ -42,7 +42,8 @@ struct MenuItemData
     OUString        aHelpCommandStr;        // Help command string (to reference external help)
     OString         sIdent;
     OString         aHelpId;                // Help-Id
-    sal_uLong           nUserValue;             // User value
+    sal_uLong       nUserValue;             // User value
+    MenuUserDataReleaseFunction aUserValueReleaseFunc;   // called when MenuItemData is destroyed
     Image           aImage;                 // Image
     vcl::KeyCode    aAccelKey;              // Accelerator-Key
     bool            bChecked;               // Checked
@@ -64,6 +65,7 @@ struct MenuItemData
         , pSubMenu(NULL)
         , pAutoSubMenu(NULL)
         , nUserValue(0)
+        , aUserValueReleaseFunc(0)
         , bChecked(false)
         , bEnabled(false)
         , bVisible(false)
@@ -81,6 +83,7 @@ struct MenuItemData
         , pAutoSubMenu(NULL)
         , aText(rStr)
         , nUserValue(0)
+        , aUserValueReleaseFunc(0)
         , aImage(rImage)
         , bChecked(false)
         , bEnabled(false)