[Webkit-unassigned] [Bug 116408] [Qt] Support Object construction for window properties added using QtWebKitBridge
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat May 25 04:39:42 PDT 2013
https://bugs.webkit.org/show_bug.cgi?id=116408
--- Comment #3 from Vivek Galatage <vivekg at webkit.org> 2013-05-25 04:38:10 PST ---
(From update of attachment 202883)
View in context: https://bugs.webkit.org/attachment.cgi?id=202883&action=review
> Source/WebCore/bridge/qt/qt_instance.cpp:48
> +static int getConstructMethodIndex(QObject* obj)
How about making this inline? Also can we move this method in QtInstance it self and name it
class QtInstance : public Instance {
...
QObject* getObject() const { return m_object.data(); }
QObject* hashKey() const { return m_hashkey; }
inline int constructMethodIndex() {
QObject* object = getObject();
if (!object)
return -1;
...
}
}
> Source/WebCore/bridge/qt/qt_instance.cpp:256
> + if (index >= 0) {
You are asserting it above. So I guess this is not required may be!
> Source/WebCore/bridge/qt/qt_instance.cpp:257
> + QMetaMethod m = obj->metaObject()->method(index);
How about naming m -> constructMethod to be more verbose?
> Source/WebCore/bridge/qt/qt_instance.cpp:265
> + void * qargs[3];
Extra white space between void & *
> Source/WebCore/bridge/qt/qt_instance.cpp:267
> + QObject* qobject = 0;
Can you name qobject to some more verbose name may be customQObject etc?
> Source/WebCore/bridge/qt/qt_instance.cpp:272
> + for (unsigned i = 0; i < args.size(); i++)
++i is preferred in WebKit.
> Source/WebCore/bridge/qt/qt_instance.h:85
> +
I am not sure but rest of the WebKit (including in Source/WebKit/qt) has the macro OVERRIDE as the suffix to the virtual methods which is missing here.
> Source/WebKit/qt/tests/qobjectbridge/tst_qobjectbridge.cpp:619
> + TestConstructObject(int param1, const QString& param2)
Use descriptive names instead of param1 and param2 (if possible here)
> Source/WebKit/qt/tests/qobjectbridge/tst_qobjectbridge.cpp:783
> + "var obj = new TestConstructObject%1(%1,'TestObject%1'); "
Missing tests for when var obj = new TestConstructObject() is invoked when the said custom object is not available. Please add the negative cases if appropriate in the context.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list