From d2628aeb6ed1ab30d1a825122dc80cfebb5093d2 Mon Sep 17 00:00:00 2001 From: Aaron McCarthy Date: Wed, 19 Feb 2014 14:52:07 +1000 Subject: Parse Nokia route response in helper thread. Complex and really long routes can take multiple seconds to parse. Use a helper thread to parse route responses to prevent blocking the main thread. Change-Id: I4130510ff15752427f31b429e53d2ab87fa1b84a Reviewed-by: Alex Blasche --- .../geoservices/nokia/qgeoroutereply_nokia.cpp | 104 ++++++++++++--------- .../geoservices/nokia/qgeoroutereply_nokia.h | 7 +- .../geoservices/nokia/qgeoroutexmlparser.cpp | 33 +++---- src/plugins/geoservices/nokia/qgeoroutexmlparser.h | 34 ++++--- tests/auto/nokia_services/routing/tst_routing.cpp | 3 +- .../qgeoroutexmlparser/tst_qgeoroutexmlparser.cpp | 29 +++++- 6 files changed, 124 insertions(+), 86 deletions(-) diff --git a/src/plugins/geoservices/nokia/qgeoroutereply_nokia.cpp b/src/plugins/geoservices/nokia/qgeoroutereply_nokia.cpp index 2245b208..72f2c60d 100644 --- a/src/plugins/geoservices/nokia/qgeoroutereply_nokia.cpp +++ b/src/plugins/geoservices/nokia/qgeoroutereply_nokia.cpp @@ -51,22 +51,21 @@ #include +Q_DECLARE_METATYPE(QList) + QT_BEGIN_NAMESPACE -QGeoRouteReplyNokia::QGeoRouteReplyNokia(const QGeoRouteRequest &request, const QList &replies, QObject *parent) - : QGeoRouteReply(request, parent), - m_replies(replies) +QGeoRouteReplyNokia::QGeoRouteReplyNokia(const QGeoRouteRequest &request, + const QList &replies, + QObject *parent) +: QGeoRouteReply(request, parent), m_replies(replies), m_parsers(0) { + qRegisterMetaType >(); + foreach (QNetworkReply *reply, m_replies) { - connect(reply, - SIGNAL(finished()), - this, - SLOT(networkFinished())); - - connect(reply, - SIGNAL(error(QNetworkReply::NetworkError)), - this, - SLOT(networkError(QNetworkReply::NetworkError))); + connect(reply, SIGNAL(finished()), this, SLOT(networkFinished())); + connect(reply, SIGNAL(error(QNetworkReply::NetworkError)), + this, SLOT(networkError(QNetworkReply::NetworkError))); } } @@ -77,7 +76,7 @@ QGeoRouteReplyNokia::~QGeoRouteReplyNokia() void QGeoRouteReplyNokia::abort() { - if (m_replies.isEmpty()) + if (m_replies.isEmpty() && !m_parsers) return; foreach (QNetworkReply *reply, m_replies) { @@ -85,6 +84,7 @@ void QGeoRouteReplyNokia::abort() reply->deleteLater(); } m_replies.clear(); + m_parsers = 0; } void QGeoRouteReplyNokia::networkFinished() @@ -93,53 +93,65 @@ void QGeoRouteReplyNokia::networkFinished() if (!reply) return; - if (reply->error() != QNetworkReply::NoError) { - // Removed because this is already done in networkError, which previously caused _two_ errors to be raised for every error. - //setError(QGeoRouteReply::CommunicationError, m_reply->errorString()); - //m_reply->deleteLater(); - //m_reply = 0; + if (reply->error() != QNetworkReply::NoError) return; - } - QGeoRouteXmlParser parser(request()); - if (parser.parse(reply)) { - addRoutes(parser.results()); - reply->deleteLater(); - m_replies.removeOne(reply); - if (m_replies.isEmpty()) - setFinished(true); - } else { - // add a qWarning with the actual parser.errorString() - setError(QGeoRouteReply::ParseError, "The response from the service was not in a recognisable format."); - abort(); - } + QGeoRouteXmlParser *parser = new QGeoRouteXmlParser(request()); + connect(parser, SIGNAL(results(QList)), + this, SLOT(appendResults(QList))); + connect(parser, SIGNAL(error(QString)), this, SLOT(parserError(QString))); + + ++m_parsers; + parser->parse(reply->readAll()); + + m_replies.removeOne(reply); + reply->deleteLater(); } void QGeoRouteReplyNokia::networkError(QNetworkReply::NetworkError error) { - Q_UNUSED(error) - QNetworkReply *reply = qobject_cast(sender()); if (!reply) return; - bool resolvedError = false; - if (QNetworkReply::UnknownContentError == error) { - QGeoRouteXmlParser parser(request()); - if (parser.parse(reply)) { - addRoutes(parser.results()); - reply->deleteLater(); - m_replies.removeOne(reply); - resolvedError = true; - if (m_replies.isEmpty()) - setFinished(true); - } - } + if (error == QNetworkReply::UnknownContentError) { + QGeoRouteXmlParser *parser = new QGeoRouteXmlParser(request()); + connect(parser, SIGNAL(results(QList)), + this, SLOT(appendResults(QList))); + connect(parser, SIGNAL(error(QString)), this, SLOT(parserError(QString))); + + ++m_parsers; + parser->parse(reply->readAll()); - if (!resolvedError) { + m_replies.removeOne(reply); + reply->deleteLater(); + } else { setError(QGeoRouteReply::CommunicationError, reply->errorString()); abort(); } } +void QGeoRouteReplyNokia::appendResults(const QList &routes) +{ + if (!m_parsers) + return; + + --m_parsers; + addRoutes(routes); + + if (!m_parsers && m_replies.isEmpty()) + setFinished(true); +} + +void QGeoRouteReplyNokia::parserError(const QString &errorString) +{ + Q_UNUSED(errorString) + + --m_parsers; + + setError(QGeoRouteReply::ParseError, + tr("The response from the service was not in a recognisable format.")); + abort(); +} + QT_END_NAMESPACE diff --git a/src/plugins/geoservices/nokia/qgeoroutereply_nokia.h b/src/plugins/geoservices/nokia/qgeoroutereply_nokia.h index 911e1920..d1684e83 100644 --- a/src/plugins/geoservices/nokia/qgeoroutereply_nokia.h +++ b/src/plugins/geoservices/nokia/qgeoroutereply_nokia.h @@ -54,6 +54,8 @@ QT_BEGIN_NAMESPACE +class QGeoRouteXmlParser; + class QGeoRouteReplyNokia : public QGeoRouteReply { Q_OBJECT @@ -66,9 +68,12 @@ public: private Q_SLOTS: void networkFinished(); void networkError(QNetworkReply::NetworkError error); + void appendResults(const QList &routes); + void parserError(const QString &errorString); private: - QList m_replies; + QList m_replies; + int m_parsers; }; QT_END_NAMESPACE diff --git a/src/plugins/geoservices/nokia/qgeoroutexmlparser.cpp b/src/plugins/geoservices/nokia/qgeoroutexmlparser.cpp index 60840ec6..e032f8d3 100644 --- a/src/plugins/geoservices/nokia/qgeoroutexmlparser.cpp +++ b/src/plugins/geoservices/nokia/qgeoroutexmlparser.cpp @@ -49,12 +49,12 @@ #include "qgeoroutexmlparser.h" #include -#include #include #include +#include -#include #include +#include QT_BEGIN_NAMESPACE @@ -74,28 +74,23 @@ QGeoRouteXmlParser::~QGeoRouteXmlParser() { } -bool QGeoRouteXmlParser::parse(QIODevice *source) +void QGeoRouteXmlParser::parse(const QByteArray &data) { - m_reader.reset(new QXmlStreamReader(source)); - - if (!parseRootElement()) { - m_errorString = m_reader->errorString(); - return false; - } - - m_errorString = ""; - - return true; + m_data = data; + QThreadPool::globalInstance()->start(this); } -QList QGeoRouteXmlParser::results() const +void QGeoRouteXmlParser::run() { - return m_results; -} + m_reader = new QXmlStreamReader(m_data); -QString QGeoRouteXmlParser::errorString() const -{ - return m_errorString; + if (!parseRootElement()) + emit error(m_reader->errorString()); + else + emit results(m_results); + + delete m_reader; + m_reader = 0; } bool QGeoRouteXmlParser::parseRootElement() diff --git a/src/plugins/geoservices/nokia/qgeoroutexmlparser.h b/src/plugins/geoservices/nokia/qgeoroutexmlparser.h index 466bca5e..7e6307e7 100644 --- a/src/plugins/geoservices/nokia/qgeoroutexmlparser.h +++ b/src/plugins/geoservices/nokia/qgeoroutexmlparser.h @@ -49,16 +49,17 @@ #ifndef QROUTEXMLPARSER_H #define QROUTEXMLPARSER_H -#include -#include -#include +#include +#include +#include +#include -#include -#include -#include +#include +#include +#include QT_BEGIN_NAMESPACE -class QIODevice; + class QXmlStreamReader; class QGeoRoute; class QGeoCoordinate; @@ -92,15 +93,20 @@ public: int baseTime; }; -class QGeoRouteXmlParser +class QGeoRouteXmlParser : public QObject, public QRunnable { + Q_OBJECT + public: QGeoRouteXmlParser(const QGeoRouteRequest &request); ~QGeoRouteXmlParser(); - bool parse(QIODevice *source); - QList results() const; - QString errorString() const; + void parse(const QByteArray &data); + void run(); + +signals: + void results(const QList &routes); + void error(const QString &errorString); private: bool parseRootElement(); @@ -119,12 +125,14 @@ private: bool parseDynamicSpeedInfo(QGeoDynamicSpeedInfoContainer &speedInfo); QGeoRouteRequest m_request; - QScopedPointer m_reader; + QByteArray m_data; + QXmlStreamReader *m_reader; + QList m_results; - QString m_errorString; QList m_maneuvers; QList m_segments; }; QT_END_NAMESPACE + #endif diff --git a/tests/auto/nokia_services/routing/tst_routing.cpp b/tests/auto/nokia_services/routing/tst_routing.cpp index 95d034da..dc77ba6c 100644 --- a/tests/auto/nokia_services/routing/tst_routing.cpp +++ b/tests/auto/nokia_services/routing/tst_routing.cpp @@ -521,11 +521,10 @@ void tst_nokia_routing::foobar_data() QTest::newRow("QNetworkReply::UnknownNetworkError") << int(QNetworkReply::UnknownNetworkError); QTest::newRow("QNetworkReply::UnknownProxyError") << int(QNetworkReply::UnknownProxyError); QTest::newRow("QNetworkReply::ProxyAuthenticationRequiredError") << int(QNetworkReply::ProxyAuthenticationRequiredError); - QTest::newRow("QNetworkReply::UnknownContentError") << int(QNetworkReply::UnknownContentError); QTest::newRow("QNetworkReply::ProtocolFailure") << int(QNetworkReply::ProtocolFailure); } -QTEST_APPLESS_MAIN(tst_nokia_routing) +QTEST_MAIN(tst_nokia_routing) #include "tst_routing.moc" diff --git a/tests/auto/qgeoroutexmlparser/tst_qgeoroutexmlparser.cpp b/tests/auto/qgeoroutexmlparser/tst_qgeoroutexmlparser.cpp index 9fb51cc6..1dde8b33 100644 --- a/tests/auto/qgeoroutexmlparser/tst_qgeoroutexmlparser.cpp +++ b/tests/auto/qgeoroutexmlparser/tst_qgeoroutexmlparser.cpp @@ -47,6 +47,9 @@ #include #include #include +#include + +Q_DECLARE_METATYPE(QList) QT_USE_NAMESPACE @@ -58,7 +61,9 @@ public: tst_QGeoRouteXmlParser() : start(0.0, 0.0), end(1.0, 1.0) - {} + { + qRegisterMetaType >(); + } private: // dummy values for creating the request object @@ -74,11 +79,18 @@ private slots: QGeoRouteRequest req(start, end); QGeoRouteXmlParser xp(req); + xp.setAutoDelete(false); + + QSignalSpy resultsSpy(&xp, SIGNAL(results(QList))); + + xp.parse(f.readAll()); - QVERIFY(xp.parse(&f)); + QTRY_COMPARE(resultsSpy.count(), 1); + + QVariantList arguments = resultsSpy.first(); // xml contains exactly 1 route - QList results = xp.results(); + QList results = arguments.at(0).value >(); QCOMPARE(results.size(), 1); QGeoRoute route = results.first(); @@ -121,11 +133,18 @@ private slots: QGeoRouteRequest req(start, end); QGeoRouteXmlParser xp(req); + xp.setAutoDelete(false); + + QSignalSpy resultsSpy(&xp, SIGNAL(results(QList))); + + xp.parse(f.readAll()); + + QTRY_COMPARE(resultsSpy.count(), 1); - QVERIFY(xp.parse(&f)); + QVariantList arguments = resultsSpy.first(); // xml contains exactly 1 route - QList results = xp.results(); + QList results = arguments.at(0).value >(); QCOMPARE(results.size(), 1); QGeoRoute route = results.first(); -- cgit v1.2.3