Bug 142079 – REGRESSION(r177075): WebProcess crashes when entering accelerating compositing mode before the WebView is realized

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Carlos Garcia Campos]

Yet another regression of r177075, where the creation of the redirected composited window was moved to realize method.

Program received signal SIGSEGV, Segmentation fault.
0x00007fc4570b8790 in WebKit::LayerTreeHostGtk::compositeLayersToContext(WebKit::LayerTreeHostGtk::CompositePurpose) ()
   from WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
(gdb) bt
#0  0x00007fc4570b8790 in WebKit::LayerTreeHostGtk::compositeLayersToContext(WebKit::LayerTreeHostGtk::CompositePurpose) ()
   from WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#1  0x00007fc4570b88bb in WebKit::LayerTreeHostGtk::sizeDidChange(WebCore::IntSize const&) () from WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#2  0x00007fc4570b253b in WebKit::DrawingAreaImpl::updateBackingStoreState(unsigned long, bool, float, WebCore::IntSize const&, WebCore::IntSize const&) ()
   from WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#3  0x00007fc4570de606 in void IPC::handleMessage<Messages::DrawingArea::UpdateBackingStoreState, WebKit::DrawingArea, void (WebKit::DrawingArea::*)(unsigned long, bool, float, WebCore::IntSize const&, WebCore::IntSize const&)>(IPC::MessageDecoder&, WebKit::DrawingArea*, void (WebKit::DrawingArea::*)(unsigned long, bool, float, WebCore::IntSize const&, WebCore::IntSize const&)) () from WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#4  0x00007fc4570de4df in WebKit::DrawingArea::didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&) ()
   from WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#5  0x00007fc456eb5280 in IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::MessageDecoder&) ()
   from WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#6  0x00007fc456fa60f9 in WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&) ()
   from WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#7  0x00007fc456eb194b in IPC::Connection::dispatchMessage(std::unique_ptr<IPC::MessageDecoder, std::default_delete<IPC::MessageDecoder> >) ()
   from WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#8  0x00007fc456eb2364 in IPC::Connection::dispatchOneMessage() () from WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#9  0x00007fc458442734 in WTF::RunLoop::performWork() () from WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#10 0x00007fc4558a0525 in WTF::GMainLoopSource::voidCallback() () from WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#11 0x00007fc45589e5ba in WTF::GMainLoopSource::voidSourceCallback(WTF::GMainLoopSource*) () from WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#12 0x00007fc4527f4aed in g_main_dispatch (context=0x24f7e50) at gmain.c:3122
#13 g_main_context_dispatch (context=context@entry=0x24f7e50) at gmain.c:3737
#14 0x00007fc4527f4e88 in g_main_context_iterate (context=0x24f7e50, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3808
#15 0x00007fc4527f51a2 in g_main_loop_run (loop=0x2b35c90) at gmain.c:4002
#16 0x00007fc4570b73b2 in WebProcessMainUnix () from WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#17 0x00007fc44cebdb45 in __libc_start_main (main=0x400aa0 <main>, argc=2, argv=0x7fff5d409608, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, 
    stack_end=0x7fff5d4095f8) at libc-start.c:287
#18 0x0000000000400af5 in _start ()

[Carlos Garcia Campos]

This can be reproduced with epiphany.

1- Disable the delayed load option of ephy
2- search for poster circle in google
3- Middle click on tyhe result to open it in a new tab (let the tab load)
4- Move to the new tab
CRASH!

The problem is that the texture mapper and native window handler are initialized when the LayerTreeHost is initialized, assuming the UI process has already sent the native window handler to the web process, but that doesn't always happen since we moved the redirected window creation to realize.

[Carlos Garcia Campos]

Created attachment 247504 [details]
Patch

[Zan Dobersek]

Comment on attachment 247504 [details]
Patch

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

> Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:86
> +        m_context = GLContext::createContextForWindow(m_layerTreeContext.contextID, GLContext::sharingContext());

GLContext::createContextForWindow() can still return null. But you don't want to call that every time you call LayerTreeHostGtk::makeContextCurrent().

> Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:90
> +    m_context->makeContextCurrent();
> +    return true;

Here you should return the return value of GLContext::makeContextCurrent().

[Carlos Garcia Campos]

Comment on attachment 247504 [details]
Patch

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

Thanks for the review, I'll submit a new patch.

>> Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:86
>> +        m_context = GLContext::createContextForWindow(m_layerTreeContext.contextID, GLContext::sharingContext());
> 
> GLContext::createContextForWindow() can still return null. But you don't want to call that every time you call LayerTreeHostGtk::makeContextCurrent().

I'm not sure I get what you mean, I need to check if m_context is nullptr or not, that's right. But why not calling this everytime LayerTreeHostGtk::makeContextCurrent() is called? It's only called when the context is nullptr. I noticed that glContext() was always used to make the context current, that's why I tried to simplify the caller with this method, that it should do the same than previous code (it doesn't because I made some mistakes, though)

>> Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:90
>> +    return true;
> 
> Here you should return the return value of GLContext::makeContextCurrent().

Right.

> Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:-123
> -    context->makeContextCurrent();

So, here for example we are not checking the return value of makeContextCurrent(), I guess we should, there's a comment saying that the creation fo the texture mapper needs an active context, and we are creating the texture mapper after this.

[Carlos Garcia Campos]

Created attachment 247655 [details]
Updated patch

[Zan Dobersek]

Comment on attachment 247655 [details]
Updated patch

Looks OK.

[Carlos Garcia Campos]

Committed r180924: <http://trac.webkit.org/changeset/180924>