diff options
author | Aaron McCarthy <aaron.mccarthy@jollamobile.com> | 2014-02-10 13:16:15 +1000 |
---|---|---|
committer | The Qt Project <gerrit-noreply@qt-project.org> | 2014-02-10 14:48:25 +0100 |
commit | cdf5af5ba4c80c98cf4967ecf82ecac80ce3b854 (patch) | |
tree | 6e5701ac6f5b2c602353430962f6ea5621d46fbc | |
parent | 62234914bc39be509c6a764c99d592e0bc5ecb5c (diff) | |
download | qtlocation-cdf5af5ba4c80c98cf4967ecf82ecac80ce3b854.tar.gz |
Test that the mapping manager engine pointer is still valid.
QPointer holds a guarded pointer to a QObject. It must always be tested
prior to use.
Changed the parameter type to remove the need for the static_cast when
setting the QPointer member.
The root cause was that the tile fetcher object was not being destroyed
when the mapping manager engine was. Which was because the two objects
were in different threads. Some of the service specific tile fetchers
were directly calling into the engine objects across thread. Gah.
Since Qt 5 QNAM uses an application global thread for processing
requests, there is no need to use threads in the tile fetcher.
Change-Id: Id9df35ddfa78df2cbf334006fe5fc9726a75f92d
Reviewed-by: Alex Blasche <alexander.blasche@digia.com>
-rw-r--r-- | src/location/maps/qgeotiledmappingmanagerengine.cpp | 32 | ||||
-rw-r--r-- | src/location/maps/qgeotiledmappingmanagerengine_p_p.h | 3 | ||||
-rw-r--r-- | src/location/maps/qgeotilefetcher.cpp | 33 | ||||
-rw-r--r-- | src/location/maps/qgeotilefetcher_p.h | 5 | ||||
-rw-r--r-- | src/location/maps/qgeotilefetcher_p_p.h | 4 | ||||
-rw-r--r-- | src/plugins/geoservices/nokia/qgeotiledmappingmanagerengine_nokia.cpp | 4 | ||||
-rw-r--r-- | src/plugins/geoservices/nokia/qgeotilefetcher_nokia.cpp | 45 | ||||
-rw-r--r-- | src/plugins/geoservices/nokia/qgeotilefetcher_nokia.h | 11 | ||||
-rw-r--r-- | src/plugins/geoservices/osm/qgeotilefetcherosm.cpp | 2 | ||||
-rw-r--r-- | src/plugins/geoservices/osm/qgeotilefetcherosm.h | 2 | ||||
-rw-r--r-- | tests/auto/geotestplugin/qgeotilefetcher_test.h | 17 |
11 files changed, 45 insertions, 113 deletions
diff --git a/src/location/maps/qgeotiledmappingmanagerengine.cpp b/src/location/maps/qgeotiledmappingmanagerengine.cpp index c0382b1a..69532f44 100644 --- a/src/location/maps/qgeotiledmappingmanagerengine.cpp +++ b/src/location/maps/qgeotiledmappingmanagerengine.cpp @@ -74,9 +74,6 @@ void QGeoTiledMappingManagerEngine::setTileFetcher(QGeoTileFetcher *fetcher) Q_D(QGeoTiledMappingManagerEngine); d->fetcher_ = fetcher; - d->fetcher_->init(); - - d->thread_ = new QThread; qRegisterMetaType<QGeoTileSpec>(); @@ -91,26 +88,7 @@ void QGeoTiledMappingManagerEngine::setTileFetcher(QGeoTileFetcher *fetcher) SLOT(engineTileError(QGeoTileSpec,QString)), Qt::QueuedConnection); - d->fetcher_->moveToThread(d_ptr->thread_); - - connect(d->thread_, - SIGNAL(started()), - d->fetcher_, - SLOT(threadStarted())); - - connect(d->thread_, - SIGNAL(finished()), - d->fetcher_, - SLOT(threadFinished())); - - connect(d->fetcher_, - SIGNAL(destroyed()), - d->thread_, - SLOT(deleteLater())); - engineInitialized(); - - QTimer::singleShot(0, d->thread_, SLOT(start())); } QGeoTileFetcher *QGeoTiledMappingManagerEngine::tileFetcher() @@ -318,17 +296,13 @@ QSharedPointer<QGeoTileTexture> QGeoTiledMappingManagerEngine::getTileTexture(co *******************************************************************************/ QGeoTiledMappingManagerEnginePrivate::QGeoTiledMappingManagerEnginePrivate() - : thread_(0), - cacheHint_(QGeoTiledMappingManagerEngine::AllCaches), - tileCache_(0), - fetcher_(0) {} +: cacheHint_(QGeoTiledMappingManagerEngine::AllCaches), tileCache_(0), fetcher_(0) +{ +} QGeoTiledMappingManagerEnginePrivate::~QGeoTiledMappingManagerEnginePrivate() { delete tileCache_; - - // will delete fetcher and thread later - thread_->exit(); } #include "moc_qgeotiledmappingmanagerengine_p.cpp" diff --git a/src/location/maps/qgeotiledmappingmanagerengine_p_p.h b/src/location/maps/qgeotiledmappingmanagerengine_p_p.h index ae26ab46..dfdab6de 100644 --- a/src/location/maps/qgeotiledmappingmanagerengine_p_p.h +++ b/src/location/maps/qgeotiledmappingmanagerengine_p_p.h @@ -55,10 +55,8 @@ // #include <QSize> -#include <QList> #include <QHash> #include <QSet> -#include <QThread> #include "qgeotiledmappingmanagerengine_p.h" QT_BEGIN_NAMESPACE @@ -74,7 +72,6 @@ public: QGeoTiledMappingManagerEnginePrivate(); ~QGeoTiledMappingManagerEnginePrivate(); - QThread *thread_; QSize tileSize_; QSet<QGeoTiledMapData *> tileMaps_; QHash<QGeoTiledMapData *, QSet<QGeoTileSpec> > mapHash_; diff --git a/src/location/maps/qgeotilefetcher.cpp b/src/location/maps/qgeotilefetcher.cpp index a86c0a43..e0d6b6ea 100644 --- a/src/location/maps/qgeotilefetcher.cpp +++ b/src/location/maps/qgeotilefetcher.cpp @@ -50,21 +50,8 @@ QT_BEGIN_NAMESPACE -QGeoTileFetcher::QGeoTileFetcher(QGeoTiledMappingManagerEngine *engine, QObject *parent) - : QObject(parent), - d_ptr(new QGeoTileFetcherPrivate(engine)) -{ - Q_D(QGeoTileFetcher); - d->engine_ = engine; -} - -QGeoTileFetcher::~QGeoTileFetcher() -{ - Q_D(QGeoTileFetcher); - delete d; -} - -void QGeoTileFetcher::threadStarted() +QGeoTileFetcher::QGeoTileFetcher(QObject *parent) +: QObject(parent), d_ptr(new QGeoTileFetcherPrivate) { Q_D(QGeoTileFetcher); @@ -74,18 +61,10 @@ void QGeoTileFetcher::threadStarted() d->timer_.start(0, this); } -bool QGeoTileFetcher::init() -{ - return false; -} - -void QGeoTileFetcher::threadFinished() +QGeoTileFetcher::~QGeoTileFetcher() { - Q_D(QGeoTileFetcher); - d->enabled_ = false; - d->timer_.stop(); - this->deleteLater(); + delete d_ptr; } void QGeoTileFetcher::updateTileRequests(const QSet<QGeoTileSpec> &tilesAdded, @@ -213,8 +192,8 @@ void QGeoTileFetcher::handleReply(QGeoTiledMapReply *reply, const QGeoTileSpec & /******************************************************************************* *******************************************************************************/ -QGeoTileFetcherPrivate::QGeoTileFetcherPrivate(QGeoTiledMappingManagerEngine *engine) -: engine_(engine), enabled_(false) +QGeoTileFetcherPrivate::QGeoTileFetcherPrivate() +: enabled_(false) { } diff --git a/src/location/maps/qgeotilefetcher_p.h b/src/location/maps/qgeotilefetcher_p.h index d08092b9..36b6e0dc 100644 --- a/src/location/maps/qgeotilefetcher_p.h +++ b/src/location/maps/qgeotilefetcher_p.h @@ -72,12 +72,10 @@ class Q_LOCATION_EXPORT QGeoTileFetcher : public QObject Q_OBJECT public: - QGeoTileFetcher(QGeoTiledMappingManagerEngine *engine, QObject *parent = 0); + QGeoTileFetcher(QObject *parent = 0); virtual ~QGeoTileFetcher(); public Q_SLOTS: - void threadStarted(); - void threadFinished(); void updateTileRequests(const QSet<QGeoTileSpec> &tilesAdded, const QSet<QGeoTileSpec> &tilesRemoved); private Q_SLOTS: @@ -91,7 +89,6 @@ Q_SIGNALS: protected: void timerEvent(QTimerEvent *event); - virtual bool init(); QGeoTiledMappingManagerEngine::CacheAreas cacheHint() const; private: diff --git a/src/location/maps/qgeotilefetcher_p_p.h b/src/location/maps/qgeotilefetcher_p_p.h index c47f8f90..aa452413 100644 --- a/src/location/maps/qgeotilefetcher_p_p.h +++ b/src/location/maps/qgeotilefetcher_p_p.h @@ -73,11 +73,9 @@ class QGeoTiledMappingManagerEngine; class QGeoTileFetcherPrivate { public: - explicit QGeoTileFetcherPrivate(QGeoTiledMappingManagerEngine *engine); + QGeoTileFetcherPrivate(); virtual ~QGeoTileFetcherPrivate(); - QGeoTiledMappingManagerEngine *engine_; - bool enabled_; QBasicTimer timer_; QMutex queueMutex_; diff --git a/src/plugins/geoservices/nokia/qgeotiledmappingmanagerengine_nokia.cpp b/src/plugins/geoservices/nokia/qgeotiledmappingmanagerengine_nokia.cpp index 1db5c9f5..e7f1fec7 100644 --- a/src/plugins/geoservices/nokia/qgeotiledmappingmanagerengine_nokia.cpp +++ b/src/plugins/geoservices/nokia/qgeotiledmappingmanagerengine_nokia.cpp @@ -141,7 +141,9 @@ QGeoTiledMappingManagerEngineNokia::QGeoTiledMappingManagerEngineNokia( QMetaObject::invokeMethod(fetcher, "fetchCopyrightsData", Qt::QueuedConnection); } -QGeoTiledMappingManagerEngineNokia::~QGeoTiledMappingManagerEngineNokia() {} +QGeoTiledMappingManagerEngineNokia::~QGeoTiledMappingManagerEngineNokia() +{ +} void QGeoTiledMappingManagerEngineNokia::populateMapSchemes() { diff --git a/src/plugins/geoservices/nokia/qgeotilefetcher_nokia.cpp b/src/plugins/geoservices/nokia/qgeotilefetcher_nokia.cpp index 997eef64..ddd55ec4 100644 --- a/src/plugins/geoservices/nokia/qgeotilefetcher_nokia.cpp +++ b/src/plugins/geoservices/nokia/qgeotilefetcher_nokia.cpp @@ -86,44 +86,34 @@ namespace return mapScheme.startsWith("satellite") || mapScheme.startsWith("hybrid") || mapScheme.startsWith("terrain"); } } -QGeoTileFetcherNokia::QGeoTileFetcherNokia( - const QVariantMap ¶meters, - QGeoNetworkAccessManager *networkManager, - QGeoTiledMappingManagerEngine *engine, - const QSize &tileSize) - : QGeoTileFetcher(engine), - m_engineNokia(static_cast<QGeoTiledMappingManagerEngineNokia *>(engine)), - m_networkManager(networkManager), - m_parameters(parameters), - m_tileSize(tileSize), - m_copyrightsReply(0), - m_baseUriProvider(new QGeoUriProvider(this, parameters, "mapping.host", MAP_TILES_HOST)), - m_aerialUriProvider(new QGeoUriProvider(this, parameters, "mapping.host.aerial", MAP_TILES_HOST_AERIAL)) +QGeoTileFetcherNokia::QGeoTileFetcherNokia(const QVariantMap ¶meters, + QGeoNetworkAccessManager *networkManager, + QGeoTiledMappingManagerEngineNokia *engine, + const QSize &tileSize) +: QGeoTileFetcher(engine), m_engineNokia(engine), m_networkManager(networkManager), + m_tileSize(tileSize), m_copyrightsReply(0), + m_baseUriProvider(new QGeoUriProvider(this, parameters, "mapping.host", MAP_TILES_HOST)), + m_aerialUriProvider(new QGeoUriProvider(this, parameters, "mapping.host.aerial", MAP_TILES_HOST_AERIAL)) { Q_ASSERT(networkManager); m_networkManager->setParent(this); -} -QGeoTileFetcherNokia::~QGeoTileFetcherNokia() {} + m_applicationId = parameters.value(QStringLiteral("app_id")).toString(); + m_token = parameters.value(QStringLiteral("token")).toString(); +} -bool QGeoTileFetcherNokia::init() +QGeoTileFetcherNokia::~QGeoTileFetcherNokia() { - qsrand((uint)QTime::currentTime().msec()); - - if (m_parameters.contains("app_id")) { - m_applicationId = m_parameters.value("app_id").toString(); - } - - if (m_parameters.contains("token")) { - m_token = m_parameters.value("token").toString(); - } - return true; } QGeoTiledMapReply *QGeoTileFetcherNokia::getTileImage(const QGeoTileSpec &spec) { // TODO add error detection for if request.connectivityMode() != QGraphicsGeoMap::OnlineMode QString rawRequest = getRequestString(spec); + if (rawRequest.isEmpty()) { + return new QGeoTiledMapReply(QGeoTiledMapReply::UnknownError, + tr("Mapping manager no longer exists"), this); + } QNetworkRequest netRequest((QUrl(rawRequest))); // The extra pair of parens disambiguates this from a function declaration netRequest.setAttribute(QNetworkRequest::HttpPipeliningAllowedAttribute, true); @@ -137,6 +127,9 @@ QGeoTiledMapReply *QGeoTileFetcherNokia::getTileImage(const QGeoTileSpec &spec) QString QGeoTileFetcherNokia::getRequestString(const QGeoTileSpec &spec) { + if (!m_engineNokia) + return QString(); + static const QString http("http://"); static const QString path("/maptile/2.1/maptile/newest/"); static const QChar slash('/'); diff --git a/src/plugins/geoservices/nokia/qgeotilefetcher_nokia.h b/src/plugins/geoservices/nokia/qgeotilefetcher_nokia.h index 794b251f..a5e1801a 100644 --- a/src/plugins/geoservices/nokia/qgeotilefetcher_nokia.h +++ b/src/plugins/geoservices/nokia/qgeotilefetcher_nokia.h @@ -68,16 +68,10 @@ class QGeoTileFetcherNokia : public QGeoTileFetcher Q_OBJECT public: - QGeoTileFetcherNokia( - const QVariantMap ¶meters, - QGeoNetworkAccessManager *networkManager, - QGeoTiledMappingManagerEngine *engine, - const QSize &tileSize); - + QGeoTileFetcherNokia(const QVariantMap ¶meters, QGeoNetworkAccessManager *networkManager, + QGeoTiledMappingManagerEngineNokia *engine, const QSize &tileSize); ~QGeoTileFetcherNokia(); - bool init(); - QGeoTiledMapReply *getTileImage(const QGeoTileSpec &spec); QString token() const; @@ -96,7 +90,6 @@ private: QPointer<QGeoTiledMappingManagerEngineNokia> m_engineNokia; QGeoNetworkAccessManager *m_networkManager; - QVariantMap m_parameters; QSize m_tileSize; QString m_token; QNetworkReply *m_copyrightsReply; diff --git a/src/plugins/geoservices/osm/qgeotilefetcherosm.cpp b/src/plugins/geoservices/osm/qgeotilefetcherosm.cpp index 75c1492b..e149c65f 100644 --- a/src/plugins/geoservices/osm/qgeotilefetcherosm.cpp +++ b/src/plugins/geoservices/osm/qgeotilefetcherosm.cpp @@ -48,7 +48,7 @@ QT_BEGIN_NAMESPACE -QGeoTileFetcherOsm::QGeoTileFetcherOsm(QGeoTiledMappingManagerEngine *parent) +QGeoTileFetcherOsm::QGeoTileFetcherOsm(QObject *parent) : QGeoTileFetcher(parent), m_networkManager(new QNetworkAccessManager(this)), m_userAgent("Qt Location based application") { diff --git a/src/plugins/geoservices/osm/qgeotilefetcherosm.h b/src/plugins/geoservices/osm/qgeotilefetcherosm.h index ae88ab97..e705a2d9 100644 --- a/src/plugins/geoservices/osm/qgeotilefetcherosm.h +++ b/src/plugins/geoservices/osm/qgeotilefetcherosm.h @@ -54,7 +54,7 @@ class QGeoTileFetcherOsm : public QGeoTileFetcher Q_OBJECT public: - explicit QGeoTileFetcherOsm(QGeoTiledMappingManagerEngine *parent = 0); + QGeoTileFetcherOsm(QObject *parent = 0); void setUserAgent(const QByteArray &userAgent); diff --git a/tests/auto/geotestplugin/qgeotilefetcher_test.h b/tests/auto/geotestplugin/qgeotilefetcher_test.h index 9c939b9d..71ae8601 100644 --- a/tests/auto/geotestplugin/qgeotilefetcher_test.h +++ b/tests/auto/geotestplugin/qgeotilefetcher_test.h @@ -78,16 +78,15 @@ class QGeoTileFetcherTest: public QGeoTileFetcher { Q_OBJECT public: - QGeoTileFetcherTest(QGeoTiledMappingManagerEngine *engine, QObject *parent = 0) - : QGeoTileFetcher(engine, parent), - finishRequestImmediately_(false), - mappingReply_(0), - errorCode_(QGeoTiledMapReply::NoError) {} + QGeoTileFetcherTest(QObject *parent = 0) + : QGeoTileFetcher(parent), finishRequestImmediately_(false), mappingReply_(0), + errorCode_(QGeoTiledMapReply::NoError) + { + } bool init() { - if (parameters_.contains("finishRequestImmediately")) - finishRequestImmediately_ = parameters_.value("finishRequestImmediately").toBool(); + return true; } @@ -132,7 +131,8 @@ public: void setParams(const QVariantMap ¶meters) { - parameters_ = parameters; + if (parameters.contains(QStringLiteral("finishRequestImmediately"))) + finishRequestImmediately_ = parameters.value(QStringLiteral("finishRequestImmediately")).toBool(); } void setTileSize(QSize tileSize) @@ -173,7 +173,6 @@ private: QBasicTimer timer_; QGeoTiledMapReply::Error errorCode_; QString errorString_; - QVariantMap parameters_; QSize tileSize_; }; |