Summary: | [Qt] Enable binding of QObjects to JavaScript environment for inspector frontend | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jamey Hicks <jamey.hicks> | ||||||||||||
Component: | WebKit Qt | Assignee: | QtWebKit Unassigned <webkit-qt-unassigned> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | commit-queue, hausmann, kling, laszlo.gombos, webkit.review.bot | ||||||||||||
Priority: | P2 | Keywords: | Qt, QtTriaged | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | All | ||||||||||||||
OS: | All | ||||||||||||||
Attachments: |
|
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. Created attachment 61219 [details]
patch revised to address comments
Patch revised to address comments.
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.
Created attachment 61508 [details]
Patch
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.
Created attachment 62061 [details]
Patch
Comment on attachment 62061 [details]
Patch
WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:126
+ QVariant inspectorJavaScriptWindowObjects = inspector->property("_q_inspectorJavaScriptWindowObjects");
indentation error
Comment on attachment 62061 [details]
Patch
WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:128
+ inspectorPage->setProperty("_q_inspectorJavaScriptWindowObjects", inspectorJavaScriptWindowObjects);
and here as well
Created attachment 62180 [details]
Patch
We would like this included in QtWebKit 2.1 for use in on-device debugging and profiling. Comment on attachment 62180 [details] Patch Clearing flags on attachment: 62180 Committed r63899: <http://trac.webkit.org/changeset/63899> All reviewed patches have been landed. Closing bug. Revision r63899 cherry-picked into qtwebkit-2.1 with commit e4306d5c4e0a48a34152e7d2498feaf4654e2bf9 |
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.