QmlDebug: Unify QPacket and QmlDebugStream and remove unused code

There is no point in having both as they serve the same purpose. Also,
most of the packet protocol API was never used or tested, and wrapping
each byte to be sent into a QPacket is wasteful and unnecessary.

Change-Id: Ia421eae33b644fe86a53bcd04092a84ea3000f0d
Reviewed-by: Joerg Bornemann <joerg.bornemann@theqtcompany.com>

src/libs/qmldebug/baseenginedebugclient.cpp

@@ -30,6 +30,7 @@
 
 #include "baseenginedebugclient.h"
 #include "qmldebugconstants.h"
+#include "qpacketprotocol.h"
 
 namespace QmlDebug {
 
@@ -182,7 +183,7 @@ void BaseEngineDebugClient::stateChanged(State state)
 
 void BaseEngineDebugClient::messageReceived(const QByteArray &data)
 {
-    QmlDebugStream ds(data);
+    QPacket ds(connection()->currentDataStreamVersion(), data);
     int queryId;
     QByteArray type;
     ds >> type >> queryId;
@@ -254,11 +255,10 @@ quint32 BaseEngineDebugClient::addWatch(const PropertyReference &property)
     quint32 id = 0;
     if (state() == Enabled) {
         id = getId();
-        QByteArray message;
-        QmlDebugStream ds(&message, QIODevice::WriteOnly);
+        QPacket ds(connection()->currentDataStreamVersion());
         ds << QByteArray("WATCH_PROPERTY") << id << property.m_objectDebugId
            << property.m_name.toUtf8();
-        sendMessage(message);
+        sendMessage(ds.data());
     }
     return id;
 }
@@ -276,10 +276,9 @@ quint32 BaseEngineDebugClient::addWatch(const ObjectReference &object,
     quint32 id = 0;
     if (state() == Enabled) {
         id = getId();
-        QByteArray message;
-        QmlDebugStream ds(&message, QIODevice::WriteOnly);
+        QPacket ds(connection()->currentDataStreamVersion());
         ds << QByteArray("WATCH_EXPR_OBJECT") << id << object.m_debugId << expr;
-        sendMessage(message);
+        sendMessage(ds.data());
     }
     return id;
 }
@@ -289,10 +288,9 @@ quint32 BaseEngineDebugClient::addWatch(int objectDebugId)
     quint32 id = 0;
     if (state() == Enabled) {
         id = getId();
-        QByteArray message;
-        QmlDebugStream ds(&message, QIODevice::WriteOnly);
+        QPacket ds(connection()->currentDataStreamVersion());
         ds << QByteArray("WATCH_OBJECT") << id << objectDebugId;
-        sendMessage(message);
+        sendMessage(ds.data());
     }
     return id;
 }
@@ -306,10 +304,9 @@ quint32 BaseEngineDebugClient::addWatch(const FileReference &/*file*/)
 void BaseEngineDebugClient::removeWatch(quint32 id)
 {
     if (state() == Enabled) {
-        QByteArray message;
-        QmlDebugStream ds(&message, QIODevice::WriteOnly);
+        QPacket ds(connection()->currentDataStreamVersion());
         ds << QByteArray("NO_WATCH") << id;
-        sendMessage(message);
+        sendMessage(ds.data());
     }
 }
 
@@ -318,10 +315,9 @@ quint32 BaseEngineDebugClient::queryAvailableEngines()
     quint32 id = 0;
     if (state() == Enabled) {
         id = getId();
-        QByteArray message;
-        QmlDebugStream ds(&message, QIODevice::WriteOnly);
+        QPacket ds(connection()->currentDataStreamVersion());
         ds << QByteArray("LIST_ENGINES") << id;
-        sendMessage(message);
+        sendMessage(ds.data());
     }
     return id;
 }
@@ -331,10 +327,9 @@ quint32 BaseEngineDebugClient::queryRootContexts(const EngineReference &engine)
     quint32 id = 0;
     if (state() == Enabled && engine.m_debugId != -1) {
         id = getId();
-        QByteArray message;
-        QmlDebugStream ds(&message, QIODevice::WriteOnly);
+        QPacket ds(connection()->currentDataStreamVersion());
         ds << QByteArray("LIST_OBJECTS") << id << engine.m_debugId;
-        sendMessage(message);
+        sendMessage(ds.data());
     }
     return id;
 }
@@ -344,11 +339,10 @@ quint32 BaseEngineDebugClient::queryObject(int objectId)
     quint32 id = 0;
     if (state() == Enabled && objectId != -1) {
         id = getId();
-        QByteArray message;
-        QmlDebugStream ds(&message, QIODevice::WriteOnly);
+        QPacket ds(connection()->currentDataStreamVersion());
         ds << QByteArray("FETCH_OBJECT") << id << objectId << false <<
               true;
-        sendMessage(message);
+        sendMessage(ds.data());
     }
     return id;
 }
@@ -358,11 +352,10 @@ quint32 BaseEngineDebugClient::queryObjectRecursive(int objectId)
     quint32 id = 0;
     if (state() == Enabled && objectId != -1) {
         id = getId();
-        QByteArray message;
-        QmlDebugStream ds(&message, QIODevice::WriteOnly);
+        QPacket ds(connection()->currentDataStreamVersion());
         ds << QByteArray("FETCH_OBJECT") << id << objectId << true <<
               true;
-        sendMessage(message);
+        sendMessage(ds.data());
     }
     return id;
 }
@@ -374,11 +367,10 @@ quint32 BaseEngineDebugClient::queryExpressionResult(int objectDebugId,
     quint32 id = 0;
     if (state() == Enabled && objectDebugId != -1) {
         id = getId();
-        QByteArray message;
-        QmlDebugStream ds(&message, QIODevice::WriteOnly);
+        QPacket ds(connection()->currentDataStreamVersion());
         ds << QByteArray("EVAL_EXPRESSION") << id << objectDebugId << expr
            << engineId;
-        sendMessage(message);
+        sendMessage(ds.data());
     }
     return id;
 }
@@ -393,11 +385,10 @@ quint32 BaseEngineDebugClient::setBindingForObject(
     quint32 id = 0;
     if (state() == Enabled && objectDebugId != -1) {
         id = getId();
-        QByteArray message;
-        QmlDebugStream ds(&message, QIODevice::WriteOnly);
+        QPacket ds(connection()->currentDataStreamVersion());
         ds << QByteArray("SET_BINDING") << id << objectDebugId << propertyName
            << bindingExpression << isLiteralValue << source << line;
-        sendMessage(message);
+        sendMessage(ds.data());
     }
     return id;
 }
@@ -409,10 +400,9 @@ quint32 BaseEngineDebugClient::resetBindingForObject(
     quint32 id = 0;
     if (state() == Enabled && objectDebugId != -1) {
         id = getId();
-        QByteArray message;
-        QmlDebugStream ds(&message, QIODevice::WriteOnly);
+        QPacket ds(connection()->currentDataStreamVersion());
         ds << QByteArray("RESET_BINDING") << id << objectDebugId << propertyName;
-        sendMessage(message);
+        sendMessage(ds.data());
     }
     return id;
 }
@@ -424,11 +414,10 @@ quint32 BaseEngineDebugClient::setMethodBody(
     quint32 id = 0;
     if (state() == Enabled && objectDebugId != -1) {
         id = getId();
-        QByteArray message;
-        QmlDebugStream ds(&message, QIODevice::WriteOnly);
+        QPacket ds(connection()->currentDataStreamVersion());
         ds << QByteArray("SET_METHOD_BODY") << id << objectDebugId
            << methodName << methodBody;
-        sendMessage(message);
+        sendMessage(ds.data());
     }
     return id;
 }
@@ -439,12 +428,11 @@ quint32 BaseEngineDebugClient::queryObjectsForLocation(
     quint32 id = 0;
     if (state() == Enabled) {
         id = getId();
-        QByteArray message;
-        QmlDebugStream ds(&message, QIODevice::WriteOnly);
+        QPacket ds(connection()->currentDataStreamVersion());
         ds << QByteArray("FETCH_OBJECTS_FOR_LOCATION") << id <<
               fileName << lineNumber << columnNumber << false <<
               true;
-        sendMessage(message);
+        sendMessage(ds.data());
     }
     return id;
 }

src/libs/qmldebug/declarativeenginedebugclient.cpp

@@ -31,6 +31,7 @@
 #include "declarativeenginedebugclient.h"
 #include "qmldebugconstants.h"
 #include "qmldebugclient.h"
+#include "qpacketprotocol.h"
 
 namespace QmlDebug {
 
@@ -50,11 +51,10 @@ quint32 DeclarativeEngineDebugClient::setBindingForObject(
     quint32 id = 0;
     if (state() == Enabled && objectDebugId != -1) {
         id = getId();
-        QByteArray message;
-        QmlDebugStream ds(&message, QIODevice::WriteOnly);
+        QPacket ds(connection()->currentDataStreamVersion());
         ds << QByteArray("SET_BINDING") << objectDebugId << propertyName
            << bindingExpression << isLiteralValue << source << line;
-        sendMessage(message);
+        sendMessage(ds.data());
     }
     return id;
 }
@@ -66,10 +66,9 @@ quint32 DeclarativeEngineDebugClient::resetBindingForObject(
     quint32 id = 0;
     if (state() == Enabled && objectDebugId != -1) {
         id = getId();
-        QByteArray message;
-        QmlDebugStream ds(&message, QIODevice::WriteOnly);
+        QPacket ds(connection()->currentDataStreamVersion());
         ds << QByteArray("RESET_BINDING") << objectDebugId << propertyName;
-        sendMessage(message);
+        sendMessage(ds.data());
     }
     return id;
 }
@@ -81,18 +80,17 @@ quint32 DeclarativeEngineDebugClient::setMethodBody(
     quint32 id = 0;
     if (state() == Enabled && objectDebugId != -1) {
         id = getId();
-        QByteArray message;
-        QmlDebugStream ds(&message, QIODevice::WriteOnly);
+        QPacket ds(connection()->currentDataStreamVersion());
         ds << QByteArray("SET_METHOD_BODY") << objectDebugId
            << methodName << methodBody;
-        sendMessage(message);
+        sendMessage(ds.data());
     }
     return id;
 }
 
 void DeclarativeEngineDebugClient::messageReceived(const QByteArray &data)
 {
-    QmlDebugStream ds(data);
+    QPacket ds(connection()->currentDataStreamVersion(), data);
     QByteArray type;
     ds >> type;
 

src/libs/qmldebug/qmldebugclient.cpp

@@ -39,7 +39,6 @@
 namespace QmlDebug {
 
 const int protocolVersion = 1;
-int QmlDebugClient::s_dataStreamVersion = QDataStream::Qt_4_7;
 
 const QString serverId = QLatin1String("QDeclarativeDebugServer");
 const QString clientId = QLatin1String("QDeclarativeDebugClient");
@@ -67,6 +66,9 @@ public:
     QHash <QString, float> serverPlugins;
     QHash<QString, QmlDebugClient *> plugins;
 
+    int currentDataStreamVersion;
+    int maximumDataStreamVersion;
+
     void advertisePlugins();
     void flush();
 
@@ -79,7 +81,9 @@ public slots:
 };
 
 QmlDebugConnectionPrivate::QmlDebugConnectionPrivate(QmlDebugConnection *c)
-    : QObject(c), q(c), protocol(0), device(0), gotHello(false)
+    : QObject(c), q(c), protocol(0), device(0), gotHello(false),
+    currentDataStreamVersion(QDataStream::Qt_4_7),
+    maximumDataStreamVersion(QDataStream::Qt_DefaultCompiledVersion)
 {
 }
 
@@ -88,17 +92,17 @@ void QmlDebugConnectionPrivate::advertisePlugins()
     if (!q->isOpen())
         return;
 
-    QPacket pack;
+    QPacket pack(currentDataStreamVersion);
     pack << serverId << 1 << plugins.keys();
-    protocol->send(pack);
+    protocol->send(pack.data());
     flush();
 }
 
 void QmlDebugConnectionPrivate::connected()
 {
-    QPacket pack;
-    pack << serverId << 0 << protocolVersion << plugins.keys() << QDataStream().version();
-    protocol->send(pack);
+    QPacket pack(currentDataStreamVersion);
+    pack << serverId << 0 << protocolVersion << plugins.keys() << maximumDataStreamVersion;
+    protocol->send(pack.data());
     flush();
 }
 
@@ -134,7 +138,7 @@ void QmlDebugConnectionPrivate::error(QAbstractSocket::SocketError socketError)
 void QmlDebugConnectionPrivate::readyRead()
 {
     if (!gotHello) {
-        QPacket pack = protocol->read();
+        QPacket pack(currentDataStreamVersion, protocol->read());
         QString name;
 
         pack >> name;
@@ -150,7 +154,7 @@ void QmlDebugConnectionPrivate::readyRead()
                     QStringList pluginNames;
                     QList<float> pluginVersions;
                     pack >> pluginNames;
-                    if (!pack.isEmpty())
+                    if (!pack.atEnd())
                         pack >> pluginVersions;
 
                     const int pluginNamesSize = pluginNames.size();
@@ -163,9 +167,8 @@ void QmlDebugConnectionPrivate::readyRead()
                     }
 
                     if (!pack.atEnd()) {
-                        pack >> QmlDebugClient::s_dataStreamVersion;
-                        if (QmlDebugClient::s_dataStreamVersion
-                                > QDataStream().version())
+                        pack >> currentDataStreamVersion;
+                        if (currentDataStreamVersion > maximumDataStreamVersion)
                             qWarning() << "Server returned invalid data stream version!";
                     }
                     validHello = true;
@@ -192,7 +195,7 @@ void QmlDebugConnectionPrivate::readyRead()
     }
 
     while (protocol && protocol->packetsAvailable()) {
-        QPacket pack = protocol->read();
+        QPacket pack(currentDataStreamVersion, protocol->read());
         QString name;
         pack >> name;
 
@@ -208,7 +211,7 @@ void QmlDebugConnectionPrivate::readyRead()
                 QStringList pluginNames;
                 QList<float> pluginVersions;
                 pack >> pluginNames;
-                if (!pack.isEmpty())
+                if (!pack.atEnd())
                     pack >> pluginVersions;
 
                 const int pluginNamesSize = pluginNames.size();
@@ -334,7 +337,15 @@ void QmlDebugConnection::connectToHost(const QString &hostName, quint16 port)
     socket->connectToHost(hostName, port);
 }
 
-//
+int QmlDebugConnection::currentDataStreamVersion() const
+{
+    return d->currentDataStreamVersion;
+}
+
+void QmlDebugConnection::setMaximumDataStreamVersion(int maximumVersion)
+{
+    d->maximumDataStreamVersion = maximumVersion;
+}
 
 QmlDebugClientPrivate::QmlDebugClientPrivate()
     : connection(0)
@@ -412,9 +423,9 @@ void QmlDebugClient::sendMessage(const QByteArray &message)
     if (state() != Enabled)
         return;
 
-    QPacket pack;
+    QPacket pack(d->connection->currentDataStreamVersion());
     pack << d->name << message;
-    d->connection->d->protocol->send(pack);
+    d->connection->d->protocol->send(pack.data());
     d->connection->d->flush();
 }
 
@@ -426,30 +437,6 @@ void QmlDebugClient::messageReceived(const QByteArray &)
 {
 }
 
-QmlDebugStream::QmlDebugStream()
-    : QDataStream()
-{
-    setVersion(QmlDebugClient::s_dataStreamVersion);
-}
-
-QmlDebugStream::QmlDebugStream(QIODevice *d)
-    : QDataStream(d)
-{
-    setVersion(QmlDebugClient::s_dataStreamVersion);
-}
-
-QmlDebugStream::QmlDebugStream(QByteArray *ba, QIODevice::OpenMode flags)
-    : QDataStream(ba, flags)
-{
-    setVersion(QmlDebugClient::s_dataStreamVersion);
-}
-
-QmlDebugStream::QmlDebugStream(const QByteArray &ba)
-    : QDataStream(ba)
-{
-    setVersion(QmlDebugClient::s_dataStreamVersion);
-}
-
 } // namespace QmlDebug
 
 #include <qmldebugclient.moc>

src/libs/qmldebug/qmldebugclient.h

@@ -56,6 +56,9 @@ public:
 
     void connectToHost(const QString &hostName, quint16 port);
 
+    int currentDataStreamVersion() const;
+    void setMaximumDataStreamVersion(int maximumVersion);
+
     bool isOpen() const;
     bool isConnecting() const;
     void close();
@@ -103,17 +106,7 @@ private:
     friend class QmlDebugConnectionPrivate;
     QScopedPointer<QmlDebugClientPrivate> d_ptr;
 
-public:
-    static int s_dataStreamVersion;
-};
 
-class QMLDEBUG_EXPORT QmlDebugStream : public QDataStream
-{
-public:
-    QmlDebugStream();
-    explicit QmlDebugStream(QIODevice *d);
-    QmlDebugStream(QByteArray *ba, QIODevice::OpenMode flags);
-    QmlDebugStream(const QByteArray &ba);
 };
 
 } // namespace QmlDebug

src/libs/qmldebug/qmlprofilertraceclient.cpp

@@ -31,6 +31,7 @@
 #include "qmlprofilertraceclient.h"
 #include "qmlenginecontrolclient.h"
 #include "qdebugmessageclient.h"
+#include "qpacketprotocol.h"
 
 namespace QmlDebug {
 
@@ -76,12 +77,11 @@ static const int GAP_TIME = 150;
 
 void QmlProfilerTraceClientPrivate::sendRecordingStatus(int engineId)
 {
-    QByteArray ba;
-    QmlDebugStream stream(&ba, QIODevice::WriteOnly);
+    QPacket stream(q->connection()->currentDataStreamVersion());
     stream << recording << engineId; // engineId -1 is OK. It means "all of them"
     if (recording)
         stream << requestedFeatures << flushInterval;
-    q->sendMessage(ba);
+    q->sendMessage(stream.data());
 }
 
 QmlProfilerTraceClient::QmlProfilerTraceClient(QmlDebugConnection *client, quint64 features)
@@ -201,8 +201,7 @@ void QmlProfilerTraceClient::stateChanged(State /*status*/)
 
 void QmlProfilerTraceClient::messageReceived(const QByteArray &data)
 {
-    QByteArray rwData = data;
-    QmlDebugStream stream(&rwData, QIODevice::ReadOnly);
+    QPacket stream(connection()->currentDataStreamVersion(), data);
 
     qint64 time;
     int messageType;

src/libs/qmldebug/qpacketprotocol.cpp

@@ -30,7 +30,6 @@
 
 #include "qpacketprotocol.h"
 
-#include <qbuffer.h>
 #include <qelapsedtimer.h>
 
 namespace QmlDebug {
@@ -111,8 +110,6 @@ public:
 
         QObject::connect(this, &QPacketProtocolPrivate::readyRead,
                          parent, &QPacketProtocol::readyRead);
-        QObject::connect(this, &QPacketProtocolPrivate::packetWritten,
-                         parent, &QPacketProtocol::packetWritten);
         QObject::connect(this, &QPacketProtocolPrivate::invalidPacket,
                          parent, &QPacketProtocol::invalidPacket);
         QObject::connect(dev, &QIODevice::readyRead,
@@ -125,7 +122,6 @@ public:
 
 signals:
     void readyRead();
-    void packetWritten();
     void invalidPacket();
 
 public slots:
@@ -147,7 +143,6 @@ public slots:
             } else {
                 bytes -= sendingPackets.at(0);
                 sendingPackets.removeFirst();
-                emit packetWritten();
             }
         }
     }
@@ -217,70 +212,22 @@ QPacketProtocol::QPacketProtocol(QIODevice *dev, QObject *parent)
     Q_ASSERT(dev);
 }
 
-/*!
-  Destroys the QPacketProtocol instance.
- */
-QPacketProtocol::~QPacketProtocol()
-{
-}
-
-/*!
-  Returns the maximum packet size allowed.  By default this is
-  2,147,483,647 bytes.
-
-  If a packet claiming to be larger than the maximum packet size is received,
-  the QPacketProtocol::invalidPacket() signal is emitted.
-
-  \sa QPacketProtocol::setMaximumPacketSize()
- */
-qint32 QPacketProtocol::maximumPacketSize() const
-{
-    return d->maxPacketSize;
-}
-
-/*!
-  Sets the maximum allowable packet size to \a max.
-
-  \sa QPacketProtocol::maximumPacketSize()
- */
-qint32 QPacketProtocol::setMaximumPacketSize(qint32 max)
-{
-    if (max > (signed)sizeof(qint32))
-        d->maxPacketSize = max;
-    return d->maxPacketSize;
-}
-
-/*!
-  Returns a streamable object that is transmitted on destruction.  For example
-
-  \code
-  protocol.send() << "Hello world" << 123;
-  \endcode
-
-  will send a packet containing "Hello world" and 123.  To construct more
-  complex packets, explicitly construct a QPacket instance.
- */
-QPacketAutoSend QPacketProtocol::send()
-{
-    return QPacketAutoSend(this);
-}
-
 /*!
   Transmits the packet \a p.
  */
-void QPacketProtocol::send(const QPacket & p)
+void QPacketProtocol::send(const QByteArray &p)
 {
-    if (p.b.isEmpty())
+    if (p.isEmpty())
         return; // We don't send empty packets
 
-    qint64 sendSize = p.b.size() + sizeof(qint32);
+    qint64 sendSize = p.size() + sizeof(qint32);
 
     d->sendingPackets.append(sendSize);
     qint32 sendSize32 = sendSize;
     qint64 writeBytes = d->dev->write((char *)&sendSize32, sizeof(qint32));
     Q_ASSERT(writeBytes == sizeof(qint32));
-    writeBytes = d->dev->write(p.b);
-    Q_ASSERT(writeBytes == p.b.size());
+    writeBytes = d->dev->write(p);
+    Q_ASSERT(writeBytes == p.size());
     Q_UNUSED(writeBytes); // For building in release mode.
 }
 
@@ -292,26 +239,16 @@ qint64 QPacketProtocol::packetsAvailable() const
     return d->packets.count();
 }
 
-/*!
-  Discards any unread packets.
-  */
-void QPacketProtocol::clear()
-{
-    d->packets.clear();
-}
-
 /*!
   Returns the next unread packet, or an invalid QPacket instance if no packets
   are available.  This function does NOT block.
   */
-QPacket QPacketProtocol::read()
+QByteArray QPacketProtocol::read()
 {
     if (0 == d->packets.count())
-        return QPacket();
+        return QByteArray();
 
-    QPacket rv(d->packets.at(0));
-    d->packets.removeFirst();
-    return rv;
+    return d->packets.takeFirst();
 }
 
 
@@ -357,14 +294,6 @@ bool QPacketProtocol::waitForReadyRead(int msecs)
     } while (true);
 }
 
-/*!
-  Returns the QIODevice passed to the QPacketProtocol constructor.
-*/
-QIODevice *QPacketProtocol::device()
-{
-    return d->dev;
-}
-
 /*!
   \fn void QPacketProtocol::readyRead()
 
@@ -380,13 +309,6 @@ QIODevice *QPacketProtocol::device()
   further packets will be received.
  */
 
-/*!