diff options
author | Paolo Angelelli <paolo.angelelli@qt.io> | 2019-03-12 20:35:26 +0100 |
---|---|---|
committer | Paolo Angelelli <paolo.angelelli@qt.io> | 2019-03-25 14:16:33 +0000 |
commit | 0af21b1f95180949f75ee2933d4147f610a29b6b (patch) | |
tree | d16a90d4f7bf46c08c9ea5377fe4faa094cbe7d0 /src/location/declarativemaps/qdeclarativegeomap.cpp | |
parent | 6e02f895a8f3a74503971dd8df02cfafd46281f8 (diff) | |
download | qtlocation-0af21b1f95180949f75ee2933d4147f610a29b6b.tar.gz |
Fix Map destructor incorrectly deleting MapItemViews
Currently the views are explicitly destroyed, leading to
issues, in particular for views declared inside the Map.
Change it to simply remove these views from the map.
Child views will be destroyed in ~QObject.
[ChangeLog] Fixed crash when destroying Maps containing
MapItemViews.
Change-Id: Iff9b1afd6b17b55671b1f999b1bf69f172a05483
Fixes: QTBUG-69195
Fixes: QTBUG-74337
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@qt.io>
Diffstat (limited to 'src/location/declarativemaps/qdeclarativegeomap.cpp')
-rw-r--r-- | src/location/declarativemaps/qdeclarativegeomap.cpp | 58 |
1 files changed, 33 insertions, 25 deletions
diff --git a/src/location/declarativemaps/qdeclarativegeomap.cpp b/src/location/declarativemaps/qdeclarativegeomap.cpp index 63587efe..106c4dae 100644 --- a/src/location/declarativemaps/qdeclarativegeomap.cpp +++ b/src/location/declarativemaps/qdeclarativegeomap.cpp @@ -234,33 +234,45 @@ QDeclarativeGeoMap::~QDeclarativeGeoMap() m_map->clearMapItems(); } - // This forces the destruction of the associated items now, not when QObject destructor is called, at which point - // QDeclarativeGeoMap is long gone + // Remove the items from the map, making them deletable. + // Go in the same order as in removeMapChild: views, groups, then items if (!m_mapViews.isEmpty()) { - for (QDeclarativeGeoMapItemView *v : qAsConst(m_mapViews)) { + const auto mapViews = m_mapViews; + for (QDeclarativeGeoMapItemView *v : mapViews) { // so that removeMapItemView_real can safely modify m_mapViews; if (!v) continue; - if (v->parent() == this) { - delete v; - } else { - // FIXME: removeInstantiatedItems should abort, as well as exit transitions terminated - v->removeInstantiatedItems(false); - v->m_map = nullptr; - } + + QQuickItem *parent = v->parentItem(); + QDeclarativeGeoMapItemGroup *group = qobject_cast<QDeclarativeGeoMapItemGroup *>(parent); + if (group) + continue; // Ignore non-top-level MIVs. They will be recursively processed. + // Identify them as being parented by a MapItemGroup. + + removeMapItemView_real(v); } } - // remove any map items associations - for (int i = 0; i < m_mapItems.count(); ++i) { - if (m_mapItems.at(i)) - m_mapItems.at(i).data()->setMap(0,0); - } - // remove any map item groups associations - for (int i = 0; i < m_mapItemGroups.count(); ++i) { - if (m_mapItemGroups.at(i)) - m_mapItemGroups.at(i).data()->setQuickMap(nullptr); + if (!m_mapItemGroups.isEmpty()) { + const auto mapGroups = m_mapItemGroups; + for (QDeclarativeGeoMapItemGroup *g : mapGroups) { + if (!g) + continue; + + QQuickItem *parent =g->parentItem(); + QDeclarativeGeoMapItemGroup *group = qobject_cast<QDeclarativeGeoMapItemGroup *>(parent); + if (group) + continue; // Ignore non-top-level Groups. They will be recursively processed. + // Identify them as being parented by a MapItemGroup. + + removeMapItemGroup_real(g); + } } + // remove any remaining map items associations + const auto mapItems = m_mapItems; + for (auto mi: mapItems) + removeMapItem_real(mi.data()); + if (m_copyrights.data()) delete m_copyrights.data(); m_copyrights.clear(); @@ -2094,14 +2106,10 @@ bool QDeclarativeGeoMap::removeMapItemView_real(QDeclarativeGeoMapItemView *item if (!itemView || itemView->m_map != this) // can't remove a view that is already added to another map return false; - // Leaving this as void since the removal is async (potentially transitioned) - // && the delegates *could* be empty mapItemGroups. - itemView->removeInstantiatedItems(); + itemView->removeInstantiatedItems(false); // remove the items without using transitions AND abort ongoing ones itemView->m_map = 0; - // it can be removed from the list at this point, since no operations that require a Map have to be done - // anymore on destruction. m_mapViews.removeOne(itemView); - return removeMapItemGroup_real(itemView); // at this point, delegate instances have been removed. + return removeMapItemGroup_real(itemView); // at this point, all delegate instances have been removed. } /*! |