diff options
author | Paolo Angelelli <paolo.angelelli@qt.io> | 2016-09-14 16:28:38 +0200 |
---|---|---|
committer | Edward Welbourne <edward.welbourne@qt.io> | 2016-11-29 10:52:47 +0000 |
commit | 0a552f285fdc3760d9755109ceef3b87392a308d (patch) | |
tree | 943e169f0c002d8e9f74badd0f3ad7a452229e4d /src/plugins/geoservices/esri | |
parent | 9b76e151b897698e285e4e39158b5934dc498bd1 (diff) | |
download | qtlocation-0a552f285fdc3760d9755109ceef3b87392a308d.tar.gz |
Refactoring: removing m_reply from GeoReply classes
This patch tries to simplify the code removing the contained m_reply
from all the Geo[Map,Route,Geocode,Places]Reply classes.
The need for m_reply was associated to the "abort" method, but this
can be solved by emitting a signal in the superclass abort() method,
and connecting that to QNetworkReply::abort() in the constructor.
Since QNetworkReplyHttpImpl always sends an OperationCanceledError
it should then be safe to call deleteLater() on the network reply
in the slot connected to QNetworkReply::error
This patch also prevents the series of
"QCoreApplication::postEvent: Unexpected null receiver" warnings
that are generated due to deletingLater already deleted objects
(abort() emits an error, the reply is destroyed inside the
onError slot, but also in the abort() method).
Finally, this patch removes the setFinished() call in
QGeoRouteReply::abort() since the documentation does not mention
this, and all the subclasses do not perform this operation and
emit the corresponding signal.
tst_qgeoroutereply has been adapted accordingly.
Change-Id: I226ee163e7bed784dd7f0da1522e651459543bca
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Diffstat (limited to 'src/plugins/geoservices/esri')
6 files changed, 54 insertions, 131 deletions
diff --git a/src/plugins/geoservices/esri/geocodereply_esri.cpp b/src/plugins/geoservices/esri/geocodereply_esri.cpp index a7ad9368..f1dac184 100644 --- a/src/plugins/geoservices/esri/geocodereply_esri.cpp +++ b/src/plugins/geoservices/esri/geocodereply_esri.cpp @@ -51,11 +51,17 @@ QT_BEGIN_NAMESPACE GeoCodeReplyEsri::GeoCodeReplyEsri(QNetworkReply *reply, OperationType operationType, QObject *parent) : - QGeoCodeReply(parent), m_reply(reply), m_operationType(operationType) + QGeoCodeReply(parent), m_operationType(operationType) { - connect(m_reply, SIGNAL(finished()), this, SLOT(networkReplyFinished())); - connect(m_reply, SIGNAL(error(QNetworkReply::NetworkError)), + if (!reply) { + setError(UnknownError, QStringLiteral("Null reply")); + return; + } + connect(reply, SIGNAL(finished()), this, SLOT(networkReplyFinished())); + connect(reply, SIGNAL(error(QNetworkReply::NetworkError)), this, SLOT(networkReplyError(QNetworkReply::NetworkError))); + connect(this, &QGeoCodeReply::aborted, reply, &QNetworkReply::abort); + connect(this, &QObject::destroyed, reply, &QObject::deleteLater); setLimit(1); setOffset(0); @@ -63,49 +69,25 @@ GeoCodeReplyEsri::GeoCodeReplyEsri(QNetworkReply *reply, OperationType operation GeoCodeReplyEsri::~GeoCodeReplyEsri() { - if (m_reply) - m_reply->deleteLater(); -} - -void GeoCodeReplyEsri::abort() -{ - if (!m_reply) - return; - - m_reply->abort(); - QGeoCodeReply::abort(); - - m_reply->deleteLater(); - m_reply = Q_NULLPTR; } void GeoCodeReplyEsri::networkReplyError(QNetworkReply::NetworkError error) { Q_UNUSED(error) - - if (!m_reply) - return; - - setError(QGeoCodeReply::CommunicationError, m_reply->errorString()); - - m_reply->deleteLater(); - m_reply = Q_NULLPTR; + QNetworkReply *reply = static_cast<QNetworkReply *>(sender()); + reply->deleteLater(); + setError(QGeoCodeReply::CommunicationError, reply->errorString()); } void GeoCodeReplyEsri::networkReplyFinished() { - if (!m_reply) - return; + QNetworkReply *reply = static_cast<QNetworkReply *>(sender()); + reply->deleteLater(); - if (m_reply->error() != QNetworkReply::NoError) - { - setError(QGeoCodeReply::CommunicationError, m_reply->errorString()); - m_reply->deleteLater(); - m_reply = Q_NULLPTR; + if (reply->error() != QNetworkReply::NoError) return; - } - QJsonDocument document = QJsonDocument::fromJson(m_reply->readAll()); + QJsonDocument document = QJsonDocument::fromJson(reply->readAll()); if (document.isObject()) { QJsonObject object = document.object(); @@ -148,9 +130,6 @@ void GeoCodeReplyEsri::networkReplyFinished() } else { setError(QGeoCodeReply::CommunicationError, QStringLiteral("Unknown document")); } - - m_reply->deleteLater(); - m_reply = Q_NULLPTR; } QGeoLocation GeoCodeReplyEsri::parseAddress(const QJsonObject& object) diff --git a/src/plugins/geoservices/esri/geocodereply_esri.h b/src/plugins/geoservices/esri/geocodereply_esri.h index 4434b7dc..a857599b 100644 --- a/src/plugins/geoservices/esri/geocodereply_esri.h +++ b/src/plugins/geoservices/esri/geocodereply_esri.h @@ -58,9 +58,7 @@ public: public: GeoCodeReplyEsri(QNetworkReply *reply, OperationType operationType, QObject *parent = Q_NULLPTR); - virtual ~GeoCodeReplyEsri(); - - void abort() Q_DECL_OVERRIDE; + ~GeoCodeReplyEsri(); inline OperationType operationType() const; @@ -72,7 +70,6 @@ private Q_SLOTS: QGeoLocation parseCandidate(const QJsonObject &candidate); private: - QNetworkReply *m_reply; OperationType m_operationType; }; diff --git a/src/plugins/geoservices/esri/georoutereply_esri.cpp b/src/plugins/geoservices/esri/georoutereply_esri.cpp index 4a7d5c67..811ffd0d 100644 --- a/src/plugins/geoservices/esri/georoutereply_esri.cpp +++ b/src/plugins/geoservices/esri/georoutereply_esri.cpp @@ -48,43 +48,32 @@ QT_BEGIN_NAMESPACE GeoRouteReplyEsri::GeoRouteReplyEsri(QNetworkReply *reply, const QGeoRouteRequest &request, QObject *parent) : - QGeoRouteReply(request, parent), m_reply(reply) + QGeoRouteReply(request, parent) { - connect(m_reply, SIGNAL(finished()), this, SLOT(networkReplyFinished())); - connect(m_reply, SIGNAL(error(QNetworkReply::NetworkError)), + if (!reply) { + setError(UnknownError, QStringLiteral("Null reply")); + return; + } + connect(reply, SIGNAL(finished()), this, SLOT(networkReplyFinished())); + connect(reply, SIGNAL(error(QNetworkReply::NetworkError)), this, SLOT(networkReplyError(QNetworkReply::NetworkError))); + connect(this, &QGeoRouteReply::aborted, reply, &QNetworkReply::abort); + connect(this, &QObject::destroyed, reply, &QObject::deleteLater); } GeoRouteReplyEsri::~GeoRouteReplyEsri() { - if (m_reply) - m_reply->deleteLater(); -} - -void GeoRouteReplyEsri::abort() -{ - if (!m_reply) - return; - - m_reply->abort(); - m_reply->deleteLater(); - m_reply = Q_NULLPTR; } void GeoRouteReplyEsri::networkReplyFinished() { - if (!m_reply) - return; + QNetworkReply *reply = static_cast<QNetworkReply *>(sender()); + reply->deleteLater(); - if (m_reply->error() != QNetworkReply::NoError) - { - setError(QGeoRouteReply::CommunicationError, m_reply->errorString()); - m_reply->deleteLater(); - m_reply = Q_NULLPTR; + if (reply->error() != QNetworkReply::NoError) return; - } - QJsonDocument document = QJsonDocument::fromJson(m_reply->readAll()); + QJsonDocument document = QJsonDocument::fromJson(reply->readAll()); GeoRouteJsonParserEsri parser(document); if (parser.isValid()) @@ -94,21 +83,14 @@ void GeoRouteReplyEsri::networkReplyFinished() } else { setError(QGeoRouteReply::ParseError, parser.errorString()); } - - m_reply->deleteLater(); - m_reply = Q_NULLPTR; } void GeoRouteReplyEsri::networkReplyError(QNetworkReply::NetworkError error) { Q_UNUSED(error) - - if (!m_reply) - return; - - setError(QGeoRouteReply::CommunicationError, m_reply->errorString()); - m_reply->deleteLater(); - m_reply = Q_NULLPTR; + QNetworkReply *reply = static_cast<QNetworkReply *>(sender()); + reply->deleteLater(); + setError(QGeoRouteReply::CommunicationError, reply->errorString()); } QT_END_NAMESPACE diff --git a/src/plugins/geoservices/esri/georoutereply_esri.h b/src/plugins/geoservices/esri/georoutereply_esri.h index 6e97ee9f..049dc3ba 100644 --- a/src/plugins/geoservices/esri/georoutereply_esri.h +++ b/src/plugins/geoservices/esri/georoutereply_esri.h @@ -51,16 +51,11 @@ class GeoRouteReplyEsri : public QGeoRouteReply public: GeoRouteReplyEsri(QNetworkReply *reply, const QGeoRouteRequest &request, QObject *parent = Q_NULLPTR); - virtual ~GeoRouteReplyEsri(); - - void abort() Q_DECL_OVERRIDE; + ~GeoRouteReplyEsri(); private Q_SLOTS: void networkReplyFinished(); void networkReplyError(QNetworkReply::NetworkError error); - -private: - QNetworkReply *m_reply; }; QT_END_NAMESPACE diff --git a/src/plugins/geoservices/esri/geotiledmapreply_esri.cpp b/src/plugins/geoservices/esri/geotiledmapreply_esri.cpp index e0816c15..f4431bf0 100644 --- a/src/plugins/geoservices/esri/geotiledmapreply_esri.cpp +++ b/src/plugins/geoservices/esri/geotiledmapreply_esri.cpp @@ -49,50 +49,32 @@ static const unsigned char gifSignature[] = {0x47, 0x49, 0x46, 0x38, 0x00}; GeoTiledMapReplyEsri::GeoTiledMapReplyEsri(QNetworkReply *reply, const QGeoTileSpec &spec, QObject *parent) : - QGeoTiledMapReply(spec, parent), m_reply(reply) + QGeoTiledMapReply(spec, parent) { - connect(m_reply, SIGNAL(finished()), this, SLOT(networkReplyFinished())); - connect(m_reply, SIGNAL(error(QNetworkReply::NetworkError)), + if (!reply) { + setError(UnknownError, QStringLiteral("Null reply")); + return; + } + connect(reply, SIGNAL(finished()), this, SLOT(networkReplyFinished())); + connect(reply, SIGNAL(error(QNetworkReply::NetworkError)), this, SLOT(networkReplyError(QNetworkReply::NetworkError))); - connect(m_reply, SIGNAL(destroyed()), this, SLOT(replyDestroyed())); + connect(this, &QGeoTiledMapReply::aborted, reply, &QNetworkReply::abort); + connect(this, &QObject::destroyed, reply, &QObject::deleteLater); } GeoTiledMapReplyEsri::~GeoTiledMapReplyEsri() { - if (m_reply) { - m_reply->deleteLater(); - m_reply = Q_NULLPTR; - } -} - -void GeoTiledMapReplyEsri::abort() -{ - if (!m_reply) - return; - - m_reply->abort(); - QGeoTiledMapReply::abort(); -} - -void GeoTiledMapReplyEsri::replyDestroyed() -{ - m_reply = Q_NULLPTR; } void GeoTiledMapReplyEsri::networkReplyFinished() { - if (!m_reply) - return; + QNetworkReply *reply = static_cast<QNetworkReply *>(sender()); + reply->deleteLater(); - if (m_reply->error() != QNetworkReply::NoError) - { - setError(QGeoTiledMapReply::CommunicationError, m_reply->errorString()); - m_reply->deleteLater(); - m_reply = Q_NULLPTR; + if (reply->error() != QNetworkReply::NoError) return; - } - QByteArray const& imageData = m_reply->readAll(); + QByteArray const& imageData = reply->readAll(); bool validFormat = true; if (imageData.startsWith(reinterpret_cast<const char*>(pngSignature))) @@ -108,22 +90,16 @@ void GeoTiledMapReplyEsri::networkReplyFinished() setMapImageData(imageData); setFinished(true); - - m_reply->deleteLater(); - m_reply = Q_NULLPTR; } void GeoTiledMapReplyEsri::networkReplyError(QNetworkReply::NetworkError error) { - if (!m_reply) - return; - - if (error != QNetworkReply::OperationCanceledError) - setError(QGeoTiledMapReply::CommunicationError, m_reply->errorString()); - - setFinished(true); - m_reply->deleteLater(); - m_reply = Q_NULLPTR; + QNetworkReply *reply = static_cast<QNetworkReply *>(sender()); + reply->deleteLater(); + if (error == QNetworkReply::OperationCanceledError) + setFinished(true); + else + setError(QGeoTiledMapReply::CommunicationError, reply->errorString()); } QT_END_NAMESPACE diff --git a/src/plugins/geoservices/esri/geotiledmapreply_esri.h b/src/plugins/geoservices/esri/geotiledmapreply_esri.h index 32a35698..b459e943 100644 --- a/src/plugins/geoservices/esri/geotiledmapreply_esri.h +++ b/src/plugins/geoservices/esri/geotiledmapreply_esri.h @@ -52,17 +52,11 @@ class GeoTiledMapReplyEsri : public QGeoTiledMapReply public: GeoTiledMapReplyEsri(QNetworkReply *reply, const QGeoTileSpec &spec, QObject *parent = Q_NULLPTR); - virtual ~GeoTiledMapReplyEsri(); - - void abort() Q_DECL_OVERRIDE; + ~GeoTiledMapReplyEsri(); private Q_SLOTS: - void replyDestroyed(); void networkReplyFinished(); void networkReplyError(QNetworkReply::NetworkError error); - -private: - QNetworkReply *m_reply; }; QT_END_NAMESPACE |