[webkit-reviews] review granted: [Bug 72704] [Qt] Use QEvent for dispatchFunctionsFromMainThread() : [Attachment 115795] Patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has granted Patrick R. Gansterer
<paroga at paroga.com>'s request for review:
Bug 72704: [Qt] Use QEvent for dispatchFunctionsFromMainThread()
https://bugs.webkit.org/show_bug.cgi?id=72704

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

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=115795&action=review


r=me. QMetaObject::invokeMethod is implemented using posted events behind the
scenes and this patch removes the unnecessary layer of indirection (which does
things like setting the current QObject::sender, which we don't need). It would
be nice if you could fix the nitpicks before landing :)

> Source/JavaScriptCore/wtf/qt/MainThreadQt.cpp:36
> +#include <QtCore/QEvent>

In general we don't need #include <ModuelName/ClassName> includes and we should
just use #include <classname> or #include <header.h>.
The only place where we _need_ to use this pattern is in header files used for
the public API in Qt (ask me on IRC if you're curious :)

> Source/JavaScriptCore/wtf/qt/MainThreadQt.cpp:52
> +    s_mainThreadInvokerEventType = QEvent::registerEventType();

Nitpick: Technically it's QCoreEvent::registerEventType ;)

> Source/JavaScriptCore/wtf/qt/MainThreadQt.cpp:58
> +    if (e->type() == s_mainThreadInvokerEventType)
> +	   dispatchFunctionsFromMainThread();

Nitpick: Missing {} with "return" in the body? (no need to call QObject::event
for _our_ event type)