[webkit-reviews] review denied: [Bug 43988] Web Inspector: Remote Web Inspector support for QtWebKit : [Attachment 65715] Patch

[bugzilla-daemon at webkit.org]

Ariya Hidayat <ariya.hidayat at gmail.com> has denied Jamey Hicks
<jamey.hicks at nokia.com>'s request for review:
Bug 43988: Web Inspector: Remote Web Inspector support for QtWebKit
https://bugs.webkit.org/show_bug.cgi?id=43988

Attachment 65715: Patch
https://bugs.webkit.org/attachment.cgi?id=65715&action=review

------- Additional Comments from Ariya Hidayat <ariya.hidayat at gmail.com>
First question: does everything still compile if we build without inspector
support?

WebKit/qt/WebCoreSupport/InspectorServerQt.cpp:89
 +	    sServer = new InspectorServerQt();
This is not deleted anywhere. To avoid intentional leak, make
QCoreApplication::instance as the parent object so it gets deleted by the
application on exit.

WebKit/qt/WebCoreSupport/InspectorServerQt.cpp:286
 +	m_tcpConnection = 0;
Any reason to only nullify this and not delete it? Although it will be deleted
by the owning server socket, it will stay around in memory until the server
socket dies.

WebKit/qt/WebCoreSupport/InspectorServerQt.cpp:151
 +	QTcpSocket* m_tcpConnection = m_tcpServer->nextPendingConnection();
This is a local variable, don't use m_ prefix (to avoid confusion with
variables of the same name in another class).

WebKit/qt/WebCoreSupport/InspectorServerQt.cpp:324
 +	    size_t length = pos - 1;
What happens if pos is 0 or 1?