tdf#97527 - vcl: reference-count Menu

some places are marked with "dodgy"- need to check those to see
what is going on, because they are leaving dangling pointers behind
in the Menu class

Change-Id: I41d5c7c0fec2f70ce9e3ffdc48cd03d26c0a869b
Reviewed-on: https://gerrit.libreoffice.org/26516
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noelgrandin@gmail.com>

4 files changed, 19 insertions, 18 deletions

diff --git a/svtools/source/contnr/fileview.cxx b/svtools/source/contnr/fileview.cxx
index d287cea38b41..433652bfc187 100644
--- a/svtools/source/contnr/fileview.cxx
+++ b/svtools/source/contnr/fileview.cxx
@@ -205,7 +205,7 @@ public:
 
     DECL_LINK_TYPED(ResetQuickSearch_Impl, Timer *, void);
 
-    virtual std::unique_ptr<PopupMenu> CreateContextMenu() override;
+    virtual VclPtr<PopupMenu> CreateContextMenu() override;
     virtual void        ExecuteContextMenuAction( sal_uInt16 nSelectedPopentry ) override;
 };
 
@@ -627,7 +627,7 @@ void ViewTabListBox_Impl::KeyInput( const KeyEvent& rKEvt )
 }
 
 
-std::unique_ptr<PopupMenu> ViewTabListBox_Impl::CreateContextMenu()
+VclPtr<PopupMenu> ViewTabListBox_Impl::CreateContextMenu()
 {
     bool bEnableDelete = mbEnableDelete;
     bool bEnableRename = mbEnableRename;
@@ -700,8 +700,7 @@ std::unique_ptr<PopupMenu> ViewTabListBox_Impl::CreateContextMenu()
 
     if ( bEnableDelete || bEnableRename )
     {
-        std::unique_ptr<PopupMenu> pRet(
-            new PopupMenu( SvtResId( RID_FILEVIEW_CONTEXTMENU ) ));
+        VclPtrInstance<PopupMenu> pRet(SvtResId( RID_FILEVIEW_CONTEXTMENU ));
         pRet->EnableItem( MID_FILEVIEW_DELETE, bEnableDelete );
         pRet->EnableItem( MID_FILEVIEW_RENAME, bEnableRename );
         pRet->RemoveDisabledEntries( true, true );
diff --git a/svtools/source/contnr/svimpbox.cxx b/svtools/source/contnr/svimpbox.cxx
index 20e218b0aa06..5a19237da730 100644
--- a/svtools/source/contnr/svimpbox.cxx
+++ b/svtools/source/contnr/svimpbox.cxx
@@ -2917,11 +2917,13 @@ static void lcl_DeleteSubPopups(PopupMenu* pPopup)
 {
     for(sal_uInt16 i = 0; i < pPopup->GetItemCount(); i++)
     {
-        PopupMenu* pSubPopup = pPopup->GetPopupMenu( pPopup->GetItemId( i ));
+        VclPtr<PopupMenu> pSubPopup = pPopup->GetPopupMenu( pPopup->GetItemId( i ));
         if(pSubPopup)
         {
             lcl_DeleteSubPopups(pSubPopup);
-            delete pSubPopup;
+            // NoelG: this looks very dodgy to me, we are attempting to delete this, but we leave a dangling pointer
+            // in the PopupMenu class?
+            pSubPopup.disposeAndClear();
         }
     }
 }
@@ -3017,8 +3019,8 @@ void SvImpLBox::Command( const CommandEvent& rCEvt )
         }
 
         {
-            std::unique_ptr<PopupMenu> pPopup = pView->CreateContextMenu();
-            if( pPopup.get() )
+            VclPtr<PopupMenu> pPopup = pView->CreateContextMenu();
+            if( pPopup )
             {
                 // do action for selected entry in popup menu
                 sal_uInt16 nMenuAction = pPopup->Execute( pView, aPopupPos );
diff --git a/svtools/source/contnr/treelistbox.cxx b/svtools/source/contnr/treelistbox.cxx
index abc8d353e0da..4a4f48f86f2b 100644
--- a/svtools/source/contnr/treelistbox.cxx
+++ b/svtools/source/contnr/treelistbox.cxx
@@ -3724,7 +3724,7 @@ void SvTreeListBox::InitStartEntry()
         pImpl->pStartEntry = GetModel()->First();
 }
 
-std::unique_ptr<PopupMenu> SvTreeListBox::CreateContextMenu()
+VclPtr<PopupMenu> SvTreeListBox::CreateContextMenu()
 {
     return nullptr;
 }
diff --git a/svtools/source/control/calendar.cxx b/svtools/source/control/calendar.cxx
index c04650c7b132..4ce2033fd3b2 100644
--- a/svtools/source/control/calendar.cxx
+++ b/svtools/source/control/calendar.cxx
@@ -1161,8 +1161,8 @@ void Calendar::ImplShowMenu( const Point& rPos, const Date& rDate )
     EndSelection();
 
     Date        aOldFirstDate = GetFirstMonth();
-    PopupMenu   aPopupMenu;
-    PopupMenu*  pYearPopupMenus[MENU_YEAR_COUNT];
+    ScopedVclPtrInstance<PopupMenu> aPopupMenu;
+    VclPtr<PopupMenu> pYearPopupMenus[MENU_YEAR_COUNT];
     sal_uInt16      nMonthOff;
     sal_uInt16      nCurItemId;
     sal_uInt16      nYear = rDate.GetYear()-1;
@@ -1179,26 +1179,26 @@ void Calendar::ImplShowMenu( const Point& rPos, const Date& rDate )
     // construct menu (include years with different months)
     for ( i = 0; i < MENU_YEAR_COUNT; i++ )
     {
-        pYearPopupMenus[i] = new PopupMenu;
+        pYearPopupMenus[i] = VclPtr<PopupMenu>::Create();
         for ( j = 1; j <= 12; j++ )
             pYearPopupMenus[i]->InsertItem( nYearIdCount+j,
                     maCalendarWrapper.getDisplayName(
                         i18n::CalendarDisplayIndex::MONTH, j-1, 1));
-        aPopupMenu.InsertItem( 10+i, OUString::number( nYear+i ) );
-        aPopupMenu.SetPopupMenu( 10+i, pYearPopupMenus[i] );
+        aPopupMenu->InsertItem( 10+i, OUString::number( nYear+i ) );
+        aPopupMenu->SetPopupMenu( 10+i, pYearPopupMenus[i] );
         nYearIdCount += 1000;
     }
 
     mbMenuDown = true;
-    nCurItemId = aPopupMenu.Execute( this, rPos );
+    nCurItemId = aPopupMenu->Execute( this, rPos );
     mbMenuDown = false;
 
     // destroy menu
-    aPopupMenu.SetPopupMenu( 2, nullptr );
+    aPopupMenu->SetPopupMenu( 2, nullptr );
     for ( i = 0; i < MENU_YEAR_COUNT; i++ )
     {
-        aPopupMenu.SetPopupMenu( 10+i, nullptr );
-        delete pYearPopupMenus[i];
+        aPopupMenu->SetPopupMenu( 10+i, nullptr );
+        pYearPopupMenus[i].disposeAndClear();
     }
 
     if ( nCurItemId )