tdf#99619 Don’t add undo action when removing conn but not deleting

Previously when undo is invoked to undo adding a connection between two
tables OQueryTableView::RemoveConnection gets invoked which would end up
trying to add another undo action which would re-add the connection when
invoked. Because this happens during an undo invocation, the undo
manager is disabled and instead the new undo action gets destroyed as
soon as the first undo invocation completes. However, because the new
action is a delete it tries to become the owner of the deleted
connection. That means that when it’s destroyed it will call dispose on
the connection. However, the original undo action also tries to become
the owner of the connection even though it is now disposed so that it
can put the connection back if redo is invoked on the action. In that
case the first undo action would try to put back a connection which is
already disposed which ultimately leads to a crash.

In order to fix this, the RemoveConnection implementation now avoids
adding the undo action if the bDelete parameter is false. As far as I
can tell from inspecting the code the only way this can happen is if
RemoveConnection is invoked from OQueryTableView::DropConnection and
that only happens when applying the undo/redo actions. I think it
semantically makes sense not to make an undo action for the removal when
bDelete is false because the undo action assumes the connection is
either currently linked to a table or it itself is the owner, which
means that it will ultimately cause the connection to be deleted. If the
connection being removed shouldn’t be deleted then it implies there is
already another owner.

Change-Id: I69976c7fa8f035f9d1613382e7d0698311cf4b71
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193761
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins

dbaccess/source/ui/querydesign/QueryTableView.cxx

@@ -606,18 +606,22 @@ void OQueryTableView::createNewConnection()
         SelectConn( pConn );
 }
 
-bool OQueryTableView::RemoveConnection(VclPtr<OTableConnection>& rConnection, 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(rConnection, false);
 
-    // add undo action
-    addUndoAction(this,
-                  std::make_unique<OQueryDelTabConnUndoAction>(this),
-                  xConnection.get(),
-                  true);
+    // Don’t add an undo action if the connection isn’t to be deleted because the action will
+    // effectively take ownership of it and end up deleting it. See tdf#99619
+    if (bDelete)
+    {
+        addUndoAction(this,
+                      std::make_unique<OQueryDelTabConnUndoAction>(this),
+                      xConnection.get(),
+                      true);
+    }
 
     return bRet;
 }