Fixes crash when full refreshing AssetProcessor (#17869) (#17874)

* Fixes crash when full refreshing AssetProcessor
Also fixes a crash when deleting elements out of the file system
Also fixes a crash when clicking on thumbnails while they are generating.

Signed-off-by: Nicholas Lawson <[email protected]>
Co-authored-by: Steve Pham <[email protected]>

Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetBrowserBus.h

@@ -298,6 +298,9 @@ namespace AzToolsFramework
 
             virtual void BeginRemoveEntry(AssetBrowserEntry* entry) = 0;
             virtual void EndRemoveEntry() = 0;
+
+            virtual void BeginReset() = 0;
+            virtual void EndReset() = 0;
         };
 
         using AssetBrowserModelRequestBus = AZ::EBus<AssetBrowserModelRequests>;

Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetBrowserComponent.cpp

@@ -47,6 +47,8 @@ namespace AzToolsFramework
             , m_dbReady(false)
             , m_waitingForMore(false)
             , m_disposed(false)
+            , m_isResetting(false)
+            , m_changesApplied(false)
             , m_assetBrowserModel(aznew AssetBrowserModel)
             , m_changeset(new AssetEntryChangeset(m_databaseConnection, m_rootEntry))
         {
@@ -174,6 +176,13 @@ namespace AzToolsFramework
                 m_entriesReady = true;
                 AssetBrowserComponentNotificationBus::Broadcast(&AssetBrowserComponentNotifications::OnAssetBrowserComponentReady);
             }
+
+            if (m_isResetting && m_changesApplied)
+            {
+                m_isResetting = false;
+                m_changesApplied = false;
+                m_assetBrowserModel->EndReset();
+            }
         }
 
         // We listen to this bus so that a file that wasn't previously a 'source file' (just a file) can become a source file
@@ -390,6 +399,11 @@ namespace AzToolsFramework
 
         void AssetBrowserComponent::PopulateAssets()
         {
+            // populating the assets is a complete reset of the model.
+            m_isResetting = true;
+            m_changesApplied = false;
+            m_assetBrowserModel->BeginReset();
+            m_rootEntry->PrepareForReset();
             m_changeset->PopulateEntries();
             NotifyUpdateThread();
         }
@@ -414,6 +428,7 @@ namespace AzToolsFramework
                 if (m_dbReady)
                 {
                     m_changeset->Update();
+                    m_changesApplied = true;
                 }
             }
         }

Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetBrowserComponent.h

@@ -124,6 +124,8 @@ namespace AzToolsFramework
             AZStd::atomic_bool m_waitingForMore;
             //! should the query thread stop
             AZStd::atomic_bool m_disposed;
+            AZStd::atomic_bool m_isResetting;
+            AZStd::atomic_bool m_changesApplied;
 
             AZStd::unique_ptr<AssetBrowserModel> m_assetBrowserModel;
             AZStd::shared_ptr<AssetEntryChangeset> m_changeset;

Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetBrowserModel.cpp

@@ -472,21 +472,31 @@ namespace AzToolsFramework
 
         void AssetBrowserModel::BeginAddEntry(AssetBrowserEntry* parent)
         {
+            if (m_isResetting)
+            {
+                return; // don't notify during reset.
+            }
+
             QModelIndex parentIndex;
             if (GetEntryIndex(parent, parentIndex))
             {
                 m_addingEntry = true;
                 int row = parent->GetChildCount();
-                beginInsertRows(parentIndex, row, row);
+                Q_EMIT beginInsertRows(parentIndex, row, row);
             }
         }
 
         void AssetBrowserModel::EndAddEntry(AssetBrowserEntry* parent)
         {
+            if (m_isResetting)
+            {
+                return; // don't notify during reset.
+            }
+
             if (m_addingEntry)
             {
                 m_addingEntry = false;
-                endInsertRows();
+                Q_EMIT endInsertRows();
 
                 // we have to also invalidate our parent all the way up the chain.
                 // since in this model, the children's data is actually relevant to the filtering of a parent
@@ -517,24 +527,46 @@ namespace AzToolsFramework
 
         void AssetBrowserModel::BeginRemoveEntry(AssetBrowserEntry* entry)
         {
+            if (m_isResetting)
+            {
+                return; // don't notify during reset.
+            }
+            
             int row = entry->row();
             QModelIndex parentIndex;
             if (GetEntryIndex(entry->m_parentAssetEntry, parentIndex))
             {
                 m_removingEntry = true;
-                beginRemoveRows(parentIndex, row, row);
+                Q_EMIT beginRemoveRows(parentIndex, row, row);
             }
         }
 
         void AssetBrowserModel::EndRemoveEntry()
         {
+            if (m_isResetting)
+            {
+                return; // don't notify during reset.
+            }
+
             if (m_removingEntry)
             {
                 m_removingEntry = false;
-                endRemoveRows();
+                Q_EMIT endRemoveRows();
             }
         }
 
+        void AssetBrowserModel::BeginReset()
+        {
+            Q_EMIT beginResetModel();
+            m_isResetting = true;
+        }
+
+        void AssetBrowserModel::EndReset()
+        {
+            m_isResetting = false;
+            Q_EMIT endResetModel();
+        }
+
         void AssetBrowserModel::HandleAssetCreatedInEditor(const AZStd::string& assetPath, const AZ::Crc32& creatorBusId, const bool initialFilenameChange)
         {
             if (initialFilenameChange)
@@ -542,7 +574,7 @@ namespace AzToolsFramework
                 QModelIndex index = findIndex(assetPath.c_str());
                 if (index.isValid())
                 {
-                    emit RequestOpenItemForEditing(index);
+                    Q_EMIT RequestOpenItemForEditing(index);
                 }
                 else
                 {
@@ -616,7 +648,7 @@ namespace AzToolsFramework
                         QModelIndex index;
                         if (GetEntryIndex(entry, index))
                         {
-                            emit RequestOpenItemForEditing(index);
+                            Q_EMIT RequestOpenItemForEditing(index);
                         }
                     });
             }

Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetBrowserModel.h

@@ -85,6 +85,9 @@ namespace AzToolsFramework
             void BeginRemoveEntry(AssetBrowserEntry* entry) override;
             void EndRemoveEntry() override;
 
+            void BeginReset() override;
+            void EndReset() override;
+
             //////////////////////////////////////////////////////////////////////////
             // TickBus
             //////////////////////////////////////////////////////////////////////////
@@ -119,6 +122,8 @@ namespace AzToolsFramework
             AZStd::unordered_map<AZStd::string, AZ::Crc32> m_newlyCreatedAssetPathsToCreatorBusIds;
 
             void WatchForExpectedAssets(AssetBrowserEntry* entry);
+
+            bool m_isResetting = false;
         };
     } // namespace AssetBrowser
 } // namespace AzToolsFramework

Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetBrowserTreeToTableProxyModel.cpp

@@ -217,6 +217,10 @@ namespace AzToolsFramework
                     [this]()
                     {
                         beginResetModel();
+                        resetInternalData();
+                        m_rowCount = 0;
+                        m_map.Clear();
+                        m_parents.clear();
                     });
 
                 connect(
@@ -265,7 +269,6 @@ namespace AzToolsFramework
                     });
             }
 
-            resetInternalData();
             if (model && model->hasChildren())
             {
                 RefreshMap();
@@ -358,7 +361,7 @@ namespace AzToolsFramework
             const int rowCount = sourceModelPtr->rowCount(parent);
             const int span = end - start + 1;
 
-            if (rowCount == span)
+            if (rowCount == span) // updating the entire parent
             {
                 const QModelIndex index = mapFromSource(parent);
                 if (parent.isValid())
@@ -385,11 +388,8 @@ namespace AzToolsFramework
             {
                 const QModelIndex oldIndex = sourceModelPtr->index(rowCount - 1 - span, column, parent);
                 AZ_Assert(m_map.TreeContains(oldIndex), "Tree does not contain index");
-
                 const QModelIndex newIndex = sourceModelPtr->index(rowCount - 1, column, parent);
-
                 QModelIndex indexAbove = oldIndex;
-
                 if (start > 0)
                 {
                     while (sourceModelPtr->hasChildren(indexAbove))
@@ -720,13 +720,14 @@ namespace AzToolsFramework
 
         void AssetBrowserTreeToTableProxyModel::ModelReset()
         {
+            m_processing = true;
             resetInternalData();
             auto sourceModelPtr = sourceModel();
             if (sourceModelPtr->hasChildren() && sourceModelPtr->rowCount() > 0)
             {
-                m_parents.append(QModelIndex());
-                ProcessParents();
+                RefreshMap();
             }
+            m_processing = false;
             endResetModel();
         }
 
@@ -785,8 +786,18 @@ namespace AzToolsFramework
             {
                 const QModelIndex sourceTopLeft = sourceModelPtr->index(i, topLeft.column(), topLeft.parent());
                 const QModelIndex proxyTopLeft = mapFromSource(sourceTopLeft);
+                if (!proxyTopLeft.isValid())
+                {
+                    // not all elements might be mapped during a partial update.
+                    continue;
+                }
                 const QModelIndex sourceBottomRight = sourceModelPtr->index(i, bottomRight.column(), bottomRight.parent());
                 const QModelIndex proxyBottomRight = mapFromSource(sourceBottomRight);
+                if (!proxyBottomRight.isValid())
+                {
+                    // not all elements might be mapped during a partial update.
+                    continue;
+                }
                 emit dataChanged(proxyTopLeft, proxyBottomRight);
             }
         }
@@ -832,6 +843,13 @@ namespace AzToolsFramework
                         break;
                     }
                 }
+
+                if (result == end)
+                {
+                    // its possible to still be building the model and to not find the target
+                    return QModelIndex();
+                }
+
                 const QModelIndex sourceLastChild = result.value();
                 int proxyRow = result.key();
                 QModelIndex index = sourceLastChild;
@@ -876,7 +894,7 @@ namespace AzToolsFramework
                 verticalDistance -= (ancestorRow + 1);
                 ancestor = ancestor.parent();
             }
-            AZ_Assert(0, "Could't find target row.");
+            // its possible not to find a target row if the model is still building incrementally.
             return QModelIndex();
         }
 

Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/Entries/AssetBrowserEntry.cpp

@@ -123,6 +123,9 @@ namespace AzToolsFramework
             child->m_parentAssetEntry = nullptr;
             AssetBrowserModelRequestBus::Broadcast(&AssetBrowserModelRequests::EndRemoveEntry);
             AssetBrowserModelNotificationBus::Broadcast(&AssetBrowserModelNotifications::EntryRemoved, childToRemove.get());
+
+            // before we allow the destructor of AssetBrowserEntry to run, we must remove its children, etc.
+            childToRemove->RemoveChildren();
         }
 
         void AssetBrowserEntry::RemoveChildren()

Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/Entries/RootAssetBrowserEntry.cpp

@@ -50,13 +50,16 @@ namespace AzToolsFramework
             return AssetEntryType::Root;
         }
 
-        void RootAssetBrowserEntry::Update(const char* enginePath)
+
+        void RootAssetBrowserEntry::PrepareForReset()
         {
             RemoveChildren();
-
             auto entryCache = EntryCache::GetInstance();
             entryCache->Clear();
+        }
 
+        void RootAssetBrowserEntry::Update(const char* enginePath)
+        {
             m_enginePath = AZ::IO::Path(enginePath).LexicallyNormal();
             m_projectPath = AZ::IO::Path(AZ::Utils::GetProjectPath()).LexicallyNormal();
             SetFullPath(m_enginePath);

Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/Entries/RootAssetBrowserEntry.h

@@ -71,6 +71,7 @@ namespace AzToolsFramework
 
             bool IsInitialUpdate() const;
             void SetInitialUpdate(bool newValue);
+            void PrepareForReset();
 
         protected:
             void UpdateChildPaths(AssetBrowserEntry* child) const override;