[Libreoffice-commits] core.git: Branch 'libreoffice-7-2' - framework/inc framework/source

[Michael Stahl (via logerrit)]

 framework/inc/uielement/toolbarmanager.hxx    |    2 +-
 framework/source/uielement/toolbarmanager.cxx |    8 ++++----
 framework/source/uielement/toolbarmerger.cxx  |    2 ++
 3 files changed, 7 insertions(+), 5 deletions(-)

New commits:
commit c8f149c62e37a465944f7aea65822b37a4836a27
Author:     Michael Stahl <michael.stahl at allotropia.de>
AuthorDate: Mon Jul 19 17:33:04 2021 +0200
Commit:     Thorsten Behrens <thorsten.behrens at allotropia.de>
CommitDate: Tue Jul 20 18:37:55 2021 +0200

    framework: fix multiple-free in VclToolBarManager::Destroy()
    
    Multiple ImplToolItem with mnId 1000 are inserted in the same ToolBox
    and the problem is that only the first one gets its mpUserData set,
    because setting and getting that is based on the assumption that mnId is
    unique per container.
    
    So in Destroy() for every item with mnId 1000 the same AddonsParams
    object is deleted.
    
    The problem is that in ToolBarManager::FillToolbar() the local nItemId
    is copied, so its value never changes when iterating the loop because
    only its copy is incremented.
    
    (regression from ac1aefd5174258d9bcb41465ce566ed1a0819f3d)
    
    Change-Id: Ica47051f9447c2b9d1981f8373d754874f0f31b0
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/119228
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <michael.stahl at allotropia.de>
    (cherry picked from commit 7d1175c46588c246c455cd7c8874136bf61197fd)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/119186
    Reviewed-by: Thorsten Behrens <thorsten.behrens at allotropia.de>

diff --git a/framework/inc/uielement/toolbarmanager.hxx b/framework/inc/uielement/toolbarmanager.hxx
index 8a87c2a6b78a..6de526423d38 100644
--- a/framework/inc/uielement/toolbarmanager.hxx
+++ b/framework/inc/uielement/toolbarmanager.hxx
@@ -92,7 +92,7 @@ public:
     virtual vcl::ImageType GetImageSize() = 0;
     virtual void ConnectCallbacks(ToolBarManager* pManager) = 0;
     virtual void SetMenuType(ToolBoxMenuType eType) = 0;
-    virtual void MergeToolbar(ToolBoxItemId nItemId,
+    virtual void MergeToolbar(ToolBoxItemId & rItemId,
                               const OUString& rModuleIdentifier,
                               CommandToInfoMap& rCommandMap,
                               MergeToolbarInstruction& rInstruction) = 0;
diff --git a/framework/source/uielement/toolbarmanager.cxx b/framework/source/uielement/toolbarmanager.cxx
index 0bd0d627ff9d..1c31beb1eae6 100644
--- a/framework/source/uielement/toolbarmanager.cxx
+++ b/framework/source/uielement/toolbarmanager.cxx
@@ -336,7 +336,7 @@ public:
         m_pToolBar->SetMenuType( eType );
     }
 
-    virtual void MergeToolbar(ToolBoxItemId nItemId,
+    virtual void MergeToolbar(ToolBoxItemId & rItemId,
                               const OUString& rModuleIdentifier,
                               CommandToInfoMap& rCommandMap,
                               MergeToolbarInstruction& rInstruction) override
@@ -352,7 +352,7 @@ public:
         {
             ToolBarMerger::ProcessMergeOperation( m_pToolBar,
                                                     aRefPoint.nPos,
-                                                    nItemId,
+                                                    rItemId,
                                                     rCommandMap,
                                                     rModuleIdentifier,
                                                     rInstruction.aMergeCommand,
@@ -362,7 +362,7 @@ public:
         else
         {
             ToolBarMerger::ProcessMergeFallback( m_pToolBar,
-                                                    nItemId,
+                                                    rItemId,
                                                     rCommandMap,
                                                     rModuleIdentifier,
                                                     rInstruction.aMergeCommand,
@@ -524,7 +524,7 @@ public:
 
     virtual void SetMenuType(ToolBoxMenuType /*eType*/) override {}
 
-    virtual void MergeToolbar(ToolBoxItemId /*nItemId*/,
+    virtual void MergeToolbar(ToolBoxItemId & /*rItemId*/,
                               const OUString& /*rModuleIdentifier*/,
                               CommandToInfoMap& /*rCommandMap*/,
                               MergeToolbarInstruction& /*rInstruction*/) override {}
diff --git a/framework/source/uielement/toolbarmerger.cxx b/framework/source/uielement/toolbarmerger.cxx
index 652383988bcd..faba3cb4e8d9 100644
--- a/framework/source/uielement/toolbarmerger.cxx
+++ b/framework/source/uielement/toolbarmerger.cxx
@@ -623,6 +623,8 @@ rtl::Reference<::cppu::OWeakObject> ToolBarMerger::CreateController(
 
 void ToolBarMerger::CreateToolbarItem( ToolBox* pToolbar, ToolBox::ImplToolItems::size_type nPos, ToolBoxItemId nItemId, const AddonToolbarItem& rItem )
 {
+    assert(pToolbar->GetItemData(nItemId) == nullptr); // that future would contain a double free
+
     pToolbar->InsertItem( nItemId, rItem.aLabel, ToolBoxItemBits::NONE, nPos );
     pToolbar->SetItemCommand( nItemId, rItem.aCommandURL );
     pToolbar->SetQuickHelpText( nItemId, rItem.aLabel );