[Webkit-unassigned] [Bug 43988] Web Inspector: Remote Web Inspector support for QtWebKit

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=43988


Kenneth Rohde Christiansen <kenneth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #64874|review?                     |review-
               Flag|                            |




--- Comment #9 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2010-08-25 04:37:20 PST ---
(From update of attachment 64874)
WebKit/qt/Api/qwebinspector.cpp:199
 +  /*! \internal */
In Qt we normally don't write this on one line.

WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:235
 +          || (m_inspectedWebPage && m_inspectedWebPage->d->inspectorServerPort())) {
no reason to put this on two lines. lines below are wider :-)

WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:238
 +              || !m_inspectedWebPage->d
do you really need to check if it has the private? Seems more like an assert to me

WebKit/qt/WebCoreSupport/InspectorServerQt.cpp:151
 +          new HttpRequestHandler(mTcpConnection, this);
put this on the same line

WebKit/qt/WebCoreSupport/InspectorServerQt.cpp:162
 +      mEndOfHeaders = false;    
We use m_ not just m, please fix.

WebKit/qt/WebCoreSupport/InspectorServerQt.cpp:189
 +  
please remove this newline

WebKit/qt/WebCoreSupport/InspectorServerQt.cpp:257
 +          int rc = file.open(QIODevice::ReadOnly);
rc meaning what?

WebKit/qt/WebCoreSupport/InspectorServerQt.cpp:255
 +          QString path = QLatin1String(":") + mPath;
Wouldn't using .arg() be more effecient?

WebKit/qt/WebCoreSupport/InspectorServerQt.cpp:262
 +          int code = file.exists() ? 200 : 404;
Dont we have defines for these numbers in webcore?

WebKit/qt/WebCoreSupport/InspectorServerQt.cpp:311
 +  
strange and useless newline inside beginning of if(

WebKit/qt/WebCoreSupport/InspectorServerQt.cpp:329
 +          mData = mData.mid(len+2);
would need spaces around +

WebKit/qt/WebCoreSupport/InspectorServerQt.h:23
 +  class QTcpServer;
These should not be defined above the includes

WebKit/qt/WebCoreSupport/InspectorServerQt.h:40
 +          public:
strange indentation of public:

WebKit/qt/WebCoreSupport/InspectorServerQt.h:46
 +      void registerPage(QWebPage *page, InspectorClientQt* client);
* should be left aligned

WebKit/qt/WebCoreSupport/InspectorServerQt.h:47
 +      void unregisterPage(QWebPage *page);
same here

WebKit/qt/WebCoreSupport/InspectorServerQt.h:62
 +      private slots:
wrong indentation! should be above the private fields

WebKit/qt/WebCoreSupport/InspectorServerQt.h:63
 +      void gotNewConnection();
receivedNewConnection? establishedNewConnection... check what is used for QSocket and friends

WebKit/qt/WebCoreSupport/InspectorServerQt.h:69
 +          public:
wrong indentation

WebKit/qt/WebCoreSupport/InspectorServerQt.h:81
 +          public:
again!

WebKit/qt/WebCoreSupport/InspectorServerQt.h:86
 +      virtual int webSocketSend(const char *payload, size_t len);
length

WebKitTools/QtTestBrowser/launcherwindow.cpp:97
 +  
no need for two newlines, one will do

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.