[webkit-reviews] review denied: [Bug 107887] [Gtk] RunLoop::run shuold run current thread's run loop. : [Attachment 184936] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Seulgi Kim
<seulgikim at company100.net>'s request for review:
Bug 107887: [Gtk] RunLoop::run shuold run current thread's run loop.
https://bugs.webkit.org/show_bug.cgi?id=107887

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

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


Looks pretty reasonable, but there is a memory leak in this new code, I think.

> Source/WebCore/platform/gtk/RunLoopGtk.cpp:35
>  #include <glib.h>
>  
> +#include <wtf/MainThread.h>
> +
>  namespace WebCore {

You should have these includes be all one block.

> Source/WebCore/platform/gtk/RunLoopGtk.cpp:40
> +    m_runLoopContext = isMainThread() ? g_main_context_default() :
g_main_context_new();

g_main_context_default does not return a new reference while g_main_context_new
does. You are leaking the new main loop here. You should do:

m_runLoopContext = isMainThread() ? g_main_context_default() :
adoptGRef(g_main_context_new());