From d4fe3fdb156373d16da8f8592081a7a6d074c352 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Tue, 16 Feb 2021 15:48:08 +0100 Subject: [PATCH] ClassView: Fix a crash when switching sessions Move a code that operates on QIcon instances out from non-GUI thread into GUI thread. Instead of storing the QIcon directly inside ParserTreeItem, store the path to the project. Set the real icon when we are back on the main thread side. Rename some fields to start with m_ prefix. Task-number: QTCREATORBUG-25317 Task-number: QTCREATORBUG-25312 Change-Id: Iaff89c0995045b70c5378a2ff72c5deb74abf89e Reviewed-by: Christian Kandeler --- src/plugins/classview/classviewmanager.cpp | 4 +- src/plugins/classview/classviewparser.cpp | 5 +- .../classview/classviewparsertreeitem.cpp | 130 ++++++++---------- .../classview/classviewparsertreeitem.h | 10 +- 4 files changed, 65 insertions(+), 84 deletions(-) diff --git a/src/plugins/classview/classviewmanager.cpp b/src/plugins/classview/classviewmanager.cpp index 53ff9ee142e..134e3314f92 100644 --- a/src/plugins/classview/classviewmanager.cpp +++ b/src/plugins/classview/classviewmanager.cpp @@ -198,7 +198,7 @@ bool Manager::hasChildren(QStandardItem *item) const ParserTreeItem::ConstPtr ptr = findItemByRoot(item); if (ptr.isNull()) return false; - return ptr->childCount() != 0; + return ptr->childCount(); } void Manager::initialize() @@ -245,7 +245,7 @@ void Manager::initialize() return; QSharedPointer rootItem(new QStandardItem()); - d->m_root->convertTo(rootItem.data()); + d->m_root->fetchMore(rootItem.data()); emit treeDataUpdate(rootItem); }, Qt::QueuedConnection); diff --git a/src/plugins/classview/classviewparser.cpp b/src/plugins/classview/classviewparser.cpp index b3c04420070..977c5a03676 100644 --- a/src/plugins/classview/classviewparser.cpp +++ b/src/plugins/classview/classviewparser.cpp @@ -35,7 +35,6 @@ #include #include #include -#include #include #include @@ -192,8 +191,6 @@ ParserTreeItem::ConstPtr Parser::parse() ParserTreeItem::Ptr item = addFlatTree(prj); if (item.isNull()) continue; - // TODO: remove a call to icon - item->setIcon(prj->containerNode()->icon()); projectTrees.insert(inf, item); } @@ -234,7 +231,7 @@ ParserTreeItem::Ptr Parser::getParseProjectTree(const QStringList &fileList, docTrees.append(docTree); } - ParserTreeItem::Ptr item = ParserTreeItem::mergeTrees(docTrees); + ParserTreeItem::Ptr item = ParserTreeItem::mergeTrees(Utils::FilePath::fromString(projectId), docTrees); // update the cache if (!projectId.isEmpty()) { diff --git a/src/plugins/classview/classviewparsertreeitem.cpp b/src/plugins/classview/classviewparsertreeitem.cpp index ca8dc8a4e55..f4e7aced34c 100644 --- a/src/plugins/classview/classviewparsertreeitem.cpp +++ b/src/plugins/classview/classviewparsertreeitem.cpp @@ -34,6 +34,10 @@ #include #include #include +#include +#include +#include +#include #include #include @@ -64,9 +68,9 @@ public: void mergeSymbol(const CPlusPlus::Symbol *symbol); ParserTreeItem::Ptr cloneTree() const; - QHash symbolInformations; - QSet symbolLocations; - QIcon icon; + QHash m_symbolInformations; + QSet m_symbolLocations; + const Utils::FilePath m_projectFilePath; }; void ParserTreeItemPrivate::mergeWith(const ParserTreeItem::ConstPtr &target) @@ -74,21 +78,21 @@ void ParserTreeItemPrivate::mergeWith(const ParserTreeItem::ConstPtr &target) if (target.isNull()) return; - symbolLocations.unite(target->d->symbolLocations); + m_symbolLocations.unite(target->d->m_symbolLocations); // merge children - for (auto it = target->d->symbolInformations.cbegin(); - it != target->d->symbolInformations.cend(); ++it) { + for (auto it = target->d->m_symbolInformations.cbegin(); + it != target->d->m_symbolInformations.cend(); ++it) { const SymbolInformation &inf = it.key(); const ParserTreeItem::Ptr &targetChild = it.value(); - ParserTreeItem::Ptr child = symbolInformations.value(inf); + ParserTreeItem::Ptr child = m_symbolInformations.value(inf); if (!child.isNull()) { child->d->mergeWith(targetChild); } else { const ParserTreeItem::Ptr clone = targetChild.isNull() ? ParserTreeItem::Ptr() : targetChild->d->cloneTree(); - symbolInformations.insert(inf, clone); + m_symbolInformations.insert(inf, clone); } } } @@ -124,7 +128,7 @@ void ParserTreeItemPrivate::mergeSymbol(const CPlusPlus::Symbol *symbol) // If next line will be removed, 5% speed up for the initial parsing. // But there might be a problem for some files ??? // Better to improve qHash timing - ParserTreeItem::Ptr childItem = symbolInformations.value(information); + ParserTreeItem::Ptr childItem = m_symbolInformations.value(information); if (childItem.isNull()) childItem = ParserTreeItem::Ptr(new ParserTreeItem()); @@ -133,7 +137,7 @@ void ParserTreeItemPrivate::mergeSymbol(const CPlusPlus::Symbol *symbol) SymbolLocation location(QString::fromUtf8(symbol->fileName() , symbol->fileNameLength()), symbol->line(), symbol->column()); - childItem->d->symbolLocations.insert(location); + childItem->d->m_symbolLocations.insert(location); // prevent showing a content of the functions if (!symbol->isFunction()) { @@ -153,7 +157,7 @@ void ParserTreeItemPrivate::mergeSymbol(const CPlusPlus::Symbol *symbol) // if item is empty and has not to be added if (!symbol->isNamespace() || childItem->childCount()) - symbolInformations.insert(information, childItem); + m_symbolInformations.insert(information, childItem); } /*! @@ -161,15 +165,14 @@ void ParserTreeItemPrivate::mergeSymbol(const CPlusPlus::Symbol *symbol) */ ParserTreeItem::Ptr ParserTreeItemPrivate::cloneTree() const { - ParserTreeItem::Ptr newItem(new ParserTreeItem); - newItem->d->symbolLocations = symbolLocations; - newItem->d->icon = icon; + ParserTreeItem::Ptr newItem(new ParserTreeItem(m_projectFilePath)); + newItem->d->m_symbolLocations = m_symbolLocations; - for (auto it = symbolInformations.cbegin(); it != symbolInformations.cend(); ++it) { + for (auto it = m_symbolInformations.cbegin(); it != m_symbolInformations.cend(); ++it) { ParserTreeItem::ConstPtr child = it.value(); if (child.isNull()) continue; - newItem->d->symbolInformations.insert(it.key(), child->d->cloneTree()); + newItem->d->m_symbolInformations.insert(it.key(), child->d->cloneTree()); } return newItem; @@ -189,17 +192,26 @@ ParserTreeItem::ParserTreeItem() { } +ParserTreeItem::ParserTreeItem(const Utils::FilePath &projectFilePath) + : d(new ParserTreeItemPrivate({{}, {}, projectFilePath})) +{ +} + ParserTreeItem::ParserTreeItem(const QHash &children) : d(new ParserTreeItemPrivate({children, {}, {}})) { } - ParserTreeItem::~ParserTreeItem() { delete d; } +Utils::FilePath ParserTreeItem::projectFilePath() const +{ + return d->m_projectFilePath; +} + /*! Gets information about symbol positions. \sa SymbolLocation, addSymbolLocation, removeSymbolLocation @@ -207,7 +219,7 @@ ParserTreeItem::~ParserTreeItem() QSet ParserTreeItem::symbolLocations() const { - return d->symbolLocations; + return d->m_symbolLocations; } /*! @@ -216,7 +228,7 @@ QSet ParserTreeItem::symbolLocations() const ParserTreeItem::Ptr ParserTreeItem::child(const SymbolInformation &inf) const { - return d->symbolInformations.value(inf); + return d->m_symbolInformations.value(inf); } /*! @@ -225,25 +237,7 @@ ParserTreeItem::Ptr ParserTreeItem::child(const SymbolInformation &inf) const int ParserTreeItem::childCount() const { - return d->symbolInformations.count(); -} - -/*! - \property QIcon::icon - \brief the icon assigned to the tree item -*/ - -QIcon ParserTreeItem::icon() const -{ - return d->icon; -} - -/*! - Sets the \a icon for the tree item. - */ -void ParserTreeItem::setIcon(const QIcon &icon) -{ - d->icon = icon; + return d->m_symbolInformations.count(); } ParserTreeItem::Ptr ParserTreeItem::parseDocument(const CPlusPlus::Document::Ptr &doc) @@ -257,9 +251,10 @@ ParserTreeItem::Ptr ParserTreeItem::parseDocument(const CPlusPlus::Document::Ptr return item; } -ParserTreeItem::Ptr ParserTreeItem::mergeTrees(const QList &docTrees) +ParserTreeItem::Ptr ParserTreeItem::mergeTrees(const Utils::FilePath &projectFilePath, + const QList &docTrees) { - Ptr item(new ParserTreeItem()); + Ptr item(new ParserTreeItem(projectFilePath)); for (const ConstPtr &docTree : docTrees) item->d->mergeWith(docTree); @@ -283,17 +278,29 @@ static QList locationsToRole(const QSet &locations) } /*! - Appends this item to the QStandardIten item \a item. + Checks \a item in a QStandardItemModel for lazy data population. + Make sure this method is called only from the GUI thread. */ - -void ParserTreeItem::convertTo(QStandardItem *item) const +bool ParserTreeItem::canFetchMore(QStandardItem *item) const { + if (!item) + return false; + return item->rowCount() < d->m_symbolInformations.count(); +} + +/*! + Appends this item to the QStandardIten item \a item. + Make sure this method is called only from the GUI thread. +*/ +void ParserTreeItem::fetchMore(QStandardItem *item) const +{ + using ProjectExplorer::SessionManager; if (!item) return; // convert to map - to sort it QMap map; - for (auto it = d->symbolInformations.cbegin(); it != d->symbolInformations.cend(); ++it) + for (auto it = d->m_symbolInformations.cbegin(); it != d->m_symbolInformations.cend(); ++it) map.insert(it.key(), it.value()); for (auto it = map.cbegin(); it != map.cend(); ++it) { @@ -307,7 +314,12 @@ void ParserTreeItem::convertTo(QStandardItem *item) const if (!ptr.isNull()) { // icon - add->setIcon(ptr->icon()); + const Utils::FilePath &filePath = ptr->projectFilePath(); + if (!filePath.isEmpty()) { + ProjectExplorer::Project *project = SessionManager::projectForFile(filePath); + if (project) + add->setIcon(project->containerNode()->icon()); + } // draggable if (!ptr->symbolLocations().isEmpty()) @@ -320,39 +332,13 @@ void ParserTreeItem::convertTo(QStandardItem *item) const } } -/*! - Checks \a item in a QStandardItemModel for lazy data population. -*/ - -bool ParserTreeItem::canFetchMore(QStandardItem *item) const -{ - if (!item) - return false; - - int storedChildren = item->rowCount(); - int internalChildren = d->symbolInformations.count(); - return storedChildren < internalChildren; -} - -/*! - Performs lazy data population for \a item in a QStandardItemModel if needed. -*/ - -void ParserTreeItem::fetchMore(QStandardItem *item) const -{ - if (!item) - return; - - convertTo(item); -} - /*! Debug dump. */ void ParserTreeItem::debugDump(int indent) const { - for (auto it = d->symbolInformations.cbegin(); it != d->symbolInformations.cend(); ++it) { + for (auto it = d->m_symbolInformations.cbegin(); it != d->m_symbolInformations.cend(); ++it) { const SymbolInformation &inf = it.key(); const Ptr &child = it.value(); qDebug() << QString(2 * indent, QLatin1Char(' ')) << inf.iconType() << inf.name() diff --git a/src/plugins/classview/classviewparsertreeitem.h b/src/plugins/classview/classviewparsertreeitem.h index 7e6901c3626..d2f7963affa 100644 --- a/src/plugins/classview/classviewparsertreeitem.h +++ b/src/plugins/classview/classviewparsertreeitem.h @@ -48,21 +48,19 @@ public: public: ParserTreeItem(); + ParserTreeItem(const Utils::FilePath &projectFilePath); ParserTreeItem(const QHash &children); ~ParserTreeItem(); static Ptr parseDocument(const CPlusPlus::Document::Ptr &doc); - static Ptr mergeTrees(const QList &docTrees); + static Ptr mergeTrees(const Utils::FilePath &projectFilePath, const QList &docTrees); + Utils::FilePath projectFilePath() const; QSet symbolLocations() const; Ptr child(const SymbolInformation &inf) const; int childCount() const; - // TODO: Remove icon from this API, we can't use QIcons in non-GUI thread - QIcon icon() const; - void setIcon(const QIcon &icon); - - void convertTo(QStandardItem *item) const; + // Make sure that below two methods are called only from the GUI thread bool canFetchMore(QStandardItem *item) const; void fetchMore(QStandardItem *item) const;