- Revision
- 192510
- Author
- carlo...@webkit.org
- Date
- 2015-11-17 00:37:21 -0800 (Tue, 17 Nov 2015)
Log Message
[GTK] Web Process crashes on reparenting a WebView with AC mode on
https://bugs.webkit.org/show_bug.cgi?id=151139
Reviewed by Mario Sanchez Prada.
When the web view is reparented, the widget is first unrealized,
and then realized again when added to the new parent. In the
second realize, the old redirected XComposite window is destroyed
and a new one is created, but the web process is still using the
old redirected window ID. As soon as the redirected window is
destroyed and the web process tries to use the window ID, it
crashes due to a BadDrawable X error. We have to notify the web
process as soon as the web view is unrealized to stop using the
current window ID and exit accelerated compositing mode until a
new window ID is given. This notification needs to be synchronous,
because the window can be destroyed in the UI process before the
message is received in the web process.
* UIProcess/API/gtk/WebKitWebViewBase.cpp:
(webkitWebViewBaseRealize): Add an assert to ensure we never have
a redirected window when the view is realized. Also check drawing
area is not nullptr, since it can be destroyed at any time if the
web process crashes.
(webkitWebViewBaseUnrealize): Call
DrawingAreaProxyImpl::destroyNativeSurfaceHandleForCompositing()
and destroy the redirected XComposite window.
* UIProcess/DrawingAreaProxyImpl.cpp:
(WebKit::DrawingAreaProxyImpl::destroyNativeSurfaceHandleForCompositing):
Send DestroyNativeSurfaceHandleForCompositing synchronous messsage
to the web process.
* UIProcess/DrawingAreaProxyImpl.h:
* WebProcess/WebPage/DrawingArea.h:
* WebProcess/WebPage/DrawingArea.messages.in: Add
DestroyNativeSurfaceHandleForCompositing message.
* WebProcess/WebPage/DrawingAreaImpl.cpp:
(WebKit::DrawingAreaImpl::destroyNativeSurfaceHandleForCompositing):
Set the native surface handler for compositing to 0 to reset it.
* WebProcess/WebPage/DrawingAreaImpl.h:
* WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:
(WebKit::LayerTreeHostGtk::makeContextCurrent): Return false
early always when layer tree context ID is 0, even if we already
have a context.
(WebKit::LayerTreeHostGtk::setNativeSurfaceHandleForCompositing):
Cancel any pending layer flush when setting a new handler.
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (192509 => 192510)
--- trunk/Source/WebKit2/ChangeLog 2015-11-17 08:11:37 UTC (rev 192509)
+++ trunk/Source/WebKit2/ChangeLog 2015-11-17 08:37:21 UTC (rev 192510)
@@ -1,3 +1,50 @@
+2015-11-17 Carlos Garcia Campos <cgar...@igalia.com>
+
+ [GTK] Web Process crashes on reparenting a WebView with AC mode on
+ https://bugs.webkit.org/show_bug.cgi?id=151139
+
+ Reviewed by Mario Sanchez Prada.
+
+ When the web view is reparented, the widget is first unrealized,
+ and then realized again when added to the new parent. In the
+ second realize, the old redirected XComposite window is destroyed
+ and a new one is created, but the web process is still using the
+ old redirected window ID. As soon as the redirected window is
+ destroyed and the web process tries to use the window ID, it
+ crashes due to a BadDrawable X error. We have to notify the web
+ process as soon as the web view is unrealized to stop using the
+ current window ID and exit accelerated compositing mode until a
+ new window ID is given. This notification needs to be synchronous,
+ because the window can be destroyed in the UI process before the
+ message is received in the web process.
+
+ * UIProcess/API/gtk/WebKitWebViewBase.cpp:
+ (webkitWebViewBaseRealize): Add an assert to ensure we never have
+ a redirected window when the view is realized. Also check drawing
+ area is not nullptr, since it can be destroyed at any time if the
+ web process crashes.
+ (webkitWebViewBaseUnrealize): Call
+ DrawingAreaProxyImpl::destroyNativeSurfaceHandleForCompositing()
+ and destroy the redirected XComposite window.
+ * UIProcess/DrawingAreaProxyImpl.cpp:
+ (WebKit::DrawingAreaProxyImpl::destroyNativeSurfaceHandleForCompositing):
+ Send DestroyNativeSurfaceHandleForCompositing synchronous messsage
+ to the web process.
+ * UIProcess/DrawingAreaProxyImpl.h:
+ * WebProcess/WebPage/DrawingArea.h:
+ * WebProcess/WebPage/DrawingArea.messages.in: Add
+ DestroyNativeSurfaceHandleForCompositing message.
+ * WebProcess/WebPage/DrawingAreaImpl.cpp:
+ (WebKit::DrawingAreaImpl::destroyNativeSurfaceHandleForCompositing):
+ Set the native surface handler for compositing to 0 to reset it.
+ * WebProcess/WebPage/DrawingAreaImpl.h:
+ * WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:
+ (WebKit::LayerTreeHostGtk::makeContextCurrent): Return false
+ early always when layer tree context ID is 0, even if we already
+ have a context.
+ (WebKit::LayerTreeHostGtk::setNativeSurfaceHandleForCompositing):
+ Cancel any pending layer flush when setting a new handler.
+
2015-11-16 Alex Christensen <achristen...@webkit.org>
Fix CMake build and make PluginProcess executable
Modified: trunk/Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp (192509 => 192510)
--- trunk/Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp 2015-11-17 08:11:37 UTC (rev 192509)
+++ trunk/Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp 2015-11-17 08:37:21 UTC (rev 192510)
@@ -280,6 +280,7 @@
#if USE(REDIRECTED_XCOMPOSITE_WINDOW)
if (PlatformDisplay::sharedDisplay().type() == PlatformDisplay::Type::X11) {
+ ASSERT(!priv->redirectedWindow);
priv->redirectedWindow = RedirectedXCompositeWindow::create(
gtk_widget_get_parent_window(widget),
[webView] {
@@ -288,8 +289,8 @@
gtk_widget_queue_draw(GTK_WIDGET(webView));
});
if (priv->redirectedWindow) {
- DrawingAreaProxyImpl* drawingArea = static_cast<DrawingAreaProxyImpl*>(priv->pageProxy->drawingArea());
- drawingArea->setNativeSurfaceHandleForCompositing(priv->redirectedWindow->windowID());
+ if (DrawingAreaProxyImpl* drawingArea = static_cast<DrawingAreaProxyImpl*>(priv->pageProxy->drawingArea()))
+ drawingArea->setNativeSurfaceHandleForCompositing(priv->redirectedWindow->windowID());
}
}
#endif
@@ -329,8 +330,8 @@
gdk_window_set_user_data(window, widget);
#if USE(TEXTURE_MAPPER) && PLATFORM(X11) && !USE(REDIRECTED_XCOMPOSITE_WINDOW)
- DrawingAreaProxyImpl* drawingArea = static_cast<DrawingAreaProxyImpl*>(priv->pageProxy->drawingArea());
- drawingArea->setNativeSurfaceHandleForCompositing(GDK_WINDOW_XID(window));
+ if (DrawingAreaProxyImpl* drawingArea = static_cast<DrawingAreaProxyImpl*>(priv->pageProxy->drawingArea()))
+ drawingArea->setNativeSurfaceHandleForCompositing(GDK_WINDOW_XID(window));
#endif
gtk_style_context_set_background(gtk_widget_get_style_context(widget), window);
@@ -345,6 +346,13 @@
static void webkitWebViewBaseUnrealize(GtkWidget* widget)
{
WebKitWebViewBase* webView = WEBKIT_WEB_VIEW_BASE(widget);
+#if USE(TEXTURE_MAPPER) && PLATFORM(X11)
+ if (DrawingAreaProxyImpl* drawingArea = static_cast<DrawingAreaProxyImpl*>(webView->priv->pageProxy->drawingArea()))
+ drawingArea->destroyNativeSurfaceHandleForCompositing();
+#endif
+#if USE(REDIRECTED_XCOMPOSITE_WINDOW)
+ webView->priv->redirectedWindow = nullptr;
+#endif
gtk_im_context_set_client_window(webView->priv->inputMethodFilter.context(), nullptr);
GTK_WIDGET_CLASS(webkit_web_view_base_parent_class)->unrealize(widget);
Modified: trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp (192509 => 192510)
--- trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp 2015-11-17 08:11:37 UTC (rev 192509)
+++ trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp 2015-11-17 08:37:21 UTC (rev 192510)
@@ -314,6 +314,12 @@
{
m_webPageProxy.process().send(Messages::DrawingArea::SetNativeSurfaceHandleForCompositing(handle), m_webPageProxy.pageID());
}
+
+void DrawingAreaProxyImpl::destroyNativeSurfaceHandleForCompositing()
+{
+ bool handled;
+ m_webPageProxy.process().sendSync(Messages::DrawingArea::DestroyNativeSurfaceHandleForCompositing(), Messages::DrawingArea::DestroyNativeSurfaceHandleForCompositing::Reply(handled), m_webPageProxy.pageID());
+}
#endif
void DrawingAreaProxyImpl::exitAcceleratedCompositingMode()
Modified: trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.h (192509 => 192510)
--- trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.h 2015-11-17 08:11:37 UTC (rev 192509)
+++ trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.h 2015-11-17 08:37:21 UTC (rev 192510)
@@ -50,6 +50,7 @@
#if USE(TEXTURE_MAPPER) && PLATFORM(GTK)
void setNativeSurfaceHandleForCompositing(uint64_t);
+ void destroyNativeSurfaceHandleForCompositing();
#endif
private:
Modified: trunk/Source/WebKit2/WebProcess/WebPage/DrawingArea.h (192509 => 192510)
--- trunk/Source/WebKit2/WebProcess/WebPage/DrawingArea.h 2015-11-17 08:11:37 UTC (rev 192509)
+++ trunk/Source/WebKit2/WebProcess/WebPage/DrawingArea.h 2015-11-17 08:37:21 UTC (rev 192510)
@@ -150,6 +150,7 @@
private:
// IPC::MessageReceiver.
virtual void didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&) override;
+ virtual void didReceiveSyncMessage(IPC::Connection&, IPC::MessageDecoder&, std::unique_ptr<IPC::MessageEncoder>&) override;
// Message handlers.
// FIXME: These should be pure virtual.
@@ -170,6 +171,7 @@
#if USE(TEXTURE_MAPPER) && PLATFORM(GTK)
virtual void setNativeSurfaceHandleForCompositing(uint64_t) = 0;
+ virtual void destroyNativeSurfaceHandleForCompositing(bool&) = 0;
#endif
};
Modified: trunk/Source/WebKit2/WebProcess/WebPage/DrawingArea.messages.in (192509 => 192510)
--- trunk/Source/WebKit2/WebProcess/WebPage/DrawingArea.messages.in 2015-11-17 08:11:37 UTC (rev 192509)
+++ trunk/Source/WebKit2/WebProcess/WebPage/DrawingArea.messages.in 2015-11-17 08:37:21 UTC (rev 192510)
@@ -42,5 +42,6 @@
#if USE(TEXTURE_MAPPER) && PLATFORM(GTK)
SetNativeSurfaceHandleForCompositing(uint64_t handle)
+ DestroyNativeSurfaceHandleForCompositing() -> (bool handled)
#endif
}
Modified: trunk/Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp (192509 => 192510)
--- trunk/Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp 2015-11-17 08:11:37 UTC (rev 192509)
+++ trunk/Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp 2015-11-17 08:37:21 UTC (rev 192510)
@@ -699,6 +699,12 @@
m_layerTreeHost->setNativeSurfaceHandleForCompositing(handle);
}
}
+
+void DrawingAreaImpl::destroyNativeSurfaceHandleForCompositing(bool& handled)
+{
+ handled = true;
+ setNativeSurfaceHandleForCompositing(0);
+}
#endif
} // namespace WebKit
Modified: trunk/Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.h (192509 => 192510)
--- trunk/Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.h 2015-11-17 08:11:37 UTC (rev 192509)
+++ trunk/Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.h 2015-11-17 08:37:21 UTC (rev 192510)
@@ -72,6 +72,7 @@
#if USE(TEXTURE_MAPPER) && PLATFORM(GTK)
virtual void setNativeSurfaceHandleForCompositing(uint64_t) override;
+ virtual void destroyNativeSurfaceHandleForCompositing(bool&) override;
#endif
#if USE(COORDINATED_GRAPHICS_MULTIPROCESS)
Modified: trunk/Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp (192509 => 192510)
--- trunk/Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp 2015-11-17 08:11:37 UTC (rev 192509)
+++ trunk/Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp 2015-11-17 08:37:21 UTC (rev 192510)
@@ -145,10 +145,12 @@
bool LayerTreeHostGtk::makeContextCurrent()
{
+ if (!m_layerTreeContext.contextID) {
+ m_context = nullptr;
+ return false;
+ }
+
if (!m_context) {
- if (!m_layerTreeContext.contextID)
- return false;
-
m_context = GLContext::createContextForWindow(reinterpret_cast<GLNativeWindowType>(m_layerTreeContext.contextID), GLContext::sharingContext());
if (!m_context)
return false;
@@ -416,6 +418,7 @@
void LayerTreeHostGtk::setNativeSurfaceHandleForCompositing(uint64_t handle)
{
+ cancelPendingLayerFlush();
m_layerTreeContext.contextID = handle;
// The creation of the TextureMapper needs an active OpenGL context.
