[webkit-reviews] review granted: [Bug 130027] [GLIB] Add GMainLoopSource class to wrap idle and timeout sources : [Attachment 226675] Updated patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has granted Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 130027: [GLIB] Add GMainLoopSource class to wrap idle and timeout sources
https://bugs.webkit.org/show_bug.cgi?id=130027

Attachment 226675: Updated patch
https://bugs.webkit.org/attachment.cgi?id=226675&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=226675&action=review


Looks good for landing with Zan's suggestions.

> Source/WTF/wtf/gobject/GMainLoopSource.cpp:46
> +GMainLoopSource& GMainLoopSource::create()
> +{
> +    return *new GMainLoopSource(DeleteOnDestroy);
> +}
> +
> +GMainLoopSource::GMainLoopSource()
> +    : m_deleteOnDestroy(DoNotDeleteOnDestroy)
> +{
> +}
> +
> +GMainLoopSource::GMainLoopSource(DeleteOnDestroyType deleteOnDestroy)
> +    : m_deleteOnDestroy(deleteOnDestroy)
> +{
> +}

This is quite clever.

> Source/WTF/wtf/gobject/GMainLoopSource.cpp:194
> +    bool retval = m_socketCallback(condition);
> +    if (!retval && source == m_source.get())
> +	   destroy();

Nit: I think you should use returnValue instead of retval.