Bug 41995 – [Qt] Enable binding of QObjects to JavaScript environment for inspector frontend

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Jamey Hicks]

Created attachment 61104 [details]
patch to enable binding QObject to the javascript environment of a QWebInspector

The attached patch enables QObjects to be bound to global variables in the javascript environment of the inspector frontend. This is useful for writing alternate inspector frontends in javascript which need additional native functionality. It's a lighter-weight alternative from using an NPAPI plugin.

The code which instantiates QWebInspector uses this by setting the dynamic property "q_inspector_js_objects" on the QWebInspector. The value of this property should be a QMap<QString,QVariant>mapping global variable name to QObject.

I have used this to add support for eclipse/chrome remote debugging in QtTestBrowser.

[Andreas Kling]

Comment on attachment 61104 [details]
patch to enable binding QObject to the javascript environment of a QWebInspector

First off, every patch needs a ChangeLog entry.
You can use WebKitTools/Scripts/prepare-ChangeLog to generate most of it.

>WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:79
> +          QVariant qvJsObjectMap = property("_q_inspector_js_objects");
Missing #ifndef QT_NO_PROPERTIES

>WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:80
> +          if (qvJsObjectMap.isValid()) {
WebKit normally uses early-return style, so this would be:
if (!qvJsObjectMap.isValid())
    return;

>WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:123
> +       QVariant qvJsObjectMap = inspector->property("_q_inspector_js_objects");
One too many spaces in the indentation here.

r- for missing QT_NO_PROPERTIES check. Change itself looks reasonable to me.

[Jamey Hicks]

Created attachment 61219 [details]
patch revised to address comments

Patch revised to address comments.

[WebKit Review Bot]

Attachment 61219 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit/qt/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:13:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:14:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:15:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:16:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:18:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:20:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:21:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 11 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jamey Hicks]

Created attachment 61508 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 61508 [details]
Patch

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.

[Jamey Hicks]

Created attachment 62061 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 62061 [details]
Patch

WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:126
 +       QVariant inspectorJavaScriptWindowObjects = inspector->property("_q_inspectorJavaScriptWindowObjects");
indentation error

[Kenneth Rohde Christiansen]

Comment on attachment 62061 [details]
Patch

WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:128
 +        inspectorPage->setProperty("_q_inspectorJavaScriptWindowObjects", inspectorJavaScriptWindowObjects);
and here as well

[Jamey Hicks]

Created attachment 62180 [details]
Patch

[Jamey Hicks]

We would like this included in QtWebKit 2.1 for use in on-device debugging and profiling.

[WebKit Commit Bot]

Comment on attachment 62180 [details]
Patch

Clearing flags on attachment: 62180

Committed r63899: <http://trac.webkit.org/changeset/63899>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Hausmann]

Revision r63899 cherry-picked into qtwebkit-2.1 with commit e4306d5c4e0a48a34152e7d2498feaf4654e2bf9