Fix object deletion action panel crash

Crash was caused by trying to access a deleted action in action
system when the delayed update to ui contents was handled.
Shortened the delay to make it harder for user to interact with
UI during the delay, and also made all action view operations
check if item is actually still valid. Also added explicit
deletion listener to catch deletions from undo/redo.

Task-number: QT3DS-922
Change-Id: I469a33541af734f1565711b963100a2b99aa3992
Reviewed-by: Antti Määttä <antti.maatta@qt.io>
Reviewed-by: Marianne Yrjänä <marianne.yrjana@qt.io>
Reviewed-by: Tomi Korpipää <tomi.korpipaa@qt.io>

1 files changed, 73 insertions, 6 deletions

diff --git a/src/Authoring/Studio/Palettes/Action/ActionView.cpp b/src/Authoring/Studio/Palettes/Action/ActionView.cpp
index e25f5672..1156e7e5 100644
--- a/src/Authoring/Studio/Palettes/Action/ActionView.cpp
+++ b/src/Authoring/Studio/Palettes/Action/ActionView.cpp
@@ -67,6 +67,9 @@ ActionView::ActionView(const QSize &preferredSize, QWidget *parent)
     g_StudioApp.GetCore()->GetDispatch()->AddPresentationChangeListener(this);
 
     connect(this, &ActionView::actionChanged, this, [this] {
+        if (!m_itemHandle.Valid())
+            return;
+
         if (!m_propertyModel)
             m_propertyModel = new PropertyModel(this);
 
@@ -83,7 +86,7 @@ ActionView::ActionView(const QSize &preferredSize, QWidget *parent)
 
         });
 
-    m_actionChangedCompressionTimer.setInterval(500);
+    m_actionChangedCompressionTimer.setInterval(100);
     m_actionChangedCompressionTimer.setSingleShot(true);
     connect(&m_actionChangedCompressionTimer, &QTimer::timeout, this, [this] {
         updateHandlerArguments();
@@ -107,6 +110,10 @@ void ActionView::setItem(const qt3dsdm::Qt3DSDMInstanceHandle &handle)
     m_objRefHelper = GetDoc()->GetDataModelObjectReferenceHelper();
     m_itemHandle = handle;
     m_actionsModel->setInstanceHandle(handle);
+    if (m_itemHandle.Valid() != m_hasItem) {
+        m_hasItem = m_itemHandle.Valid();
+        Q_EMIT hasItemChanged();
+    }
     emitActionChanged();
     Q_EMIT itemChanged();
 }
@@ -153,7 +160,7 @@ QAbstractItemModel *ActionView::propertyModel() const
 
 QString ActionView::targetObjectName() const
 {
-    if (!GetDoc()->IsValid())
+    if (!GetDoc()->IsValid() || !m_itemHandle.Valid())
         return {};
 
     const auto actionInfo = m_actionsModel->actionInfoAt(m_currentActionIndex);
@@ -170,7 +177,7 @@ QString ActionView::targetObjectName() const
 
 QString ActionView::triggerObjectName() const
 {
-    if (!GetDoc()->IsValid())
+    if (!GetDoc()->IsValid() || !m_itemHandle.Valid())
         return {};
 
     const auto actionInfo = m_actionsModel->actionInfoAt(m_currentActionIndex);
@@ -187,7 +194,7 @@ QString ActionView::triggerObjectName() const
 
 QString ActionView::eventName() const
 {
-    if (!GetDoc()->IsValid())
+    if (!GetDoc()->IsValid() || !m_itemHandle.Valid())
         return {};
 
     const auto actionInfo = m_actionsModel->actionInfoAt(m_currentActionIndex);
@@ -201,7 +208,7 @@ QString ActionView::eventName() const
 
 QString ActionView::handlerName() const
 {
-    if (!GetDoc()->IsValid())
+    if (!GetDoc()->IsValid() || !m_itemHandle.Valid())
         return {};
 
     const auto actionInfo = m_actionsModel->actionInfoAt(m_currentActionIndex);
@@ -290,6 +297,9 @@ void ActionView::addAction()
 
 void ActionView::deleteAction(int index)
 {
+    if (!m_itemHandle.Valid())
+        return;
+
     const auto action = m_actionsModel->actionAt(index);
     if (action.Valid()) {
         Q3DStudio::SCOPED_DOCUMENT_EDITOR(*GetDoc(), QObject::tr("Delete Action"))->DeleteAction(action);
@@ -298,6 +308,9 @@ void ActionView::deleteAction(int index)
 
 QObject *ActionView::showTriggerObjectBrowser(const QPoint &point)
 {
+    if (!m_itemHandle.Valid())
+        return nullptr;
+
     if (!m_objectsModel) {
         m_objectsModel = new ObjectListModel(g_StudioApp.GetCore(),
                                              GetDoc()->GetActiveRootInstance(), this);
@@ -328,6 +341,9 @@ QObject *ActionView::showTriggerObjectBrowser(const QPoint &point)
 
 QObject *ActionView::showTargetObjectBrowser(const QPoint &point)
 {
+    if (!m_itemHandle.Valid())
+        return nullptr;
+
     if (!m_objectsModel) {
         m_objectsModel = new ObjectListModel(g_StudioApp.GetCore(),
                                              GetDoc()->GetActiveRootInstance(), this);
@@ -359,6 +375,9 @@ QObject *ActionView::showTargetObjectBrowser(const QPoint &point)
 
 void ActionView::showContextMenu(int x, int y)
 {
+    if (!m_itemHandle.Valid())
+        return;
+
     CActionContextMenu contextMenu(this);
 
     connect(&contextMenu, &CActionContextMenu::copyAction, this, &ActionView::copyAction);
@@ -373,6 +392,9 @@ void ActionView::showContextMenu(int x, int y)
 
 QObject *ActionView::showEventBrowser(const QPoint &point)
 {
+    if (!m_itemHandle.Valid())
+        return nullptr;
+
     const auto actionInfo = m_actionsModel->actionInfoAt(m_currentActionIndex);
     const auto bridge = GetBridge();
     const auto instanceHandle = bridge->GetInstance(actionInfo.m_Owner, actionInfo.m_TriggerObject);
@@ -406,6 +428,9 @@ QObject *ActionView::showEventBrowser(const QPoint &point)
 
 QObject *ActionView::showHandlerBrowser(const QPoint &point)
 {
+    if (!m_itemHandle.Valid())
+        return nullptr;
+
     const auto actionInfo = m_actionsModel->actionInfoAt(m_currentActionIndex);
     const auto bridge = GetBridge();
     const auto instanceHandle = bridge->GetInstance(actionInfo.m_Owner, actionInfo.m_TargetObject);
@@ -439,6 +464,9 @@ QObject *ActionView::showHandlerBrowser(const QPoint &point)
 
 QObject *ActionView::showEventBrowserForArgument(int handle, const QPoint &point)
 {
+    if (!m_itemHandle.Valid())
+        return nullptr;
+
     const auto actionInfo = m_actionsModel->actionInfoAt(m_currentActionIndex);
     const auto bridge = GetBridge();
     const auto instanceHandle = bridge->GetInstance(actionInfo.m_Owner, actionInfo.m_TargetObject);
@@ -511,6 +539,9 @@ void ActionView::showBrowser(QQuickWidget *browser, const QPoint &point)
 
 void ActionView::updateFiredEvent()
 {
+    if (!m_itemHandle.Valid())
+        return;
+
     const auto actionInfo = m_actionsModel->actionInfoAt(m_currentActionIndex);
     if (actionInfo.m_Handler != L"Fire Event") {
         m_firedEvent = tr("[Unknown event]");
@@ -578,6 +609,8 @@ void ActionView::OnNewPresentation()
     m_connections.push_back(theSignalProvider->ConnectInstancePropertyValue(
         std::bind(&ActionView::OnInstancePropertyValueChanged, this, std::placeholders::_1,
                   std::placeholders::_2)));
+    m_connections.push_back(theSignalProvider->ConnectInstanceDeleted(
+        std::bind(&ActionView::OnInstanceDeleted, this, std::placeholders::_1)));
     CDispatch *theDispatch = g_StudioApp.GetCore()->GetDispatch();
     m_connections.push_back(theDispatch->ConnectSelectionChange(
                                 std::bind(&ActionView::OnSelectionSet, this,
@@ -640,11 +673,15 @@ void ActionView::OnActionDeleted(qt3dsdm::Qt3DSDMActionHandle inAction, qt3dsdm:
 {
     Q_UNUSED(inSlide);
     Q_UNUSED(inOwner);
+
     m_actionsModel->removeAction(inAction);
 }
 
 void ActionView::OnActionModified(qt3dsdm::Qt3DSDMActionHandle inAction)
 {
+    if (!m_itemHandle.Valid())
+        return;
+
     if (GetDoc()->GetStudioSystem()->GetActionCore()->HandleValid(inAction)) {
            m_actionsModel->updateAction(inAction);
            emitActionChanged();
@@ -653,16 +690,34 @@ void ActionView::OnActionModified(qt3dsdm::Qt3DSDMActionHandle inAction)
 
 void ActionView::OnHandlerArgumentModified(qt3dsdm::Qt3DSDMHandlerArgHandle inHandlerArgument)
 {
+    if (!m_itemHandle.Valid())
+        return;
+
     emitActionChanged();
 }
 
 void ActionView::OnInstancePropertyValueChanged(qt3dsdm::Qt3DSDMInstanceHandle inInstance, qt3dsdm::Qt3DSDMPropertyHandle inProperty)
 {
+    if (!m_itemHandle.Valid())
+        return;
+
     emitActionChanged();
 }
 
+void ActionView::OnInstanceDeleted(qt3dsdm::Qt3DSDMInstanceHandle inInstance)
+{
+    // Clear the model on instance deletion
+    if (inInstance == m_itemHandle) {
+        qt3dsdm::Qt3DSDMInstanceHandle noInstance;
+        setItem(noInstance);
+    }
+}
+
 void ActionView::copyAction()
 {
+    if (!m_itemHandle.Valid())
+        return;
+
     auto theTempAPFile =
         GetDoc()->GetDocumentReader().CopyAction(m_actionsModel->actionAt(m_currentActionIndex),
                                                  GetDoc()->GetActiveSlide());
@@ -672,6 +727,9 @@ void ActionView::copyAction()
 
 void ActionView::cutAction()
 {
+    if (!m_itemHandle.Valid())
+        return;
+
     copyAction();
     auto action = m_actionsModel->actionAt(m_currentActionIndex);
     Q3DStudio::SCOPED_DOCUMENT_EDITOR(*GetDoc(), QObject::tr("Cut Action"))->DeleteAction(action);
@@ -679,6 +737,9 @@ void ActionView::cutAction()
 
 void ActionView::pasteAction()
 {
+    if (!m_itemHandle.Valid())
+        return;
+
     Qt3DSFile theTempAPFile = CStudioClipboard::GetActionFromClipboard();
     Q3DStudio::SCOPED_DOCUMENT_EDITOR(*GetDoc(), QObject::tr("Paste Action"))
         ->PasteAction(theTempAPFile.GetAbsolutePath(), m_itemHandle);
@@ -779,7 +840,7 @@ void ActionView::updateHandlerArguments()
     m_currentPropertyNameHandle = 0;
     m_handlerArguments.clear();
     const auto doc = GetDoc();
-    if (!doc->IsValid())
+    if (!doc->IsValid() || !m_itemHandle.Valid())
         return;
 
     const auto actionInfo = m_actionsModel->actionInfoAt(m_currentActionIndex);
@@ -814,6 +875,9 @@ void ActionView::emitActionChanged()
 
 void ActionView::setArgumentValue(int handle, const QVariant &value)
 {
+    if (!m_itemHandle.Valid())
+        return;
+
     if (handle == 0)
         return;
 
@@ -867,6 +931,9 @@ void ActionView::initialize()
 
 QStringList ActionView::slideNames()
 {
+    if (!m_itemHandle.Valid())
+        return {};
+
     std::list<Q3DStudio::CString> outSlideNames;
     QStringList slideNames;
     CClientDataModelBridge *theBridge = GetBridge();