Resolves: tdf#97854 crash on deleting inner join relationship

seems to be a regression from...

commit 356bc0f697299e5fb6156ce25dc69845eaa6f9e6
Author: Michael Meeks <michael.meeks@collabora.com>
Date:   Wed Jul 1 19:03:55 2015 +0100

    tdf#92434 - fix iteration, and remember to disposeAndClear.

which is part of the VclPtr stuff

so hold the connection by VclPtr when removing it from m_vTableConnection
and only call dispose on it when _bDelete is true, not always.

Change-Id: I38e65c6928499dc1e8bbf6b71e9901a4eaa5d913
(cherry picked from commit e55f83e1731e1031c63d8463b57c434cceb126e3)

plus (easier to read as individual commits)

Related: tdf#97854 confirm that on !bDelete there's a ref to the connection

so that its not deleted on removal from the vector

Change-Id: I6c1f7691ee415abf0f419fdd7fcf2341fe2bd1f3
(cherry picked from commit 3185d9b8fbaad56fc7170a277569b5e441d946f4)
Reviewed-on: https://gerrit.libreoffice.org/25509
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>

6 files changed, 64 insertions, 63 deletions

diff --git a/dbaccess/source/ui/inc/JoinTableView.hxx b/dbaccess/source/ui/inc/JoinTableView.hxx
index daaaacf90523..4b8c7cb3e469 100644
--- a/dbaccess/source/ui/inc/JoinTableView.hxx
+++ b/dbaccess/source/ui/inc/JoinTableView.hxx
@@ -148,12 +148,12 @@ namespace dbaui
 
             it implies that the same as addConnection
 
-            @param  _pConnection the connection which should be removed
-            @param  _bDelete     when true then the connection will be deleted
+            @param  rConnection the connection which should be removed
+            @param  bDelete     when true then the connection will be deleted
 
             @return an iterator to next valid connection, so it can be used in any loop
         */
-        virtual bool RemoveConnection(OTableConnection* _pConnection,bool _bDelete);
+        virtual bool RemoveConnection(VclPtr<OTableConnection>& rConnection, bool bDelete);
 
         /** allows to add new connections to join table view
 
@@ -172,7 +172,7 @@ namespace dbaui
         OJoinDesignView* getDesignView() const { return m_pView; }
         OTableWindow* GetTabWindow( const OUString& rName );
 
-        OTableConnection* GetSelectedConn() { return m_pSelectedConn; }
+        VclPtr<OTableConnection>& GetSelectedConn() { return m_pSelectedConn; }
         /** @note NULL is explicitly allowed (then no-op) */
         void DeselectConn(OTableConnection* pConn);
         void SelectConn(OTableConnection* pConn);
@@ -271,7 +271,7 @@ namespace dbaui
         virtual void MouseButtonDown( const MouseEvent& rEvt ) override;
         virtual void Tracking( const TrackingEvent& rTEvt ) override;
         virtual void Paint( vcl::RenderContext& rRenderContext, const Rectangle& rRect ) override;
-        virtual void ConnDoubleClicked( OTableConnection* pConnection );
+        virtual void ConnDoubleClicked(VclPtr<OTableConnection>& rConnection);
         void SetDefaultTabWinPosSize( OTableWindow* pTabWin );
         virtual void DataChanged( const DataChangedEvent& rDCEvt ) override;
 
@@ -312,7 +312,7 @@ namespace dbaui
             @param _aPos the position where the popup menu should appear
             @param _pSelConnection the connection which should be deleted
         */
-        void executePopup(const Point& _aPos,OTableConnection* _pSelConnection);
+        void executePopup(const Point& _aPos, VclPtr<OTableConnection>& rSelConnection);
 
         /** invalidates this window without children and set the controller
             modified
diff --git a/dbaccess/source/ui/inc/QueryTableView.hxx b/dbaccess/source/ui/inc/QueryTableView.hxx
index 05687bb33e3e..4b27fa3b80e5 100644
--- a/dbaccess/source/ui/inc/QueryTableView.hxx
+++ b/dbaccess/source/ui/inc/QueryTableView.hxx
@@ -33,7 +33,7 @@ namespace dbaui
     class OQueryTableView : public OJoinTableView
     {
     protected:
-        virtual void ConnDoubleClicked(OTableConnection* pConnection) override;
+        virtual void ConnDoubleClicked(VclPtr<OTableConnection>& rConnection) override;
         virtual void KeyInput(const KeyEvent& rEvt) override;
 
         virtual VclPtr<OTableWindow> createWindow(const TTableWindowData::value_type& _pData) override;
@@ -63,7 +63,7 @@ namespace dbaui
         /// base class overwritten: create and delete Connections
         virtual void AddConnection(const OJoinExchangeData& jxdSource, const OJoinExchangeData& jxdDest) override;
 
-        virtual bool RemoveConnection( OTableConnection* _pConn ,bool _bDelete) override;
+        virtual bool RemoveConnection(VclPtr<OTableConnection>& rConn, bool bDelete) override;
 
         // transfer of connections from and to UndoAction
 
@@ -74,7 +74,7 @@ namespace dbaui
             This results effectively in complete reset of request form, as all
             windows are hidden, as are all Connections to these windows and all
             request columns based on those tables */
-        void DropConnection(OQueryTableConnection* pConn);
+        void DropConnection(VclPtr<OQueryTableConnection>& rConn);
 
         // show and hide TabWin (NOT create or delete)
         bool ShowTabWin(OQueryTableWindow* pTabWin, OQueryTabWinUndoAct* pUndoAction, bool _bAppend);
diff --git a/dbaccess/source/ui/inc/RelationTableView.hxx b/dbaccess/source/ui/inc/RelationTableView.hxx
index aef883609645..28f45f6e0000 100644
--- a/dbaccess/source/ui/inc/RelationTableView.hxx
+++ b/dbaccess/source/ui/inc/RelationTableView.hxx
@@ -37,7 +37,7 @@ namespace dbaui
         ::rtl::Reference< comphelper::OContainerListenerAdapter> m_pContainerListener;
         bool m_bInRemove;
 
-        virtual void ConnDoubleClicked( OTableConnection* pConnection ) override;
+        virtual void ConnDoubleClicked(VclPtr<OTableConnection>& rConnection) override;
         virtual void AddTabWin(const OUString& _rComposedName, const OUString& rWinName, bool bNewTable = false) override;
 
         virtual VclPtr<OTableWindow> createWindow(const TTableWindowData::value_type& _pData) override;
@@ -58,7 +58,7 @@ namespace dbaui
 
         virtual void RemoveTabWin( OTableWindow* pTabWin ) override;
         virtual void AddConnection(const OJoinExchangeData& jxdSource, const OJoinExchangeData& jxdDest) override;
-        virtual bool RemoveConnection(OTableConnection* pConn,bool _bDelete) override;
+        virtual bool RemoveConnection(VclPtr<OTableConnection>& rConn, bool _bDelete) override;
 
         virtual void ReSync() override;
 
diff --git a/dbaccess/source/ui/querydesign/JoinTableView.cxx b/dbaccess/source/ui/querydesign/JoinTableView.cxx
index 4dd4fff2268d..d12eca785cad 100644
--- a/dbaccess/source/ui/querydesign/JoinTableView.cxx
+++ b/dbaccess/source/ui/querydesign/JoinTableView.cxx
@@ -242,29 +242,26 @@ sal_uLong OJoinTableView::GetTabWinCount()
     return m_aTableMap.size();
 }
 
-bool OJoinTableView::RemoveConnection( OTableConnection* _pConn, bool _bDelete )
+bool OJoinTableView::RemoveConnection(VclPtr<OTableConnection>& rConn, bool _bDelete)
 {
-    DeselectConn(_pConn);
+    VclPtr<OTableConnection> xConn(rConn);
+
+    DeselectConn(xConn);
 
     // to force a redraw
-    _pConn->InvalidateConnection();
+    xConn->InvalidateConnection();
 
-    m_pView->getController().removeConnectionData( _pConn->GetData() );
+    m_pView->getController().removeConnectionData(xConn->GetData());
 
-    auto it = ::std::find(m_vTableConnection.begin(),m_vTableConnection.end(),_pConn);
-    if (it != m_vTableConnection.end())
-    {
-        it->disposeAndClear();
-        m_vTableConnection.erase( it );
-    }
+    m_vTableConnection.erase(std::find(m_vTableConnection.begin(), m_vTableConnection.end(), xConn));
 
     modified();
     if ( m_pAccessible )
         m_pAccessible->notifyAccessibleEvent(   AccessibleEventId::CHILD,
-                                                makeAny(_pConn->GetAccessible()),
+                                                makeAny(xConn->GetAccessible()),
                                                 Any());
-    if ( _bDelete )
-        _pConn->disposeOnce();
+    if (_bDelete)
+        xConn->disposeOnce();
 
     return true;
 }
@@ -359,14 +356,14 @@ void OJoinTableView::RemoveTabWin( OTableWindow* pTabWin )
     auto aIter = m_vTableConnection.rbegin();
     while(aIter != m_vTableConnection.rend() && bRemove)
     {
-        OTableConnection* pTabConn = (*aIter);
-        if(
-            ( pData == pTabConn->GetData()->getReferencingTable())      ||
-            ( pData == pTabConn->GetData()->getReferencedTable())
-        )
+        VclPtr<OTableConnection>& rTabConn = (*aIter);
+        if (
+             (pData == rTabConn->GetData()->getReferencingTable()) ||
+             (pData == rTabConn->GetData()->getReferencedTable())
+           )
         {
-          bRemove = RemoveConnection( pTabConn ,true);
-          aIter = m_vTableConnection.rbegin();
+            bRemove = RemoveConnection(rTabConn, true);
+            aIter = m_vTableConnection.rbegin();
         }
         else
             ++aIter;
@@ -812,7 +809,7 @@ void OJoinTableView::Tracking( const TrackingEvent& rTEvt )
     }
 }
 
-void OJoinTableView::ConnDoubleClicked( OTableConnection* /*pConnection*/ )
+void OJoinTableView::ConnDoubleClicked(VclPtr<OTableConnection>& /*rConnection*/)
 {
 }
 
@@ -857,7 +854,7 @@ void OJoinTableView::KeyInput( const KeyEvent& rEvt )
     if( !bCtrl && !bShift && (nCode==KEY_DELETE) )
     {
         if (GetSelectedConn())
-            RemoveConnection( GetSelectedConn() ,true);
+            RemoveConnection(GetSelectedConn(), true);
     }
     else
         Window::KeyInput( rEvt );
@@ -1129,16 +1126,16 @@ bool OJoinTableView::IsAddAllowed()
     return true;
 }
 
-void OJoinTableView::executePopup(const Point& _aPos,OTableConnection* _pSelConnection)
+void OJoinTableView::executePopup(const Point& _aPos, VclPtr<OTableConnection>& rSelConnection)
 {
     PopupMenu aContextMenu( ModuleRes( RID_MENU_JOINVIEW_CONNECTION ) );
     switch (aContextMenu.Execute(this, _aPos))
     {
         case SID_DELETE:
-            RemoveConnection( _pSelConnection ,true);
+            RemoveConnection(rSelConnection, true);
             break;
         case ID_QUERY_EDIT_JOINCONNECTION:
-            ConnDoubleClicked( _pSelConnection ); // is the same as double clicked
+            ConnDoubleClicked(rSelConnection); // is the same as double clicked
             break;
     }
 }
@@ -1155,21 +1152,21 @@ void OJoinTableView::Command(const CommandEvent& rEvt)
             if( m_vTableConnection.empty() )
                 return;
 
-            OTableConnection* pSelConnection = GetSelectedConn();
+            VclPtr<OTableConnection>& rSelConnection = GetSelectedConn();
             // when it wasn't a mouse event use the selected connection
             if (!rEvt.IsMouseEvent())
             {
-                if( pSelConnection )
+                if (rSelConnection)
                 {
-                    const ::std::vector<OConnectionLine*>& rLines = pSelConnection->GetConnLineList();
+                    const ::std::vector<OConnectionLine*>& rLines = rSelConnection->GetConnLineList();
                     ::std::vector<OConnectionLine*>::const_iterator aIter = ::std::find_if(rLines.begin(), rLines.end(),::std::mem_fun(&OConnectionLine::IsValid));
                     if( aIter != rLines.end() )
-                        executePopup((*aIter)->getMidPoint(),pSelConnection);
+                        executePopup((*aIter)->getMidPoint(), rSelConnection);
                 }
             }
             else
             {
-                DeselectConn(pSelConnection);
+                DeselectConn(rSelConnection);
 
                 const Point& aMousePos = rEvt.GetMousePosPixel();
                 auto aIter = m_vTableConnection.begin();
diff --git a/dbaccess/source/ui/querydesign/QueryTableView.cxx b/dbaccess/source/ui/querydesign/QueryTableView.cxx
index 8a36b91642b4..67bd063ec99d 100644
--- a/dbaccess/source/ui/querydesign/QueryTableView.cxx
+++ b/dbaccess/source/ui/querydesign/QueryTableView.cxx
@@ -635,12 +635,12 @@ void OQueryTableView::AddConnection(const OJoinExchangeData& jxdSource, const OJ
     }
 }
 
-void OQueryTableView::ConnDoubleClicked(OTableConnection* pConnection)
+void OQueryTableView::ConnDoubleClicked(VclPtr<OTableConnection>& rConnection)
 {
-    if( openJoinDialog(this,pConnection->GetData(),false) )
+    if (openJoinDialog(this, rConnection->GetData(), false))
     {
-        connectionModified(this,pConnection,false);
-        SelectConn( pConnection );
+        connectionModified(this, rConnection, false);
+        SelectConn(rConnection);
     }
 }
 
@@ -673,17 +673,19 @@ void OQueryTableView::createNewConnection()
     }
 }
 
-bool OQueryTableView::RemoveConnection( OTableConnection* _pConnection,bool /*_bDelete*/ )
+bool OQueryTableView::RemoveConnection(VclPtr<OTableConnection>& rConnection, bool /*_bDelete*/)
 {
+    VclPtr<OQueryTableConnection> xConnection(static_cast<OQueryTableConnection*>(rConnection.get()));
 
     // we don't want that our connection will be deleted, we put it in the undo manager
-    bool bRet = OJoinTableView::RemoveConnection( _pConnection,false);
+    bool bRet = OJoinTableView::RemoveConnection(rConnection, false);
 
     // add undo action
-    addUndoAction(  this,
-                    new OQueryDelTabConnUndoAction(this),
-                    static_cast< OQueryTableConnection*>(_pConnection),
-                    true);
+    addUndoAction(this,
+                  new OQueryDelTabConnUndoAction(this),
+                  xConnection.get(),
+                  true);
+
     return bRet;
 }
 
@@ -782,11 +784,12 @@ void OQueryTableView::GetConnection(OQueryTableConnection* pConn)
     addConnection( pConn );
 }
 
-void OQueryTableView::DropConnection(OQueryTableConnection* pConn)
+void OQueryTableView::DropConnection(VclPtr<OQueryTableConnection>& rConn)
 {
     // Pay attention to the selection
     // remove from me and the document
-    RemoveConnection( pConn ,false);
+    VclPtr<OTableConnection> xConn(rConn.get());
+    RemoveConnection(xConn, false);
 }
 
 void OQueryTableView::HideTabWin( OQueryTableWindow* pTabWin, OQueryTabWinUndoAct* pUndoAction )
@@ -826,17 +829,18 @@ void OQueryTableView::HideTabWin( OQueryTableWindow* pTabWin, OQueryTabWinUndoAc
     auto aIter2 = rTabConList.begin();
     for(;aIter2 != rTabConList.end();)// the end may change
     {
-        OQueryTableConnection* pTmpEntry = static_cast<OQueryTableConnection*>((*aIter2).get());
+        VclPtr<OTableConnection> xTmpEntry = *aIter2;
+        OQueryTableConnection* pTmpEntry = static_cast<OQueryTableConnection*>(xTmpEntry.get());
         OSL_ENSURE(pTmpEntry,"OQueryTableConnection is null!");
         if( pTmpEntry->GetAliasName(JTCS_FROM) == pTabWin->GetAliasName() ||
             pTmpEntry->GetAliasName(JTCS_TO) == pTabWin->GetAliasName() )
         {
             // add to undo list
-            pUndoAction->InsertConnection(pTmpEntry);
+            pUndoAction->InsertConnection(xTmpEntry);
 
             // call base class because we append an undo action
             // but this time we are in a undo action list
-            OJoinTableView::RemoveConnection(pTmpEntry,false);
+            OJoinTableView::RemoveConnection(xTmpEntry, false);
             aIter2 = rTabConList.begin();
             ++nCnt;
         }
diff --git a/dbaccess/source/ui/relationdesign/RelationTableView.cxx b/dbaccess/source/ui/relationdesign/RelationTableView.cxx
index c546c36f35d9..f0c4fbc75d0f 100644
--- a/dbaccess/source/ui/relationdesign/RelationTableView.cxx
+++ b/dbaccess/source/ui/relationdesign/RelationTableView.cxx
@@ -221,21 +221,21 @@ void ORelationTableView::AddConnection(const OJoinExchangeData& jxdSource, const
     }
 }
 
-void ORelationTableView::ConnDoubleClicked( OTableConnection* pConnection )
+void ORelationTableView::ConnDoubleClicked(VclPtr<OTableConnection>& rConnection)
 {
-    ScopedVclPtrInstance< ORelationDialog > aRelDlg( this, pConnection->GetData() );
+    ScopedVclPtrInstance< ORelationDialog > aRelDlg( this, rConnection->GetData() );
     switch (aRelDlg->Execute())
     {
         case RET_OK:
             // successfully updated
-            pConnection->UpdateLineList();
+            rConnection->UpdateLineList();
             // The connection references 1 ConnData and n ConnLines, each ConnData references n LineDatas, each Line exactly 1 LineData
             // As the Dialog and the ConnData->Update may have changed the LineDatas we have to restore the consistent state
             break;
 
         case RET_NO:
             // tried at least one update, but did not succeed -> the original connection is lost
-            RemoveConnection( pConnection ,true);
+            RemoveConnection(rConnection ,true);
             break;
 
         case RET_CANCEL:
@@ -262,13 +262,13 @@ void ORelationTableView::AddNewRelation()
     }
 }
 
-bool ORelationTableView::RemoveConnection( OTableConnection* pConn ,bool /*_bDelete*/)
+bool ORelationTableView::RemoveConnection(VclPtr<OTableConnection>& rConn, bool /*_bDelete*/)
 {
-    ORelationTableConnectionData* pTabConnData = static_cast<ORelationTableConnectionData*>(pConn->GetData().get());
+    ORelationTableConnectionData* pTabConnData = static_cast<ORelationTableConnectionData*>(rConn->GetData().get());
     try
     {
         if ( m_bInRemove || pTabConnData->DropRelation())
-            return OJoinTableView::RemoveConnection( pConn ,true);
+            return OJoinTableView::RemoveConnection(rConn, true);
     }
     catch(SQLException& e)
     {