Changeset 89635 in webkit


Ignore:
Timestamp:
Jun 23, 2011 5:11:51 PM (13 years ago)
Author:
commit-queue@webkit.org
Message:

2011-06-23 John Bates <jbates@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 John Bates <jbates@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):
Location:
trunk/Source
Files:
10 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r89634 r89635  
     12011-06-23  John Bates  <jbates@google.com>
     2
     3        Reviewed by James Robinson.
     4
     5        Fix latch deadlock when GPU process crashes or context is lost.
     6        https://bugs.webkit.org/show_bug.cgi?id=63189
     7        The main bug fix is to only set/wait latches if the child context has no errors.
     8        Additionally, the LayerChromium classes needed to be modified to not continue drawing when
     9        their corresponding contexts have errors. Otherwise, they would draw with invalid texture ids.
     10
     11        Test: open particles WebGL demo in chrome, kill GPU process from Task Manager; observe no deadlock.
     12
     13        * platform/graphics/chromium/LayerRendererChromium.cpp:
     14        (WebCore::LayerRendererChromium::LayerRendererChromium):
     15        (WebCore::LayerRendererChromium::updateAndDrawLayers):
     16        (WebCore::LayerRendererChromium::updateLayers):
     17        (WebCore::LayerRendererChromium::isCompositorContextLost):
     18        * platform/graphics/chromium/LayerRendererChromium.h:
     19        * platform/graphics/chromium/WebGLLayerChromium.cpp:
     20        (WebCore::WebGLLayerChromium::drawsContent):
     21        (WebCore::WebGLLayerChromium::updateCompositorResources):
     22        (WebCore::WebGLLayerChromium::setContext):
     23        * platform/graphics/chromium/WebGLLayerChromium.h:
     24        * platform/graphics/chromium/Canvas2DLayerChromium.cpp:
     25        (WebCore::Canvas2DLayerChromium::drawsContent):
     26        * platform/graphics/chromium/Canvas2DLayerChromium.h:
     27
    1282011-06-23  Alok Priyadarshi  <alokp@chromium.org>
    229
  • trunk/Source/WebCore/platform/graphics/Extensions3D.h

    r83762 r89635  
    107107
    108108    // GL_ARB_robustness
     109    // Note: This method's behavior differs from the GL_ARB_robustness
     110    // specification in the following way:
     111    // The implementation must not reset the error state during this call.
     112    // If getGraphicsResetStatusARB returns an error, it should continue
     113    // returning the same error. Restoring the GraphicsContext3D is handled
     114    // externally.
    109115    virtual int getGraphicsResetStatusARB() = 0;
    110116   
  • trunk/Source/WebCore/platform/graphics/chromium/Canvas2DLayerChromium.cpp

    r85661 r89635  
    3636
    3737#include "DrawingBuffer.h"
     38#include "Extensions3DChromium.h"
    3839#include "GraphicsContext3D.h"
    3940#include "LayerRendererChromium.h"
     
    6061}
    6162
     63bool Canvas2DLayerChromium::drawsContent() const
     64{
     65    GraphicsContext3D* context;
     66    return (m_drawingBuffer
     67            && (context = m_drawingBuffer->graphicsContext3D().get())
     68            && (context->getExtensions()->getGraphicsResetStatusARB() == GraphicsContext3D::NO_ERROR));
     69}
     70
    6271void Canvas2DLayerChromium::updateCompositorResources()
    6372{
    64     if (!m_contentsDirty || !m_drawingBuffer)
     73    if (!m_contentsDirty || !drawsContent())
    6574        return;
    6675    if (m_textureChanged) { // We have to generate a new backing texture.
  • trunk/Source/WebCore/platform/graphics/chromium/Canvas2DLayerChromium.h

    r83552 r89635  
    4646    static PassRefPtr<Canvas2DLayerChromium> create(DrawingBuffer*, GraphicsLayerChromium* owner);
    4747    virtual ~Canvas2DLayerChromium();
    48     virtual bool drawsContent() const { return true; }
     48    virtual bool drawsContent() const;
    4949    virtual void updateCompositorResources();
    5050
  • trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp

    r89634 r89635  
    121121    , m_compositeOffscreen(false)
    122122    , m_context(context)
    123     , m_childContextsWereCopied(false)
    124123    , m_contextSupportsLatch(false)
    125124    , m_contextSupportsTextureFormatBGRA(false)
     
    256255        // potentially clobbering the parent texture that is being renderered
    257256        // by the compositor thread.
    258         if (m_childContextsWereCopied) {
    259             Extensions3DChromium* parentExt = static_cast<Extensions3DChromium*>(m_context->getExtensions());
    260             // For each child context:
    261             //   glWaitLatch(Offscreen->Compositor);
    262             ChildContextMap::iterator i = m_childContexts.begin();
    263             for (; i != m_childContexts.end(); ++i) {
    264                 Extensions3DChromium* childExt = static_cast<Extensions3DChromium*>(i->first->getExtensions());
    265                 GC3Duint latchId;
    266                 childExt->getChildToParentLatchCHROMIUM(&latchId);
    267                 parentExt->waitLatchCHROMIUM(latchId);
     257        Extensions3DChromium* parentExt = static_cast<Extensions3DChromium*>(m_context->getExtensions());
     258        // For each child context:
     259        //   glWaitLatch(Offscreen->Compositor);
     260        ChildContextMap::iterator i = m_childContexts.begin();
     261        for (; i != m_childContexts.end(); ++i) {
     262            Extensions3DChromium* childExt = static_cast<Extensions3DChromium*>(i->first->getExtensions());
     263            if (childExt->getGraphicsResetStatusARB() == GraphicsContext3D::NO_ERROR) {
     264                GC3Duint childToParentLatchId;
     265                childExt->getChildToParentLatchCHROMIUM(&childToParentLatchId);
     266                childExt->setLatchCHROMIUM(childToParentLatchId);
     267                parentExt->waitLatchCHROMIUM(childToParentLatchId);
    268268            }
    269269        }
    270         // Reset to false to indicate that we have consumed the dirty child
    271         // contexts' parent textures. (This is only useful when the compositor
    272         // is multithreaded.)
    273         m_childContextsWereCopied = false;
    274270    }
    275271
     
    286282        for (; i != m_childContexts.end(); ++i) {
    287283            Extensions3DChromium* childExt = static_cast<Extensions3DChromium*>(i->first->getExtensions());
    288             GC3Duint latchId;
    289             childExt->getParentToChildLatchCHROMIUM(&latchId);
    290             parentExt->setLatchCHROMIUM(latchId);
     284            if (childExt->getGraphicsResetStatusARB() == GraphicsContext3D::NO_ERROR) {
     285                GC3Duint parentToChildLatchId;
     286                childExt->getParentToChildLatchCHROMIUM(&parentToChildLatchId);
     287                parentExt->setLatchCHROMIUM(parentToChildLatchId);
     288                childExt->waitLatchCHROMIUM(parentToChildLatchId);
     289            }
    291290        }
    292291    }
     
    341340#endif
    342341
    343     // FIXME: Before updateCompositorResources, when the compositor runs in
    344     // its own thread, and when the copyTexImage2D bug is fixed, insert
    345     // a glWaitLatch(Compositor->Offscreen) on all child contexts here instead
    346     // of after updateCompositorResources.
    347     // Also uncomment the glSetLatch(Compositor->Offscreen) code in addChildContext.
    348 //  if (hardwareCompositing() && m_contextSupportsLatch) {
    349 //      // For each child context:
    350 //      //   glWaitLatch(Compositor->Offscreen);
    351 //      ChildContextMap::iterator i = m_childContexts.begin();
    352 //      for (; i != m_childContexts.end(); ++i) {
    353 //          Extensions3DChromium* ext = static_cast<Extensions3DChromium*>(i->first->getExtensions());
    354 //          GC3Duint childToParentLatchId, parentToChildLatchId;
    355 //          ext->getParentToChildLatchCHROMIUM(&parentToChildLatchId);
    356 //          ext->waitLatchCHROMIUM(parentToChildLatchId);
    357 //      }
    358 //  }
    359 
    360342    updateCompositorResources(renderSurfaceLayerList);
    361343    // Update compositor resources for root layer.
     
    363345        TRACE_EVENT("LayerRendererChromium::updateLayer::updateRoot", this, 0);
    364346        m_rootLayerContentTiler->updateRect();
    365     }
    366 
    367     // After updateCompositorResources, set/wait latches for all child
    368     // contexts. This will prevent the compositor from using any of the child
    369     // parent textures while WebGL commands are executing from javascript *and*
    370     // while the final parent texture is being blit'd. copyTexImage2D
    371     // uses the parent texture as a temporary resolve buffer, so that's why the
    372     // waitLatch is below, to block the compositor from using the parent texture
    373     // until the next WebGL SwapBuffers (or copyTextureToParentTexture for
    374     // Canvas2D).
    375     if (hardwareCompositing() && m_contextSupportsLatch) {
    376         m_childContextsWereCopied = true;
    377         // For each child context:
    378         //   glSetLatch(Offscreen->Compositor);
    379         //   glWaitLatch(Compositor->Offscreen);
    380         ChildContextMap::iterator i = m_childContexts.begin();
    381         for (; i != m_childContexts.end(); ++i) {
    382             Extensions3DChromium* ext = static_cast<Extensions3DChromium*>(i->first->getExtensions());
    383             GC3Duint childToParentLatchId, parentToChildLatchId;
    384             ext->getParentToChildLatchCHROMIUM(&parentToChildLatchId);
    385             ext->getChildToParentLatchCHROMIUM(&childToParentLatchId);
    386             ext->setLatchCHROMIUM(childToParentLatchId);
    387             ext->waitLatchCHROMIUM(parentToChildLatchId);
    388         }
    389347    }
    390348}
     
    12961254}
    12971255
     1256bool LayerRendererChromium::isCompositorContextLost()
     1257{
     1258    return (m_context.get()->getExtensions()->getGraphicsResetStatusARB() != GraphicsContext3D::NO_ERROR);
     1259}
     1260
    12981261void LayerRendererChromium::dumpRenderSurfaces(TextStream& ts, int indent, LayerChromium* layer) const
    12991262{
  • trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.h

    r89634 r89635  
    151151    void removeChildContext(GraphicsContext3D*);
    152152
     153    // Return true if the compositor context has an error.
     154    bool isCompositorContextLost();
     155
    153156#ifndef NDEBUG
    154157    static bool s_inPaintLayerContents;
     
    242245
    243246    ChildContextMap m_childContexts;
    244     // If true, the child contexts were copied to the compositor texture targets
    245     // and the compositor will need to wait on the proper latches before using
    246     // the target textures. If false, the compositor is reusing the textures
    247     // from last frame.
    248     bool m_childContextsWereCopied;
    249247
    250248    bool m_contextSupportsLatch;
  • trunk/Source/WebCore/platform/graphics/chromium/WebGLLayerChromium.cpp

    r86905 r89635  
    6262}
    6363
     64bool WebGLLayerChromium::drawsContent() const
     65{
     66    return (m_context && m_context->getExtensions()->getGraphicsResetStatusARB() == GraphicsContext3D::NO_ERROR);
     67}
     68
    6469void WebGLLayerChromium::updateCompositorResources()
    6570{
    66     if (!m_context)
     71    if (!drawsContent())
    6772        return;
    6873
     
    104109void WebGLLayerChromium::setContext(const GraphicsContext3D* context)
    105110{
    106     if (m_context != context && layerRenderer()) {
     111    bool contextChanged = (m_context != context);
     112    if (contextChanged && layerRenderer()) {
    107113        if (m_context)
    108114            layerRenderer()->removeChildContext(m_context);
     
    117123
    118124    unsigned int textureId = m_context->platformTexture();
    119     if (textureId != m_textureId) {
     125    if (textureId != m_textureId || contextChanged) {
    120126        m_textureChanged = true;
    121127        m_textureUpdated = true;
  • trunk/Source/WebCore/platform/graphics/chromium/WebGLLayerChromium.h

    r86278 r89635  
    5050    virtual ~WebGLLayerChromium();
    5151
    52     virtual bool drawsContent() const { return m_context; }
     52    virtual bool drawsContent() const;
    5353    virtual void updateCompositorResources();
    5454    void setTextureUpdated();
  • trunk/Source/WebKit/chromium/ChangeLog

    r89628 r89635  
     12011-06-23  John Bates  <jbates@google.com>
     2
     3        Reviewed by James Robinson.
     4
     5        Fix latch deadlock when GPU process crashes or context is lost
     6        https://bugs.webkit.org/show_bug.cgi?id=63189
     7
     8        * src/WebViewImpl.cpp:
     9        (WebKit::WebViewImpl::composite):
     10
    1112011-06-23  Ryosuke Niwa  <rniwa@webkit.org>
    212
  • trunk/Source/WebKit/chromium/src/WebViewImpl.cpp

    r89529 r89635  
    11551155    m_layerRenderer->present();
    11561156
    1157     GraphicsContext3D* context = m_layerRenderer->context();
    1158     if (context->getExtensions()->getGraphicsResetStatusARB() != GraphicsContext3D::NO_ERROR) {
     1157    if (m_layerRenderer->isCompositorContextLost()) {
    11591158        // Trying to recover the context right here will not work if GPU process
    11601159        // died. This is because GpuChannelHost::OnErrorMessage will only be
Note: See TracChangeset for help on using the changeset viewer.