[webkit-reviews] review denied: [Bug 41995] [Qt] Enable binding of QObjects to JavaScript environment for inspector frontend : [Attachment 61508] Patch

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has denied Jamey Hicks
<jamey.hicks at nokia.com>'s request for review:
Bug 41995: [Qt] Enable binding of QObjects to JavaScript environment for
inspector frontend
https://bugs.webkit.org/show_bug.cgi?id=41995

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

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
WebKit/qt/ChangeLog:14
 +	    dynamic property "q_inspector_js_objects" on the
That is not normally how we name our private dynamic properties, plus I find
the name quite non-descriptive. I believe the others are prefixed with _qt_ or
_qt_webkit_ - you will have to check.

WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:81
 +	    if (!qvJsObjectMap.isValid())
qvJsObjectMap - please come up with a more descriptive name

WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:85
 +	    for (QMap<QString, QVariant>::const_iterator it =
jsObjectMap.constBegin(); it != jsObjectMap.constEnd(); ++it) {
I would move QMap<QString, QVariant>::const_iterator it =
jsObjectMap.constBegin() outside the look.