[webkit-reviews] review denied: [Bug 79499] [GTK] Add support for nested event loops in RunLoop : [Attachment 128747] Patch Proposal

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Mario Sanchez Prada
<msanchez at igalia.com>'s request for review:
Bug 79499: [GTK] Add support for nested event loops in RunLoop
https://bugs.webkit.org/show_bug.cgi?id=79499

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

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


One thing that worries me about this patch is that RunLoop::mainLoop will
return the wrong GMainLoop when you are in an interior loop.
Perhapsm_runLoopMain should be replaced with a stack of loops?

> Source/WebCore/platform/gtk/RunLoopGtk.cpp:50
> +static GRefPtr<GMainLoop> currentEventLoop;

I think this should be a member of RunLoop. Isn't it important that this can be
used across different threads? You should probably call this currentMainLoop to
match the type.

> Source/WebCore/platform/gtk/RunLoopGtk.cpp:55
> +    static bool mainEventLoopIsRunning = false;
> +    if (!mainEventLoopIsRunning) {

mainEventLoopIsRunning can be replaced with a call to g_main_loop_is_running.