Bug 154444 – [ThreadedCompositor] Opening the inspector in a window causes a crash.

• Component: WebKitGTK
• Status: RESOLVED FIXED

[ChangSeok Oh]

1. Build webkitgtk+ with --threaded-compositor
2. Run the mini browser.
3. Run the inspector through ctrl+i or popup menu.
4. Click the button, 'open in a window' next to the close button, "X"

#0  0x00007fffef74c803 in _g_log_abort (breakpoint=1)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmessages.c:315
#1  g_logv (log_domain=0x7fffec3e70ae "Gdk", log_level=G_LOG_LEVEL_ERROR, format=<optimized out>, 
    args=args@entry=0x7fff8bffe4a8)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmessages.c:1041
#2  0x00007fffef74c962 in g_log (log_domain=log_domain@entry=0x7fffec3e70ae "Gdk", 
    log_level=log_level@entry=G_LOG_LEVEL_ERROR, format=format@entry=0x7fffec403fe7 "%s")
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmessages.c:1079
#3  0x00007fffec3bf77f in _gdk_x11_display_error_event (display=display@entry=0x47b000, 
    error=error@entry=0x7fff8bffe650)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/gtk+-3.16.4/gdk/x11/gdkdisplay-x11.c:2552
#4  0x00007fffec3ca329 in gdk_x_error (xdisplay=0x46b6c0, error=0x7fff8bffe650)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/gtk+-3.16.4/gdk/x11/gdkmain-x11.c:303
#5  0x00007fffee00539d in _XError () from /lib64/libX11.so.6
#6  0x00007fffee002227 in handle_error () from /lib64/libX11.so.6
#7  0x00007fffee0033a8 in _XReply () from /lib64/libX11.so.6
#8  0x00007fffedfe7c4e in XGetGeometry () from /lib64/libX11.so.6
#9  0x00007ffff6d37fec in WebCore::GLContextGLX::defaultFrameBufferSize() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#10 0x00007ffff5da6175 in WebKit::ThreadedCompositor::ensureGLContext() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#11 0x00007ffff5da6318 in WebKit::ThreadedCompositor::renderLayerTree() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#12 0x00007ffff5da9992 in WebKit::CompositingRunLoop::updateTimerFired() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#13 0x00007ffff44586fa in WTF::RunLoop::TimerBase::TimerBase(WTF::RunLoop&)::{lambda(void*)#1}::_FUN(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#14 0x00007fffef74581a in g_main_dispatch (context=0x7fff80000900)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:3122
#15 g_main_context_dispatch (context=context@entry=0x7fff80000900)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:3737
#16 0x00007fffef745b98 in g_main_context_iterate (context=0x7fff80000900, block=block@entry=1, 
    dispatch=dispatch@entry=1, self=<optimized out>)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:3808
#17 0x00007fffef745eb2 in g_main_loop_run (loop=0x7fff80001240)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:4002
#18 0x00007ffff4458aa0 in WTF::RunLoop::run() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#19 0x00007ffff5da691b in WebKit::ThreadedCompositor::runCompositingThread() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#20 0x00007ffff442a395 in WTF::threadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#21 0x00007ffff445673a in WTF::wtfThreadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#22 0x00007ffff302e60a in start_thread () from /lib64/libpthread.so.0
#23 0x00007fffead8ba4d in clone () from /lib64/libc.so.6

[Carlos Garcia Campos]

Similar bt to bug #149568. This could be related to the web view reparenting, the view is unrealized/realized causing the redirected window to be destroyed/created. My guess is that there's a race making the web process use the x window after being destroyed by the ui process.

[ChangSeok Oh]

(In reply to comment #1)
> Similar bt to bug #149568. This could be related to the web view
> reparenting, the view is unrealized/realized causing the redirected window
> to be destroyed/created. My guess is that there's a race making the web
> process use the x window after being destroyed by the ui process.

Thanks for your comment, Carlos. You are right. The root cause is trying gl rendering after destroying the redirected window. I think it causes Bug 154071 as well. Any idea is welcome. =)

[Carlos Garcia Campos]

Created attachment 272106 [details]
Patch

The threaded compositor doesn't handle the case of destroying the native surface handle and keeps using the GL context for the destroyed native surface. I think this patch also fixes bug #154071, since now when the web view is destroyed and unrealize method destroys the surface the threaded compositor correctly handles this case.

[ChangSeok Oh]

(In reply to comment #3)
> Created attachment 272106 [details]
> Patch
> 
> The threaded compositor doesn't handle the case of destroying the native
> surface handle and keeps using the GL context for the destroyed native
> surface. I think this patch also fixes bug #154071, since now when the web
> view is destroyed and unrealize method destroys the surface the threaded
> compositor correctly handles this case.

Thanks for this patch! I am also working on this though, you are faster than me. =) At glance, your apporach looks pretty different from mine. I don't have time to look into the detail now. I believe yoon will review the patch. ;)

[Carlos Garcia Campos]

(In reply to comment #4)
> (In reply to comment #3)
> > Created attachment 272106 [details]
> > Patch
> > 
> > The threaded compositor doesn't handle the case of destroying the native
> > surface handle and keeps using the GL context for the destroyed native
> > surface. I think this patch also fixes bug #154071, since now when the web
> > view is destroyed and unrealize method destroys the surface the threaded
> > compositor correctly handles this case.
> 
> Thanks for this patch! I am also working on this though, you are faster than
> me. =) At glance, your apporach looks pretty different from mine. I don't
> have time to look into the detail now. I believe yoon will review the patch.
> ;)

What approach were you following? It could be better than mine.

[ChangSeok Oh]

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > Created attachment 272106 [details]
> > > Patch
> > > 
> > > The threaded compositor doesn't handle the case of destroying the native
> > > surface handle and keeps using the GL context for the destroyed native
> > > surface. I think this patch also fixes bug #154071, since now when the web
> > > view is destroyed and unrealize method destroys the surface the threaded
> > > compositor correctly handles this case.
> > 
> > Thanks for this patch! I am also working on this though, you are faster than
> > me. =) At glance, your apporach looks pretty different from mine. I don't
> > have time to look into the detail now. I believe yoon will review the patch.
> > ;)
> 
> What approach were you following? It could be better than mine.

My idea is to destroy only the compositing thread and recreate it without touching others like glContext and CoordinatedGraphicsScene, I have a rough patch doing that, but it will take some more time until being ready to share. Let me share it with you when it is ready.
BTW, I am still seeing a crash even after applying your patch. Try to repeatedly open/close the inspector repeatedly by ctrl + shift + i or open/close it in a new window over again.

[ChangSeok Oh]

Created attachment 272314 [details]
Alternative apporach

KaL, I thought something like this. But I don't think this is the best. Anyway please have a look. I hope this to inspire you. =)

[Carlos Garcia Campos]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > (In reply to comment #3)
> > > > Created attachment 272106 [details]
> > > > Patch
> > > > 
> > > > The threaded compositor doesn't handle the case of destroying the native
> > > > surface handle and keeps using the GL context for the destroyed native
> > > > surface. I think this patch also fixes bug #154071, since now when the web
> > > > view is destroyed and unrealize method destroys the surface the threaded
> > > > compositor correctly handles this case.
> > > 
> > > Thanks for this patch! I am also working on this though, you are faster than
> > > me. =) At glance, your apporach looks pretty different from mine. I don't
> > > have time to look into the detail now. I believe yoon will review the patch.
> > > ;)
> > 
> > What approach were you following? It could be better than mine.
> 
> My idea is to destroy only the compositing thread and recreate it without
> touching others like glContext and CoordinatedGraphicsScene, I have a rough
> patch doing that, but it will take some more time until being ready to
> share. Let me share it with you when it is ready.
> BTW, I am still seeing a crash even after applying your patch. Try to
> repeatedly open/close the inspector repeatedly by ctrl + shift + i or
> open/close it in a new window over again.

I'm unable to make it crash with my patch applied.

[Carlos Garcia Campos]

Comment on attachment 272314 [details]
Alternative apporach

So, this approach is similar indeed, but a bit more aggressive since it destroys the thread which cleans up not only the gl context but all gl resources. Since the thread termination is sync there's no risk of the ui process destroys the native handle while the main loop is still running in the compositor thread, so I think it's safe. The thing is whether we really need to destroy/create the thread and all its resources. Reparenting a web view is not that common in the end, so maybe it's a safer approach.

[ChangSeok Oh]

(In reply to comment #8)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > (In reply to comment #4)
> > > > (In reply to comment #3)
> > > > > Created attachment 272106 [details]
> > > > > Patch
> > > > > 
> > > > > The threaded compositor doesn't handle the case of destroying the native
> > > > > surface handle and keeps using the GL context for the destroyed native
> > > > > surface. I think this patch also fixes bug #154071, since now when the web
> > > > > view is destroyed and unrealize method destroys the surface the threaded
> > > > > compositor correctly handles this case.
> > > > 
> > > > Thanks for this patch! I am also working on this though, you are faster than
> > > > me. =) At glance, your apporach looks pretty different from mine. I don't
> > > > have time to look into the detail now. I believe yoon will review the patch.
> > > > ;)
> > > 
> > > What approach were you following? It could be better than mine.
> > 
> > My idea is to destroy only the compositing thread and recreate it without
> > touching others like glContext and CoordinatedGraphicsScene, I have a rough
> > patch doing that, but it will take some more time until being ready to
> > share. Let me share it with you when it is ready.
> > BTW, I am still seeing a crash even after applying your patch. Try to
> > repeatedly open/close the inspector repeatedly by ctrl + shift + i or
> > open/close it in a new window over again.
> 
> I'm unable to make it crash with my patch applied.

Hrm. I am not sure that the crash is related with your change, but it surely happens. I don't know the condition yet, but fortunately I got a backtrace.

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ff6b968b700 (LWP 28787)]
0x00007ff716eba471 in WebKit::CoordinatedBackingStoreTile::setBackBuffer(WebCore::IntRect const&, WebCore::IntRect const&, WTF::PassRefPtr<WebCore::CoordinatedSurface>, WebCore::IntPoint const&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
(gdb) bt
#0  0x00007ff716eba471 in WebKit::CoordinatedBackingStoreTile::setBackBuffer(WebCore::IntRect const&, WebCore::IntRect const&, WTF::PassRefPtr<WebCore::CoordinatedSurface>, WebCore::IntPoint const&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#1  0x00007ff716eba57b in WebKit::CoordinatedBackingStore::updateTile(unsigned int, WebCore::IntRect const&, WebCore::IntRect const&, WTF::PassRefPtr<WebCore::CoordinatedSurface>, WebCore::IntPoint const&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#2  0x00007ff716ec05d4 in WebKit::CoordinatedGraphicsScene::updateTilesIfNeeded(WebCore::TextureMapperLayer*, WebCore::CoordinatedGraphicsLayerState const&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#3  0x00007ff716ec1b58 in WebKit::CoordinatedGraphicsScene::setLayerState(unsigned int, WebCore::CoordinatedGraphicsLayerState const&) () from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#4  0x00007ff716ec2575 in WebKit::CoordinatedGraphicsScene::commitSceneState(WebCore::CoordinatedGraphicsState const&) () from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#5  0x00007ff716ebe241 in WebKit::CoordinatedGraphicsScene::syncRemoteContent() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#6  0x00007ff716ebe710 in WebKit::CoordinatedGraphicsScene::paintToCurrentGLContext(WebCore::TransformationMatrix const&, float, WebCore::FloatRect const&, WebCore::Color const&, bool, WebCore::FloatPoint const&, unsigned int) () from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#7  0x00007ff716ec56c3 in WebKit::ThreadedCompositor::renderLayerTree() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#8  0x00007ff716ec8f12 in WebKit::CompositingRunLoop::updateTimerFired() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#9  0x00007ff7155776fa in WTF::RunLoop::TimerBase::TimerBase(WTF::RunLoop&)::{lambda(void*)#1}::_FUN(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#10 0x00007ff71086481a in g_main_dispatch (context=0x7ff6a8000900)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:3122
#11 g_main_context_dispatch (context=context@entry=0x7ff6a8000900)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:3737
#12 0x00007ff710864b98 in g_main_context_iterate (context=0x7ff6a8000900, block=block@entry=1, 
    dispatch=dispatch@entry=1, self=<optimized out>)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:3808
#13 0x00007ff710864eb2 in g_main_loop_run (loop=0x7ff6a8001240)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:4002
#14 0x00007ff715577aa0 in WTF::RunLoop::run() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#15 0x00007ff716ec5ea8 in WebKit::ThreadedCompositor::runCompositingThread() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#16 0x00007ff715549395 in WTF::threadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#17 0x00007ff71557573a in WTF::wtfThreadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#18 0x00007ff71414d60a in start_thread () from /lib64/libpthread.so.0
#19 0x00007ff70bea6a4d in clone () from /lib64/libc.so.6

[Carlos Garcia Campos]

Do you have the bt of the other threads? or at least the main thread

[ChangSeok Oh]

(In reply to comment #11)
> Do you have the bt of the other threads? or at least the main thread

Here it goes.

Thread 12 (Thread 0x7f475affd700 (LWP 31877)):
#0  0x00007f47b8c2c27d in nanosleep () from /lib64/libpthread.so.0
#1  0x00007f47ba054ea0 in bmalloc::Heap::scavenge(std::unique_lock<bmalloc::StaticMutex>&, std::chrono::duration<long, std::ratio<1l, 1000l> >) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#2  0x00007f47ba054f2f in bmalloc::Heap::concurrentScavenge() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#3  0x00007f47ba055c2e in bmalloc::AsyncTask<bmalloc::Heap, void (bmalloc::Heap::*)()>::threadRunLoop() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#4  0x00007f47b120cf30 in ?? () from /lib64/libstdc++.so.6
#5  0x00007f47b8c2360a in start_thread () from /lib64/libpthread.so.0
#6  0x00007f47b097ca4d in clone () from /lib64/libc.so.6

Thread 11 (Thread 0x7f4760a90700 (LWP 31834)):
#0  0x00007f47b0970fdd in poll () from /lib64/libc.so.6
#1  0x00007f47b533ab34 in g_main_context_poll (priority=2147483647, n_fds=1, fds=0x7f475c0014a0, 
    timeout=<optimized out>, context=0x7f475c000900)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:4103
#2  g_main_context_iterate (context=0x7f475c000900, block=block@entry=1, dispatch=dispatch@entry=1, 
    self=<optimized out>)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:3803
#3  0x00007f47b533aeb2 in g_main_loop_run (loop=0x7f475c001240)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:4002
#4  0x00007f47ba04daa0 in WTF::RunLoop::run() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#5  0x00007f47ba04df5b in std::_Function_handler<void (), WTF::WorkQueue::platformInitialize(char const*, WTF::WorkQueue::Type, WTF::WorkQueue::QOS)::{lambda()#1}>::_M_invoke(std::_Any_data const&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#6  0x00007f47ba01f395 in WTF::threadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#7  0x00007f47ba04b73a in WTF::wtfThreadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#8  0x00007f47b8c2360a in start_thread () from /lib64/libpthread.so.0
#9  0x00007f47b097ca4d in clone () from /lib64/libc.so.6

Thread 10 (Thread 0x7f475bfff700 (LWP 31835)):
#0  0x00007f47b0970fdd in poll () from /lib64/libc.so.6
#1  0x00007f47b533ab34 in g_main_context_poll (priority=2147483647, n_fds=1, fds=0x7f4754001480, 
    timeout=<optimized out>, context=0x7f4754000900)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:4103
#2  g_main_context_iterate (context=0x7f4754000900, block=block@entry=1, dispatch=dispatch@entry=1, 
    self=<optimized out>)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:3803
#3  0x00007f47b533aeb2 in g_main_loop_run (loop=0x7f4754001240)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:4002
#4  0x00007f47ba04daa0 in WTF::RunLoop::run() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#5  0x00007f47ba04df5b in std::_Function_handler<void (), WTF::WorkQueue::platformInitialize(char const*, WTF::WorkQueue::Type, WTF::WorkQueue::QOS)::{lambda()#1}>::_M_invoke(std::_Any_data const&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#6  0x00007f47ba01f395 in WTF::threadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#7  0x00007f47ba04b73a in WTF::wtfThreadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#8  0x00007f47b8c2360a in start_thread () from /lib64/libpthread.so.0
#9  0x00007f47b097ca4d in clone () from /lib64/libc.so.6

Thread 9 (Thread 0x7f475b7fe700 (LWP 31836)):
#0  0x00007f47b0970fdd in poll () from /lib64/libc.so.6
#1  0x00007f47b533ab34 in g_main_context_poll (priority=2147483647, n_fds=2, fds=0x7f474c001480, 
    timeout=<optimized out>, context=0x7f474c000900)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:4103
#2  g_main_context_iterate (context=0x7f474c000900, block=block@entry=1, dispatch=dispatch@entry=1, 
    self=<optimized out>)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:3803
#3  0x00007f47b533aeb2 in g_main_loop_run (loop=0x7f474c001240)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:4002
#4  0x00007f47ba04daa0 in WTF::RunLoop::run() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#5  0x00007f47ba04df5b in std::_Function_handler<void (), WTF::WorkQueue::platformInitialize(char const*, WTF::WorkQueue::Type, WTF::WorkQueue::QOS)::{lambda()#1}>::_M_invoke(std::_Any_data const&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#6  0x00007f47ba01f395 in WTF::threadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#7  0x00007f47ba04b73a in WTF::wtfThreadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#8  0x00007f47b8c2360a in start_thread () from /lib64/libpthread.so.0
#9  0x00007f47b097ca4d in clone () from /lib64/libc.so.6

Thread 8 (Thread 0x7f475a7fc700 (LWP 31839)):
#0  0x00007f47b0970fdd in poll () from /lib64/libc.so.6
#1  0x00007f47b533ab34 in g_main_context_poll (priority=2147483647, n_fds=2, fds=0x7f4750001480, 
    timeout=<optimized out>, context=0x7f4750000900)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:4103
#2  g_main_context_iterate (context=0x7f4750000900, block=block@entry=1, dispatch=dispatch@entry=1, 
    self=<optimized out>)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:3803
#3  0x00007f47b533aeb2 in g_main_loop_run (loop=0x7f4750001240)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:4002
#4  0x00007f47ba04daa0 in WTF::RunLoop::run() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#5  0x00007f47ba04df5b in std::_Function_handler<void (), WTF::WorkQueue::platformInitialize(char const*, WTF::WorkQueue::Type, WTF::WorkQueue::QOS)::{lambda()#1}>::_M_invoke(std::_Any_data const&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#6  0x00007f47ba01f395 in WTF::threadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#7  0x00007f47ba04b73a in WTF::wtfThreadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#8  0x00007f47b8c2360a in start_thread () from /lib64/libpthread.so.0
#9  0x00007f47b097ca4d in clone () from /lib64/libc.so.6

Thread 7 (Thread 0x7f4759ffb700 (LWP 31840)):
#0  0x00007f47bb990471 in WebKit::CoordinatedBackingStoreTile::setBackBuffer(WebCore::IntRect const&, WebCore::IntRect const&, WTF::PassRefPtr<WebCore::CoordinatedSurface>, WebCore::IntPoint const&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#1  0x00007f47bb99057b in WebKit::CoordinatedBackingStore::updateTile(unsigned int, WebCore::IntRect const&, WebCore::IntRect const&, WTF::PassRefPtr<WebCore::CoordinatedSurface>, WebCore::IntPoint const&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#2  0x00007f47bb9965d4 in WebKit::CoordinatedGraphicsScene::updateTilesIfNeeded(WebCore::TextureMapperLayer*, WebCore::CoordinatedGraphicsLayerState const&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#3  0x00007f47bb997b58 in WebKit::CoordinatedGraphicsScene::setLayerState(unsigned int, WebCore::CoordinatedGraphicsLayerState const&) () from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#4  0x00007f47bb998575 in WebKit::CoordinatedGraphicsScene::commitSceneState(WebCore::CoordinatedGraphicsState const&) () from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#5  0x00007f47bb994241 in WebKit::CoordinatedGraphicsScene::syncRemoteContent() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#6  0x00007f47bb994710 in WebKit::CoordinatedGraphicsScene::paintToCurrentGLContext(WebCore::TransformationMatrix const&, float, WebCore::FloatRect const&, WebCore::Color const&, bool, WebCore::FloatPoint const&, unsigned int) () from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#7  0x00007f47bb99b6c3 in WebKit::ThreadedCompositor::renderLayerTree() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#8  0x00007f47bb99ef12 in WebKit::CompositingRunLoop::updateTimerFired() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#9  0x00007f47ba04d6fa in WTF::RunLoop::TimerBase::TimerBase(WTF::RunLoop&)::{lambda(void*)#1}::_FUN(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#10 0x00007f47b533a81a in g_main_dispatch (context=0x7f4744000900)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:3122
#11 g_main_context_dispatch (context=context@entry=0x7f4744000900)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:3737
#12 0x00007f47b533ab98 in g_main_context_iterate (context=0x7f4744000900, block=block@entry=1, 
    dispatch=dispatch@entry=1, self=<optimized out>)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:3808
#13 0x00007f47b533aeb2 in g_main_loop_run (loop=0x7f4744001240)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:4002
#14 0x00007f47ba04daa0 in WTF::RunLoop::run() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#15 0x00007f47bb99bea8 in WebKit::ThreadedCompositor::runCompositingThread() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#16 0x00007f47ba01f395 in WTF::threadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#17 0x00007f47ba04b73a in WTF::wtfThreadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#18 0x00007f47b8c2360a in start_thread () from /lib64/libpthread.so.0
#19 0x00007f47b097ca4d in clone () from /lib64/libc.so.6

Thread 6 (Thread 0x7f47597fa700 (LWP 31841)):
#0  0x00007f47b0970fdd in poll () from /lib64/libc.so.6
#1  0x00007f47b533ab34 in g_main_context_poll (priority=2147483647, n_fds=2, fds=0x7f4748001480, 
    timeout=<optimized out>, context=0x7f4748000900)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:4103
#2  g_main_context_iterate (context=0x7f4748000900, block=block@entry=1, dispatch=dispatch@entry=1, 
    self=<optimized out>)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:3803
#3  0x00007f47b533aeb2 in g_main_loop_run (loop=0x7f4748001240)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:4002
#4  0x00007f47ba04daa0 in WTF::RunLoop::run() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#5  0x00007f47ba04df5b in std::_Function_handler<void (), WTF::WorkQueue::platformInitialize(char const*, WTF::WorkQueue::Type, WTF::WorkQueue::QOS)::{lambda()#1}>::_M_invoke(std::_Any_data const&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#6  0x00007f47ba01f395 in WTF::threadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#7  0x00007f47ba04b73a in WTF::wtfThreadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#8  0x00007f47b8c2360a in start_thread () from /lib64/libpthread.so.0
#9  0x00007f47b097ca4d in clone () from /lib64/libc.so.6

Thread 5 (Thread 0x7f471d3ff700 (LWP 31853)):
#0  0x00007f47b8c28b10 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
#1  0x00007f47b1207aec in std::condition_variable::wait(std::unique_lock<std::mutex>&) ()
   from /lib64/libstdc++.so.6
#2  0x00007f47ba01c8a9 in WTF::ParkingLot::parkConditionally(void const*, std::function<bool ()>, std::function<void ()>, std::chrono::time_point<std::chrono::_V2::steady_clock, std::chrono::duration<long, std::ratio<1l, 100---Type <return> to continue, or q <return> to quit---
0000000l> > >) () from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#3  0x00007f47b9b17f34 in JSC::DFG::Worklist::runThread(JSC::DFG::ThreadData*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#4  0x00007f47ba01f395 in WTF::threadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#5  0x00007f47ba04b73a in WTF::wtfThreadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#6  0x00007f47b8c2360a in start_thread () from /lib64/libpthread.so.0
#7  0x00007f47b097ca4d in clone () from /lib64/libc.so.6

Thread 4 (Thread 0x7f4706523700 (LWP 31854)):
#0  0x00007f47b8c28b10 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
#1  0x00007f47b1207aec in std::condition_variable::wait(std::unique_lock<std::mutex>&) ()
   from /lib64/libstdc++.so.6
#2  0x00007f47ba01c8a9 in WTF::ParkingLot::parkConditionally(void const*, std::function<bool ()>, std::function<void ()>, std::chrono::time_point<std::chrono::_V2::steady_clock, std::chrono::duration<long, std::ratio<1l, 1000000000l> > >) () from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#3  0x00007f47ba01b772 in WTF::ParallelHelperPool::waitForClientWithTask(WTF::Locker<WTF::LockBase> const&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#4  0x00007f47ba01b837 in WTF::ParallelHelperPool::helperThreadBody() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#5  0x00007f47ba01f395 in WTF::threadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#6  0x00007f47ba04b73a in WTF::wtfThreadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#7  0x00007f47b8c2360a in start_thread () from /lib64/libpthread.so.0
#8  0x00007f47b097ca4d in clone () from /lib64/libc.so.6

Thread 3 (Thread 0x7f4705d22700 (LWP 31855)):
#0  0x00007f47b8c28b10 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
#1  0x00007f47b1207aec in std::condition_variable::wait(std::unique_lock<std::mutex>&) ()
   from /lib64/libstdc++.so.6
#2  0x00007f47ba01c8a9 in WTF::ParkingLot::parkConditionally(void const*, std::function<bool ()>, std::function<void ()>, std::chrono::time_point<std::chrono::_V2::steady_clock, std::chrono::duration<long, std::ratio<1l, 1000000000l> > >) () from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#3  0x00007f47ba01b772 in WTF::ParallelHelperPool::waitForClientWithTask(WTF::Locker<WTF::LockBase> const&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#4  0x00007f47ba01b837 in WTF::ParallelHelperPool::helperThreadBody() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#5  0x00007f47ba01f395 in WTF::threadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#6  0x00007f47ba04b73a in WTF::wtfThreadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#7  0x00007f47b8c2360a in start_thread () from /lib64/libpthread.so.0
#8  0x00007f47b097ca4d in clone () from /lib64/libc.so.6

Thread 2 (Thread 0x7f4705521700 (LWP 31856)):
#0  0x00007f47b8c28b10 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
#1  0x00007f47b1207aec in std::condition_variable::wait(std::unique_lock<std::mutex>&) ()
   from /lib64/libstdc++.so.6
#2  0x00007f47ba01c8a9 in WTF::ParkingLot::parkConditionally(void const*, std::function<bool ()>, std::function<void ()>, std::chrono::time_point<std::chrono::_V2::steady_clock, std::chrono::duration<long, std::ratio<1l, 1000000000l> > >) () from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#3  0x00007f47ba01b772 in WTF::ParallelHelperPool::waitForClientWithTask(WTF::Locker<WTF::LockBase> const&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#4  0x00007f47ba01b837 in WTF::ParallelHelperPool::helperThreadBody() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#5  0x00007f47ba01f395 in WTF::threadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#6  0x00007f47ba04b73a in WTF::wtfThreadEntryPoint(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#7  0x00007f47b8c2360a in start_thread () from /lib64/libpthread.so.0
#8  0x00007f47b097ca4d in clone () from /lib64/libc.so.6

Thread 1 (Thread 0x7f47bdb18a80 (LWP 31825)):
#0  0x00007f47bbe2a2b1 in WebCore::Node::eventTargetData() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#1  0x00007f47bc25bdfb in WebCore::EventHandler::updateMouseEventTargetNode(WebCore::Node*, WebCore::PlatformMouseEvent const&, bool) () from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#2  0x00007f47bc25c616 in WebCore::EventHandler::dispatchMouseEvent(WTF::AtomicString const&, WebCore::Node*, bo---Type <return> to continue, or q <return> to quit---
ol, int, WebCore::PlatformMouseEvent const&, bool) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#3  0x00007f47bc25d37f in WebCore::EventHandler::handleMouseMoveEvent(WebCore::PlatformMouseEvent const&, WebCore::HitTestResult*, bool) [clone .part.226] [clone .constprop.236] ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#4  0x00007f47bc25f9ca in WebCore::EventHandler::mouseMoved(WebCore::PlatformMouseEvent const&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#5  0x00007f47bb8be3e5 in WebKit::WebPage::mouseEvent(WebKit::WebMouseEvent const&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#6  0x00007f47bb9e21d3 in WebKit::WebPage::didReceiveWebPageMessage(IPC::Connection&, IPC::MessageDecoder&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#7  0x00007f47bb6fa749 in IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::MessageDecoder&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#8  0x00007f47bb814026 in WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#9  0x00007f47bb6f6c56 in IPC::Connection::dispatchMessage(std::unique_ptr<IPC::MessageDecoder, std::default_delete<IPC::MessageDecoder> >) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#10 0x00007f47bb6f7623 in IPC::Connection::dispatchOneMessage() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#11 0x00007f47ba01e2cf in WTF::RunLoop::performWork() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#12 0x00007f47ba04d279 in WTF::RunLoop::RunLoop()::{lambda(void*)#1}::_FUN(void*) ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#13 0x00007f47b533a81a in g_main_dispatch (context=0x829aa0)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:3122
#14 g_main_context_dispatch (context=context@entry=0x829aa0)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:3737
#15 0x00007f47b533ab98 in g_main_context_iterate (context=0x829aa0, block=block@entry=1, 
    dispatch=dispatch@entry=1, self=<optimized out>)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:3808
#16 0x00007f47b533aeb2 in g_main_loop_run (loop=0xe872c0)
    at /home/changseok/Projects/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.44.1/glib/gmain.c:4002
#17 0x00007f47ba04daa0 in WTF::RunLoop::run() ()
   from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#18 0x00007f47bb98f842 in int WebKit::ChildProcessMain<WebKit::WebProcess, WebKit::WebProcessMain>(int, char**)
    () from /home/changseok/Projects/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#19 0x00007f47b089a580 in __libc_start_main () from /lib64/libc.so.6
#20 0x0000000000400b49 in _start ()

[Carlos Garcia Campos]

Created attachment 281169 [details]
Updated patch

Patch updated on top of patches attached to bugs #154070 and #158615. It also depends on bug #158689 to ensure the detached inspector is correctly shown. I still think we shouldn't need to destroy and create the compositing thread if possible. I have been doing a lot of tests while writing these patches and I haven't been able to make it crash. And as with the previous patch this also fixes bug #154071.

[Zan Dobersek]

Comment on attachment 281169 [details]
Updated patch

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

> Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:66
> +        m_scene->setActive(handle);

!!handle converts better (i.e. cleaner) to a boolean value.

> Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:69
> +        ensureGLContext();
> +        updateViewport();

It's not clear from the change log -- are these calls required in case of handle being null?

> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp:202
> +    scheduleLayerFlush();

Why cancel and then re-schedule the layer flush? Just re-scheduling should be enough.

[Carlos Garcia Campos]

(In reply to comment #14)
> Comment on attachment 281169 [details]
> Updated patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=281169&action=review
> 
> > Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:66
> > +        m_scene->setActive(handle);
> 
> !!handle converts better (i.e. cleaner) to a boolean value.

Ok.

> > Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:69
> > +        ensureGLContext();
> > +        updateViewport();
> 
> It's not clear from the change log -- are these calls required in case of
> handle being null?

Ok, I'll update the ChangeLog. When the handler is null, ensureGLContext actually destroys the current cotext, to ensure any pending or further operation on the compositing run loop returns early in !glContext(). The updateViewport is to ensure we re-schedule a display on next frame for the new handle, since we have cancelled any previous one. It's probably not needed in the case of null handle.

> > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp:202
> > +    scheduleLayerFlush();
> 
> Why cancel and then re-schedule the layer flush? Just re-scheduling should
> be enough.

Since setting the new handler on the compositor is sync now, I want to make sure we cancel any scheduled layer flush before actually changing the handler, and then scheduling a new one once we have the new handler. That's exactly what we do in LayerTreeHostGtk too.

[Zan Dobersek]

(In reply to comment #15)
> > > Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:69
> > > +        ensureGLContext();
> > > +        updateViewport();
> > 
> > It's not clear from the change log -- are these calls required in case of
> > handle being null?
> 
> Ok, I'll update the ChangeLog. When the handler is null, ensureGLContext
> actually destroys the current cotext, to ensure any pending or further
> operation on the compositing run loop returns early in !glContext(). The
> updateViewport is to ensure we re-schedule a display on next frame for the
> new handle, since we have cancelled any previous one. It's probably not
> needed in the case of null handle.
> 

In case the handle is null you don't need to call the modified ensureGLContext(), it should be enough to just destroy the current GLContext on the spot. And the update should only be issued in case of a non-null handle -- if it's null, renderLayerTree() will bail out early anyway because of an inactive scene.

> > > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp:202
> > > +    scheduleLayerFlush();
> > 
> > Why cancel and then re-schedule the layer flush? Just re-scheduling should
> > be enough.
> 
> Since setting the new handler on the compositor is sync now, I want to make
> sure we cancel any scheduled layer flush before actually changing the
> handler, and then scheduling a new one once we have the new handler. That's
> exactly what we do in LayerTreeHostGtk too.

This method is called on the main thread, just like the layer flushes. You shouldn't have to worry about spurious layer flushes occurring while executing this method.

[Carlos Garcia Campos]

(In reply to comment #16)
> (In reply to comment #15)
> > > > Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:69
> > > > +        ensureGLContext();
> > > > +        updateViewport();
> > > 
> > > It's not clear from the change log -- are these calls required in case of
> > > handle being null?
> > 
> > Ok, I'll update the ChangeLog. When the handler is null, ensureGLContext
> > actually destroys the current cotext, to ensure any pending or further
> > operation on the compositing run loop returns early in !glContext(). The
> > updateViewport is to ensure we re-schedule a display on next frame for the
> > new handle, since we have cancelled any previous one. It's probably not
> > needed in the case of null handle.
> > 
> 
> In case the handle is null you don't need to call the modified
> ensureGLContext(), it should be enough to just destroy the current GLContext
> on the spot. And the update should only be issued in case of a non-null
> handle -- if it's null, renderLayerTree() will bail out early anyway because
> of an inactive scene.

The update is not actually needed in any case, I've just removed it. When the handle is null, ensureGLContext() simply destroys the current context. I've renamed it to tryEnsureGLContext() to make it clear that it can fail, and I've added a comment there explaining what happens. Also added an assert to ensure that we always destroy the current native handle before setting a new one.

> > > > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp:202
> > > > +    scheduleLayerFlush();
> > > 
> > > Why cancel and then re-schedule the layer flush? Just re-scheduling should
> > > be enough.
> > 
> > Since setting the new handler on the compositor is sync now, I want to make
> > sure we cancel any scheduled layer flush before actually changing the
> > handler, and then scheduling a new one once we have the new handler. That's
> > exactly what we do in LayerTreeHostGtk too.
> 
> This method is called on the main thread, just like the layer flushes. You
> shouldn't have to worry about spurious layer flushes occurring while
> executing this method.

hmm, you are right, this is all in the main thread, so it's not possible that a layer flush happens until the new handle is updated, so if there's on scheduled, we don't need to do anything, it will happen after setting the new handler.

[Carlos Garcia Campos]

Created attachment 281244 [details]
Updated patch

[Zan Dobersek]

Comment on attachment 281244 [details]
Updated patch

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

> Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:69
> +        ASSERT((handle && !m_nativeSurfaceHandle) || (m_nativeSurfaceHandle && !handle));

That's a good assert. Could be simplified into an exclusive-or.

> Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:76
> +        // In case m_nativeSurfaceHandle is 0 now, tryEnsureGLContext will destroy the current context and will
> +        // fail to ensure a new one, so that any previous operation pending and new ones will be discarded.
> +        // In case m_nativeSurfaceHandle is a valid handle, tryEnsureGLContext will create a new context for the
> +        // new handle.
> +        tryEnsureGLContext();

You're not 'ensuring' anything if the handle is null, and you are creating the context prematurely (and blocking the main thread) if the handle is not null.

I propose just nulling out m_context here if the handle is null, avoiding any changes to ThreadedCompositor::ensureGLContext().

[Carlos Garcia Campos]

(In reply to comment #19)
> Comment on attachment 281244 [details]
> Updated patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=281244&action=review
> 
> > Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:69
> > +        ASSERT((handle && !m_nativeSurfaceHandle) || (m_nativeSurfaceHandle && !handle));
> 
> That's a good assert. Could be simplified into an exclusive-or.

OK.

> > Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:76
> > +        // In case m_nativeSurfaceHandle is 0 now, tryEnsureGLContext will destroy the current context and will
> > +        // fail to ensure a new one, so that any previous operation pending and new ones will be discarded.
> > +        // In case m_nativeSurfaceHandle is a valid handle, tryEnsureGLContext will create a new context for the
> > +        // new handle.
> > +        tryEnsureGLContext();
> 
> You're not 'ensuring' anything if the handle is null, and you are creating
> the context prematurely (and blocking the main thread) if the handle is not
> null.
> 
> I propose just nulling out m_context here if the handle is null, avoiding
> any changes to ThreadedCompositor::ensureGLContext().

hmm, yes, much simpler, thanks! I'll keep the rename though, since ensureGLContext still doesn't ensure that a context is created and can fail.

[Carlos Garcia Campos]

Created attachment 281250 [details]
Updated patch

[Zan Dobersek]

Comment on attachment 281250 [details]
Updated patch

r=me

[Carlos Garcia Campos]

Committed r202040: <http://trac.webkit.org/changeset/202040>