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 <alexander.blasche@digia.com>

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 <qgeorouterequest.h>
 
+Q_DECLARE_METATYPE(QList<QGeoRoute>)
+
 QT_BEGIN_NAMESPACE
 
-QGeoRouteReplyNokia::QGeoRouteReplyNokia(const QGeoRouteRequest &request, const QList<QNetworkReply*> &replies, QObject *parent)
-        : QGeoRouteReply(request, parent),
-        m_replies(replies)
+QGeoRouteReplyNokia::QGeoRouteReplyNokia(const QGeoRouteRequest &request,
+                                         const QList<QNetworkReply *> &replies,
+                                         QObject *parent)
+:   QGeoRouteReply(request, parent), m_replies(replies), m_parsers(0)
 {
+    qRegisterMetaType<QList<QGeoRoute> >();
+
     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<QGeoRoute>)),
+            this, SLOT(appendResults(QList<QGeoRoute>)));
+    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<QNetworkReply *>(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<QGeoRoute>)),
+                this, SLOT(appendResults(QList<QGeoRoute>)));
+        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<QGeoRoute> &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<QGeoRoute> &routes);
+    void parserError(const QString &errorString);
 
 private:
-    QList<QNetworkReply*> m_replies;
+    QList<QNetworkReply *> 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 <QXmlStreamReader>
-#include <QIODevice>
 #include <QStringList>
 #include <QString>
+#include <QtCore/QThreadPool>
 
-#include <qgeoroute.h>
 #include <QtPositioning/QGeoRectangle>
+#include <QtLocation/QGeoRoute>
 
 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<QGeoRoute> 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 <QList>
-#include <QString>
-#include <QScopedPointer>
+#include <QtCore/QObject>
+#include <QtCore/QRunnable>
+#include <QtCore/QString>
+#include <QtCore/QList>
 
-#include <qgeoroutesegment.h>
-#include <qgeorouterequest.h>
-#include <qgeomaneuver.h>
+#include <QtLocation/QGeoRouteRequest>
+#include <QtLocation/QGeoRouteSegment>
+#include <QtLocation/QGeoManeuver>
 
 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<QGeoRoute> results() const;
-    QString errorString() const;
+    void parse(const QByteArray &data);
+    void run();
+
+signals:
+    void results(const QList<QGeoRoute> &routes);
+    void error(const QString &errorString);
 
 private:
     bool parseRootElement();
@@ -119,12 +125,14 @@ private:
     bool parseDynamicSpeedInfo(QGeoDynamicSpeedInfoContainer &speedInfo);
 
     QGeoRouteRequest m_request;
-    QScopedPointer<QXmlStreamReader> m_reader;
+    QByteArray m_data;
+    QXmlStreamReader *m_reader;
+
     QList<QGeoRoute> m_results;
-    QString m_errorString;
     QList<QGeoManeuverContainer> m_maneuvers;
     QList<QGeoRouteSegmentContainer> 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 <QMetaType>
 #include <QDebug>
 #include <QFile>
+#include <QSignalSpy>
+
+Q_DECLARE_METATYPE(QList<QGeoRoute>)
 
 QT_USE_NAMESPACE
 
@@ -58,7 +61,9 @@ public:
     tst_QGeoRouteXmlParser()
         : start(0.0, 0.0),
           end(1.0, 1.0)
-    {}
+    {
+        qRegisterMetaType<QList<QGeoRoute> >();
+    }
 
 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<QGeoRoute>)));
+
+        xp.parse(f.readAll());
 
-        QVERIFY(xp.parse(&f));
+        QTRY_COMPARE(resultsSpy.count(), 1);
+
+        QVariantList arguments = resultsSpy.first();
 
         // xml contains exactly 1 route
-        QList<QGeoRoute> results = xp.results();
+        QList<QGeoRoute> results = arguments.at(0).value<QList<QGeoRoute> >();
         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<QGeoRoute>)));
+
+        xp.parse(f.readAll());
+
+        QTRY_COMPARE(resultsSpy.count(), 1);
 
-        QVERIFY(xp.parse(&f));
+        QVariantList arguments = resultsSpy.first();
 
         // xml contains exactly 1 route
-        QList<QGeoRoute> results = xp.results();
+        QList<QGeoRoute> results = arguments.at(0).value<QList<QGeoRoute> >();
         QCOMPARE(results.size(), 1);
         QGeoRoute route = results.first();