Bug 144732 – [GTK] RunLoop constructor should properly retrieve or establish the thread-default GMainContext

• Component: New Bugs
• Status: RESOLVED FIXED

[Zan Dobersek]

[GTK] RunLoop constructor should properly retrieve or establish the thread-default GMainContext

[Zan Dobersek]

Created attachment 252581 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 252581 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=252581&action=review

> Source/WTF/wtf/gtk/RunLoopGtk.cpp:40
> +    GMainContext* runLoopContext = g_main_context_get_thread_default();
> +    if (!runLoopContext) {
> +        if (!isMainThread()) {
> +            m_mainContext = adoptGRef(g_main_context_new());

This could be simplified by using g_main_context_ref_thread_default(), because it ensures a main context is returned.

> Source/WTF/wtf/gtk/RunLoopGtk.cpp:41
> +            g_main_context_push_thread_default(m_mainContext.get());

I'm not sure about doing this here, I think it should be done before running the loop, and pop it after it quits. Doing it here, means that if a thread doesn't use a RunLoop, but something like RunLoop::isMain() is called, RunLoop::current() creates a new RunLoop for the thread and pushes a new main context.

[Zan Dobersek]

Created attachment 252582 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 252582 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=252582&action=review

> Source/WTF/wtf/gtk/RunLoopGtk.cpp:66
> +        g_main_context_push_thread_default(mainContext);

So, are we doing this also for the main thread?

[Zan Dobersek]

Comment on attachment 252582 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=252582&action=review

>> Source/WTF/wtf/gtk/RunLoopGtk.cpp:66
>> +        g_main_context_push_thread_default(mainContext);
> 
> So, are we doing this also for the main thread?

Yes. g_main_context_push_thread_default() checks if the passed-in context is the global-default one, pushing a null pointer on the top of the stack in that case. That way g_main_context_get_thread_default() still returns a null pointer on the main thread.

[Zan Dobersek]

Created attachment 253110 [details]
Patch

[Zan Dobersek]

Comment on attachment 253110 [details]
Patch

Clearing flags on attachment: 253110

Committed r184333: <http://trac.webkit.org/changeset/184333>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.