[Development] Qt 5.3 Feature freeze is coming quite soon...

[Simon Hausmann]

(removing releasing and other dev subscribed folks from CC)

On Thursday 16. January 2014 12.51.28 Kurt Pattyn wrote:
> I would like to propose the QtWebSockets module as a new feature for Qt 5.3
> (see https://qt.gitorious.org/qtplayground/websockets/source/master)
> 
> There are a number of requests in Jira:
> https://bugreports.qt-project.org/issues/?jql=labels%20%3D%20websockets
> asking to include web socket functionality in Qt, so maybe it is a good
> time to include it now. Besides that, EnginIo could make use of this module
> as well (currently it uses an own implementation of web sockets).
> 
> QtWebSockets is fully RFC6455 compliant (see
> http://tools.ietf.org/html/rfc6455) and is successfully tested against the
> latest Autobahn TestSuite. Performance is OK as well (test report is not
> included because of too big).
> 
> There is a C++ interface as well as a QML module.
> The C++ API consists of the following classes:
> QWebSocket
> QWebSocketServer
> QWebSocketProtocol
> 
> The QML API consists of the following component:
> WebSocket (included in the Qt.WebSockets 1.0 module).
> 
> I see 2 options: either add the functionality to the QtNetwork module, or
> add it as a Qt add-on. Maybe adding it to QtNetwork would be a ‘natural’
> place: QWebSocket would then live besides QTcpSocket, QWebSocketServer
> would live besides QTcpServer.
> 
> The module compiles on all reference platforms, has automated unit tests and
> manual unit tests (against Autobahn TestSuite), is fully documented, and
> has a number of examples. The module is included in the CI system of Qt
> (see http://testresults.qt-project.org/ci/WebSockets_master_Integration/).
> 
> 
> Please raise your votes.

I think it would be a great feature to have in Qt.

I've had a brief look at the API and I have a few comments:

* qwebsocketprotocol.h:
   * V_Unknow should probably be V_Unknow_n_ :)
   * CC_GOING_AWAY, etc: This should probably use camel casing instead of all upper case, in order to be consistent with the rest of the Qt API.
   * I suggest to remove inline Version currentVersion() { return V_LATEST; } - the function adds no value IMHO as the latest enum value will always be that.

* qwebsocket.h:

  * qint64 QWebSocket::write(const char *message)
    This function appears to send "message" as a text message and expects utf-8 encoding. But unfortunately I think it looks misleading because QIODevice::write has the same signature and means binary content - no assumption about encodings are done. I suggest to rename the
method to writeTextMessage(const QString &) and just writeBinaryMessage(const QByteArray &) - without any const char * overloads. It makes it very easy to understand the client code when reading it ("ahh, it's sending a text message" or "ahh, it's sending a binary message").

   * The API appears to miss a complete the notion of fragments. writeMessage appears to always send a frame, but on the receiving end the concept of fragments appears - this looks inconsistent. I'll elaborate a bit more about that separately.

I can think of two different ways of using web sockets:

1) Sending/receiving messages of a given protocol
2) Sending/receiving streams

It appears that the web socket protocol encapsulates both, with the notion of messages consisting of a variable (potentially unlimited) number of fragments. I think it is important to make both use-cases very easy. When I'm implementing
a message based protocol (for example chat client), I really want the underlying framework to deal with fragments and only deliver entire frames to me. When implementing for example a video streaming service, I need full control (on both ends)
over fragmentation. And to make it worse, it seems that over the same connection I could be sending a stream (variable fragments of the same frame) intermixed with single fragments. Is my interpretation of the spec correct here?


Either way what I'm missing from the API is making these two use-cases easier. Perhaps it would make sense to offer convenience, such as a boolean property to indicate that all text frames should only be signalled (textFrameReceived) as entire
frame instead of fragments.

I have more thoughts on this, but let's start the discussion here maybe :)

Simon