Diff
Modified: trunk/Source/WebCore/ChangeLog (89634 => 89635)
--- trunk/Source/WebCore/ChangeLog 2011-06-23 23:47:39 UTC (rev 89634)
+++ trunk/Source/WebCore/ChangeLog 2011-06-24 00:11:51 UTC (rev 89635)
@@ -1,3 +1,30 @@
+2011-06-23 John Bates <jba...@google.com>
+
+ Reviewed by James Robinson.
+
+ Fix latch deadlock when GPU process crashes or context is lost.
+ https://bugs.webkit.org/show_bug.cgi?id=63189
+ The main bug fix is to only set/wait latches if the child context has no errors.
+ Additionally, the LayerChromium classes needed to be modified to not continue drawing when
+ their corresponding contexts have errors. Otherwise, they would draw with invalid texture ids.
+
+ Test: open particles WebGL demo in chrome, kill GPU process from Task Manager; observe no deadlock.
+
+ * platform/graphics/chromium/LayerRendererChromium.cpp:
+ (WebCore::LayerRendererChromium::LayerRendererChromium):
+ (WebCore::LayerRendererChromium::updateAndDrawLayers):
+ (WebCore::LayerRendererChromium::updateLayers):
+ (WebCore::LayerRendererChromium::isCompositorContextLost):
+ * platform/graphics/chromium/LayerRendererChromium.h:
+ * platform/graphics/chromium/WebGLLayerChromium.cpp:
+ (WebCore::WebGLLayerChromium::drawsContent):
+ (WebCore::WebGLLayerChromium::updateCompositorResources):
+ (WebCore::WebGLLayerChromium::setContext):
+ * platform/graphics/chromium/WebGLLayerChromium.h:
+ * platform/graphics/chromium/Canvas2DLayerChromium.cpp:
+ (WebCore::Canvas2DLayerChromium::drawsContent):
+ * platform/graphics/chromium/Canvas2DLayerChromium.h:
+
2011-06-23 Alok Priyadarshi <al...@chromium.org>
Reviewed by James Robinson.
Modified: trunk/Source/WebCore/platform/graphics/Extensions3D.h (89634 => 89635)
--- trunk/Source/WebCore/platform/graphics/Extensions3D.h 2011-06-23 23:47:39 UTC (rev 89634)
+++ trunk/Source/WebCore/platform/graphics/Extensions3D.h 2011-06-24 00:11:51 UTC (rev 89635)
@@ -106,6 +106,12 @@
};
// GL_ARB_robustness
+ // Note: This method's behavior differs from the GL_ARB_robustness
+ // specification in the following way:
+ // The implementation must not reset the error state during this call.
+ // If getGraphicsResetStatusARB returns an error, it should continue
+ // returning the same error. Restoring the GraphicsContext3D is handled
+ // externally.
virtual int getGraphicsResetStatusARB() = 0;
// GL_ANGLE_framebuffer_blit
Modified: trunk/Source/WebCore/platform/graphics/chromium/Canvas2DLayerChromium.cpp (89634 => 89635)
--- trunk/Source/WebCore/platform/graphics/chromium/Canvas2DLayerChromium.cpp 2011-06-23 23:47:39 UTC (rev 89634)
+++ trunk/Source/WebCore/platform/graphics/chromium/Canvas2DLayerChromium.cpp 2011-06-24 00:11:51 UTC (rev 89635)
@@ -35,6 +35,7 @@
#include "Canvas2DLayerChromium.h"
#include "DrawingBuffer.h"
+#include "Extensions3DChromium.h"
#include "GraphicsContext3D.h"
#include "LayerRendererChromium.h"
@@ -59,9 +60,17 @@
layerRenderer()->removeChildContext(m_drawingBuffer->graphicsContext3D().get());
}
+bool Canvas2DLayerChromium::drawsContent() const
+{
+ GraphicsContext3D* context;
+ return (m_drawingBuffer
+ && (context = m_drawingBuffer->graphicsContext3D().get())
+ && (context->getExtensions()->getGraphicsResetStatusARB() == GraphicsContext3D::NO_ERROR));
+}
+
void Canvas2DLayerChromium::updateCompositorResources()
{
- if (!m_contentsDirty || !m_drawingBuffer)
+ if (!m_contentsDirty || !drawsContent())
return;
if (m_textureChanged) { // We have to generate a new backing texture.
GraphicsContext3D* context = layerRendererContext();
Modified: trunk/Source/WebCore/platform/graphics/chromium/Canvas2DLayerChromium.h (89634 => 89635)
--- trunk/Source/WebCore/platform/graphics/chromium/Canvas2DLayerChromium.h 2011-06-23 23:47:39 UTC (rev 89634)
+++ trunk/Source/WebCore/platform/graphics/chromium/Canvas2DLayerChromium.h 2011-06-24 00:11:51 UTC (rev 89635)
@@ -45,7 +45,7 @@
public:
static PassRefPtr<Canvas2DLayerChromium> create(DrawingBuffer*, GraphicsLayerChromium* owner);
virtual ~Canvas2DLayerChromium();
- virtual bool drawsContent() const { return true; }
+ virtual bool drawsContent() const;
virtual void updateCompositorResources();
void setTextureChanged();
Modified: trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp (89634 => 89635)
--- trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp 2011-06-23 23:47:39 UTC (rev 89634)
+++ trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp 2011-06-24 00:11:51 UTC (rev 89635)
@@ -120,7 +120,6 @@
, m_offscreenFramebufferId(0)
, m_compositeOffscreen(false)
, m_context(context)
- , m_childContextsWereCopied(false)
, m_contextSupportsLatch(false)
, m_contextSupportsTextureFormatBGRA(false)
, m_contextSupportsReadFormatBGRA(false)
@@ -255,22 +254,19 @@
// thread can execute WebGL calls on the child context at any time,
// potentially clobbering the parent texture that is being renderered
// by the compositor thread.
- if (m_childContextsWereCopied) {
- Extensions3DChromium* parentExt = static_cast<Extensions3DChromium*>(m_context->getExtensions());
- // For each child context:
- // glWaitLatch(Offscreen->Compositor);
- ChildContextMap::iterator i = m_childContexts.begin();
- for (; i != m_childContexts.end(); ++i) {
- Extensions3DChromium* childExt = static_cast<Extensions3DChromium*>(i->first->getExtensions());
- GC3Duint latchId;
- childExt->getChildToParentLatchCHROMIUM(&latchId);
- parentExt->waitLatchCHROMIUM(latchId);
+ Extensions3DChromium* parentExt = static_cast<Extensions3DChromium*>(m_context->getExtensions());
+ // For each child context:
+ // glWaitLatch(Offscreen->Compositor);
+ ChildContextMap::iterator i = m_childContexts.begin();
+ for (; i != m_childContexts.end(); ++i) {
+ Extensions3DChromium* childExt = static_cast<Extensions3DChromium*>(i->first->getExtensions());
+ if (childExt->getGraphicsResetStatusARB() == GraphicsContext3D::NO_ERROR) {
+ GC3Duint childToParentLatchId;
+ childExt->getChildToParentLatchCHROMIUM(&childToParentLatchId);
+ childExt->setLatchCHROMIUM(childToParentLatchId);
+ parentExt->waitLatchCHROMIUM(childToParentLatchId);
}
}
- // Reset to false to indicate that we have consumed the dirty child
- // contexts' parent textures. (This is only useful when the compositor
- // is multithreaded.)
- m_childContextsWereCopied = false;
}
drawLayers(renderSurfaceLayerList);
@@ -285,9 +281,12 @@
ChildContextMap::iterator i = m_childContexts.begin();
for (; i != m_childContexts.end(); ++i) {
Extensions3DChromium* childExt = static_cast<Extensions3DChromium*>(i->first->getExtensions());
- GC3Duint latchId;
- childExt->getParentToChildLatchCHROMIUM(&latchId);
- parentExt->setLatchCHROMIUM(latchId);
+ if (childExt->getGraphicsResetStatusARB() == GraphicsContext3D::NO_ERROR) {
+ GC3Duint parentToChildLatchId;
+ childExt->getParentToChildLatchCHROMIUM(&parentToChildLatchId);
+ parentExt->setLatchCHROMIUM(parentToChildLatchId);
+ childExt->waitLatchCHROMIUM(parentToChildLatchId);
+ }
}
}
@@ -340,53 +339,12 @@
s_inPaintLayerContents = false;
#endif
- // FIXME: Before updateCompositorResources, when the compositor runs in
- // its own thread, and when the copyTexImage2D bug is fixed, insert
- // a glWaitLatch(Compositor->Offscreen) on all child contexts here instead
- // of after updateCompositorResources.
- // Also uncomment the glSetLatch(Compositor->Offscreen) code in addChildContext.
-// if (hardwareCompositing() && m_contextSupportsLatch) {
-// // For each child context:
-// // glWaitLatch(Compositor->Offscreen);
-// ChildContextMap::iterator i = m_childContexts.begin();
-// for (; i != m_childContexts.end(); ++i) {
-// Extensions3DChromium* ext = static_cast<Extensions3DChromium*>(i->first->getExtensions());
-// GC3Duint childToParentLatchId, parentToChildLatchId;
-// ext->getParentToChildLatchCHROMIUM(&parentToChildLatchId);
-// ext->waitLatchCHROMIUM(parentToChildLatchId);
-// }
-// }
-
updateCompositorResources(renderSurfaceLayerList);
// Update compositor resources for root layer.
{
TRACE_EVENT("LayerRendererChromium::updateLayer::updateRoot", this, 0);
m_rootLayerContentTiler->updateRect();
}
-
- // After updateCompositorResources, set/wait latches for all child
- // contexts. This will prevent the compositor from using any of the child
- // parent textures while WebGL commands are executing from _javascript_ *and*
- // while the final parent texture is being blit'd. copyTexImage2D
- // uses the parent texture as a temporary resolve buffer, so that's why the
- // waitLatch is below, to block the compositor from using the parent texture
- // until the next WebGL SwapBuffers (or copyTextureToParentTexture for
- // Canvas2D).
- if (hardwareCompositing() && m_contextSupportsLatch) {
- m_childContextsWereCopied = true;
- // For each child context:
- // glSetLatch(Offscreen->Compositor);
- // glWaitLatch(Compositor->Offscreen);
- ChildContextMap::iterator i = m_childContexts.begin();
- for (; i != m_childContexts.end(); ++i) {
- Extensions3DChromium* ext = static_cast<Extensions3DChromium*>(i->first->getExtensions());
- GC3Duint childToParentLatchId, parentToChildLatchId;
- ext->getParentToChildLatchCHROMIUM(&parentToChildLatchId);
- ext->getChildToParentLatchCHROMIUM(&childToParentLatchId);
- ext->setLatchCHROMIUM(childToParentLatchId);
- ext->waitLatchCHROMIUM(parentToChildLatchId);
- }
- }
}
void LayerRendererChromium::paintLayerContents(const LayerList& renderSurfaceLayerList)
@@ -1295,6 +1253,11 @@
}
}
+bool LayerRendererChromium::isCompositorContextLost()
+{
+ return (m_context.get()->getExtensions()->getGraphicsResetStatusARB() != GraphicsContext3D::NO_ERROR);
+}
+
void LayerRendererChromium::dumpRenderSurfaces(TextStream& ts, int indent, LayerChromium* layer) const
{
if (layer->ccLayerImpl()->renderSurface())
Modified: trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.h (89634 => 89635)
--- trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.h 2011-06-23 23:47:39 UTC (rev 89634)
+++ trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.h 2011-06-24 00:11:51 UTC (rev 89635)
@@ -150,6 +150,9 @@
void addChildContext(GraphicsContext3D*);
void removeChildContext(GraphicsContext3D*);
+ // Return true if the compositor context has an error.
+ bool isCompositorContextLost();
+
#ifndef NDEBUG
static bool s_inPaintLayerContents;
#endif
@@ -241,11 +244,6 @@
#endif
ChildContextMap m_childContexts;
- // If true, the child contexts were copied to the compositor texture targets
- // and the compositor will need to wait on the proper latches before using
- // the target textures. If false, the compositor is reusing the textures
- // from last frame.
- bool m_childContextsWereCopied;
bool m_contextSupportsLatch;
bool m_contextSupportsMapSub;
Modified: trunk/Source/WebCore/platform/graphics/chromium/WebGLLayerChromium.cpp (89634 => 89635)
--- trunk/Source/WebCore/platform/graphics/chromium/WebGLLayerChromium.cpp 2011-06-23 23:47:39 UTC (rev 89634)
+++ trunk/Source/WebCore/platform/graphics/chromium/WebGLLayerChromium.cpp 2011-06-24 00:11:51 UTC (rev 89635)
@@ -61,9 +61,14 @@
layerRenderer()->removeChildContext(m_context);
}
+bool WebGLLayerChromium::drawsContent() const
+{
+ return (m_context && m_context->getExtensions()->getGraphicsResetStatusARB() == GraphicsContext3D::NO_ERROR);
+}
+
void WebGLLayerChromium::updateCompositorResources()
{
- if (!m_context)
+ if (!drawsContent())
return;
if (!m_contentsDirty)
@@ -103,7 +108,8 @@
void WebGLLayerChromium::setContext(const GraphicsContext3D* context)
{
- if (m_context != context && layerRenderer()) {
+ bool contextChanged = (m_context != context);
+ if (contextChanged && layerRenderer()) {
if (m_context)
layerRenderer()->removeChildContext(m_context);
if (context)
@@ -116,7 +122,7 @@
return;
unsigned int textureId = m_context->platformTexture();
- if (textureId != m_textureId) {
+ if (textureId != m_textureId || contextChanged) {
m_textureChanged = true;
m_textureUpdated = true;
}
Modified: trunk/Source/WebCore/platform/graphics/chromium/WebGLLayerChromium.h (89634 => 89635)
--- trunk/Source/WebCore/platform/graphics/chromium/WebGLLayerChromium.h 2011-06-23 23:47:39 UTC (rev 89634)
+++ trunk/Source/WebCore/platform/graphics/chromium/WebGLLayerChromium.h 2011-06-24 00:11:51 UTC (rev 89635)
@@ -49,7 +49,7 @@
virtual ~WebGLLayerChromium();
- virtual bool drawsContent() const { return m_context; }
+ virtual bool drawsContent() const;
virtual void updateCompositorResources();
void setTextureUpdated();
Modified: trunk/Source/WebKit/chromium/ChangeLog (89634 => 89635)
--- trunk/Source/WebKit/chromium/ChangeLog 2011-06-23 23:47:39 UTC (rev 89634)
+++ trunk/Source/WebKit/chromium/ChangeLog 2011-06-24 00:11:51 UTC (rev 89635)
@@ -1,3 +1,13 @@
+2011-06-23 John Bates <jba...@google.com>
+
+ Reviewed by James Robinson.
+
+ Fix latch deadlock when GPU process crashes or context is lost
+ https://bugs.webkit.org/show_bug.cgi?id=63189
+
+ * src/WebViewImpl.cpp:
+ (WebKit::WebViewImpl::composite):
+
2011-06-23 Ryosuke Niwa <rn...@webkit.org>
Rolled DEPS.
Modified: trunk/Source/WebKit/chromium/src/WebViewImpl.cpp (89634 => 89635)
--- trunk/Source/WebKit/chromium/src/WebViewImpl.cpp 2011-06-23 23:47:39 UTC (rev 89634)
+++ trunk/Source/WebKit/chromium/src/WebViewImpl.cpp 2011-06-24 00:11:51 UTC (rev 89635)
@@ -1154,8 +1154,7 @@
// Put result onscreen.
m_layerRenderer->present();
- GraphicsContext3D* context = m_layerRenderer->context();
- if (context->getExtensions()->getGraphicsResetStatusARB() != GraphicsContext3D::NO_ERROR) {
+ if (m_layerRenderer->isCompositorContextLost()) {
// Trying to recover the context right here will not work if GPU process
// died. This is because GpuChannelHost::OnErrorMessage will only be
// called at the next iteration of the message loop, reverting our
