Changeset 116587 in webkit


Ignore:
Timestamp:
May 9, 2012 5:41:28 PM (12 years ago)
Author:
danakj@chromium.org
Message:

[chromium] Don't draw when canDraw() is false
​https://bugs.webkit.org/show_bug.cgi?id=85829

Reviewed by Adrienne Walker.

Source/WebCore:

This is based on the work of Daniel Sievers in bug
​https://bugs.webkit.org/show_bug.cgi?id=82680. When canDraw() is false,
we should not call drawLayers() or prepareToDraw() in both Single- and
Multi-Threaded mode.

drawLayers() is crashing in single threaded mode, and this attempts to
prevent it from being called with invalid state. While making it behave
properly in single-threaded mode, it seems appropriate to unrevert the
parts of 82680 that made threaded mode behave similarly appropriately.

A single-threaded test is not included since LTHTests is unable to run
in single-threaded mode at this time (pending work from Ian Vollick). So
we test in threaded mode only with a note to include a single thread
version.

Tests: CCLayerTreeHostTestCanDrawBlocksDrawing.runMultiThread

  • platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:

(WebCore::CCLayerTreeHostImpl::prepareToDraw):
(WebCore::CCLayerTreeHostImpl::drawLayers):

  • platform/graphics/chromium/cc/CCSingleThreadProxy.cpp:

(WebCore::CCSingleThreadProxy::doComposite):

  • platform/graphics/chromium/cc/CCThreadProxy.cpp:

(WebCore::CCThreadProxy::scheduledActionDrawAndSwapInternal):

Source/WebKit/chromium:

  • tests/CCLayerTreeHostImplTest.cpp:

(WebKitTests::CCLayerTreeHostImplTest::CCLayerTreeHostImplTest):
(WebKitTests::TEST_F):

  • tests/CCLayerTreeHostTest.cpp:

(WTF):
(CCLayerTreeHostTestCanDrawBlocksDrawing):
(WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::CCLayerTreeHostTestCanDrawBlocksDrawing):
(WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::beginTest):
(WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::commitCompleteOnCCThread):
(WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::drawLayersOnCCThread):
(WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::didCommitAndDrawFrame):
(WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::afterTest):
(WTF::TEST_F):

Location:
trunk/Source
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r116585 r116587  
     12012-05-09  Dana Jansens  <danakj@chromium.org>
     2
     3        [chromium] Don't draw when canDraw() is false
     4        https://bugs.webkit.org/show_bug.cgi?id=85829
     5
     6        Reviewed by Adrienne Walker.
     7
     8        This is based on the work of Daniel Sievers in bug
     9        https://bugs.webkit.org/show_bug.cgi?id=82680. When canDraw() is false,
     10        we should not call drawLayers() or prepareToDraw() in both Single- and
     11        Multi-Threaded mode.
     12
     13        drawLayers() is crashing in single threaded mode, and this attempts to
     14        prevent it from being called with invalid state. While making it behave
     15        properly in single-threaded mode, it seems appropriate to unrevert the
     16        parts of 82680 that made threaded mode behave similarly appropriately.
     17
     18        A single-threaded test is not included since LTHTests is unable to run
     19        in single-threaded mode at this time (pending work from Ian Vollick). So
     20        we test in threaded mode only with a note to include a single thread
     21        version.
     22
     23        Tests: CCLayerTreeHostTestCanDrawBlocksDrawing.runMultiThread
     24
     25        * platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:
     26        (WebCore::CCLayerTreeHostImpl::prepareToDraw):
     27        (WebCore::CCLayerTreeHostImpl::drawLayers):
     28        * platform/graphics/chromium/cc/CCSingleThreadProxy.cpp:
     29        (WebCore::CCSingleThreadProxy::doComposite):
     30        * platform/graphics/chromium/cc/CCThreadProxy.cpp:
     31        (WebCore::CCThreadProxy::scheduledActionDrawAndSwapInternal):
     32
    1332012-05-09  Martin Robinson  <mrobinson@igalia.com>
    234

  • trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp

    r116351 r116587  
    381381{
    382382    TRACE_EVENT("CCLayerTreeHostImpl::prepareToDraw", this, 0);
     383    ASSERT(canDraw());
    383384
    384385    frame.renderPasses.clear();
    385386    frame.renderSurfaceLayerList.clear();
    386 
    387     if (!m_rootLayerImpl)
    388         return false;
    389387
    390388    if (!calculateRenderPasses(frame.renderPasses, frame.renderSurfaceLayerList)) {
    … …  
    406404{
    407405    TRACE_EVENT("CCLayerTreeHostImpl::drawLayers", this, 0);
    408     ASSERT(m_layerRenderer);
    409 
    410     if (!m_rootLayerImpl)
    411         return;
     406    ASSERT(canDraw());
    412407
    413408    // FIXME: use the frame begin time from the overall compositor scheduler.

  • trunk/Source/WebCore/platform/graphics/chromium/cc/CCSingleThreadProxy.cpp

    r116351 r116587  
    349349      double monotonicTime = monotonicallyIncreasingTime();
    350350      double wallClockTime = currentTime();
    351 
    352351      m_layerTreeHostImpl->animate(monotonicTime, wallClockTime);
     352
     353      // We guard prepareToDraw() with canDraw() because it always returns a valid frame, so can only
     354      // be used when such a frame is possible. Since drawLayers() depends on the result of
     355      // prepareToDraw(), it is guarded on canDraw() as well.
     356      if (!m_layerTreeHostImpl->canDraw())
     357          return false;
     358
    353359      CCLayerTreeHostImpl::FrameData frame;
    354360      m_layerTreeHostImpl->prepareToDraw(frame);

  • trunk/Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp

    r116351 r116587  
    640640    m_inputHandlerOnImplThread->animate(monotonicTime);
    641641    m_layerTreeHostImpl->animate(monotonicTime, wallClockTime);
     642
     643    // This method is called on a forced draw, regardless of whether we are able to produce a frame,
     644    // as the calling site on main thread is blocked until its request completes, and we signal
     645    // completion here. If canDraw() is false, we will indicate success=false to the caller, but we
     646    // must still signal completion to avoid deadlock.
     647
     648    // We guard prepareToDraw() with canDraw() because it always returns a valid frame, so can only
     649    // be used when such a frame is possible. Since drawLayers() depends on the result of
     650    // prepareToDraw(), it is guarded on canDraw() as well.
     651
    642652    CCLayerTreeHostImpl::FrameData frame;
    643     bool drawFrame = m_layerTreeHostImpl->prepareToDraw(frame) || forcedDraw;
     653    bool drawFrame = m_layerTreeHostImpl->canDraw() && (m_layerTreeHostImpl->prepareToDraw(frame) || forcedDraw);
    644654    if (drawFrame) {
    645655        m_layerTreeHostImpl->drawLayers(frame);
    … …  
    650660    // Check for a pending compositeAndReadback.
    651661    if (m_readbackRequestOnImplThread) {
    652         ASSERT(drawFrame); // This should be a forcedDraw
    653         m_layerTreeHostImpl->readback(m_readbackRequestOnImplThread->pixels, m_readbackRequestOnImplThread->rect);
    654         m_readbackRequestOnImplThread->success = !m_layerTreeHostImpl->isContextLost();
     662        m_readbackRequestOnImplThread->success = false;
     663        if (drawFrame) {
     664            m_layerTreeHostImpl->readback(m_readbackRequestOnImplThread->pixels, m_readbackRequestOnImplThread->rect);
     665            m_readbackRequestOnImplThread->success = !m_layerTreeHostImpl->isContextLost();
     666        }
    655667        m_readbackRequestOnImplThread->completion.signal();
    656668        m_readbackRequestOnImplThread = 0;
    … …  
    662674    // Process any finish request
    663675    if (m_finishAllRenderingCompletionEventOnImplThread) {
    664         ASSERT(drawFrame); // This should be a forcedDraw
    665676        m_layerTreeHostImpl->finishAllRendering();
    666677        m_finishAllRenderingCompletionEventOnImplThread->signal();
    … …  
    674685    }
    675686
    676     ASSERT(drawFrame || (!drawFrame && !forcedDraw));
    677687    return result;
    678688}

  • trunk/Source/WebKit/chromium/ChangeLog

    r116582 r116587  
     12012-05-09  Dana Jansens  <danakj@chromium.org>
     2
     3        [chromium] Don't draw when canDraw() is false
     4        https://bugs.webkit.org/show_bug.cgi?id=85829
     5
     6        Reviewed by Adrienne Walker.
     7
     8        * tests/CCLayerTreeHostImplTest.cpp:
     9        (WebKitTests::CCLayerTreeHostImplTest::CCLayerTreeHostImplTest):
     10        (WebKitTests::TEST_F):
     11        * tests/CCLayerTreeHostTest.cpp:
     12        (WTF):
     13        (CCLayerTreeHostTestCanDrawBlocksDrawing):
     14        (WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::CCLayerTreeHostTestCanDrawBlocksDrawing):
     15        (WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::beginTest):
     16        (WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::commitCompleteOnCCThread):
     17        (WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::drawLayersOnCCThread):
     18        (WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::didCommitAndDrawFrame):
     19        (WTF::CCLayerTreeHostTestCanDrawBlocksDrawing::afterTest):
     20        (WTF::TEST_F):
     21
    1222012-05-09  Sheriff Bot  <webkit.review.bot@gmail.com>
    223

  • trunk/Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp

    r116351 r116587  
    6161        CCSettings settings;
    6262        m_hostImpl = CCLayerTreeHostImpl::create(settings, this);
     63        m_hostImpl->initializeLayerRenderer(createContext(), adoptPtr(new FakeTextureUploader));
     64        m_hostImpl->setViewportSize(IntSize(10, 10));
    6365    }
    6466
    … …  
    256258TEST_F(CCLayerTreeHostImplTest, nonFastScrollableRegionWithOffset)
    257259{
    258     m_hostImpl->initializeLayerRenderer(createContext(), adoptPtr(new FakeTextureUploader));
    259 
    260260    OwnPtr<CCLayerImpl> root = CCLayerImpl::create(0);
    261261    root->setScrollable(true);
    … …  
    442442TEST_F(CCLayerTreeHostImplTest, didDrawNotCalledOnHiddenLayer)
    443443{
    444     m_hostImpl->initializeLayerRenderer(createContext(), adoptPtr(new FakeTextureUploader));
    445 
    446     // Ensure visibleLayerRect for root layer is empty
    447     m_hostImpl->setViewportSize(IntSize(0, 0));
    448 
     444    // The root layer is always drawn, so run this test on a child layer that
     445    // will be masked out by the root layer's bounds.
    449446    m_hostImpl->setRootLayer(DidDrawCheckLayer::create(0));
    450447    DidDrawCheckLayer* root = static_cast<DidDrawCheckLayer*>(m_hostImpl->rootLayer());
     448    root->setMasksToBounds(true);
     449
     450    root->addChild(DidDrawCheckLayer::create(1));
     451    DidDrawCheckLayer* layer = static_cast<DidDrawCheckLayer*>(root->children()[0].get());
     452    // Ensure visibleLayerRect for layer is empty
     453    layer->setPosition(FloatPoint(100, 100));
     454    layer->setBounds(IntSize(10, 10));
     455    layer->setContentBounds(IntSize(10, 10));
    451456
    452457    CCLayerTreeHostImpl::FrameData frame;
    453458
    454     EXPECT_FALSE(root->willDrawCalled());
    455     EXPECT_FALSE(root->didDrawCalled());
    456 
    457     EXPECT_TRUE(m_hostImpl->prepareToDraw(frame));
    458     m_hostImpl->drawLayers(frame);
    459     m_hostImpl->didDrawAllLayers(frame);
    460 
    461     EXPECT_FALSE(root->willDrawCalled());
    462     EXPECT_FALSE(root->didDrawCalled());
    463 
    464     EXPECT_TRUE(root->visibleLayerRect().isEmpty());
    465 
    466     // Ensure visibleLayerRect for root layer is not empty
    467     m_hostImpl->setViewportSize(IntSize(10, 10));
    468 
    469     EXPECT_FALSE(root->willDrawCalled());
    470     EXPECT_FALSE(root->didDrawCalled());
    471 
    472     EXPECT_TRUE(m_hostImpl->prepareToDraw(frame));
    473     m_hostImpl->drawLayers(frame);
    474     m_hostImpl->didDrawAllLayers(frame);
    475 
    476     EXPECT_TRUE(root->willDrawCalled());
    477     EXPECT_TRUE(root->didDrawCalled());
    478 
    479     EXPECT_FALSE(root->visibleLayerRect().isEmpty());
     459    EXPECT_FALSE(layer->willDrawCalled());
     460    EXPECT_FALSE(layer->didDrawCalled());
     461
     462    EXPECT_TRUE(m_hostImpl->prepareToDraw(frame));
     463    m_hostImpl->drawLayers(frame);
     464    m_hostImpl->didDrawAllLayers(frame);
     465
     466    EXPECT_FALSE(layer->willDrawCalled());
     467    EXPECT_FALSE(layer->didDrawCalled());
     468
     469    EXPECT_TRUE(layer->visibleLayerRect().isEmpty());
     470
     471    // Ensure visibleLayerRect for layer layer is not empty
     472    layer->setPosition(FloatPoint(0, 0));
     473
     474    EXPECT_FALSE(layer->willDrawCalled());
     475    EXPECT_FALSE(layer->didDrawCalled());
     476
     477    EXPECT_TRUE(m_hostImpl->prepareToDraw(frame));
     478    m_hostImpl->drawLayers(frame);
     479    m_hostImpl->didDrawAllLayers(frame);
     480
     481    EXPECT_TRUE(layer->willDrawCalled());
     482    EXPECT_TRUE(layer->didDrawCalled());
     483
     484    EXPECT_FALSE(layer->visibleLayerRect().isEmpty());
    480485}
    481486
    482487TEST_F(CCLayerTreeHostImplTest, didDrawCalledOnAllLayers)
    483488{
    484     m_hostImpl->initializeLayerRenderer(createContext(), adoptPtr(new FakeTextureUploader));
    485     m_hostImpl->setViewportSize(IntSize(10, 10));
    486 
    487489    m_hostImpl->setRootLayer(DidDrawCheckLayer::create(0));
    488490    DidDrawCheckLayer* root = static_cast<DidDrawCheckLayer*>(m_hostImpl->rootLayer());
    … …  
    535537TEST_F(CCLayerTreeHostImplTest, prepareToDrawFailsWhenAnimationUsesCheckerboard)
    536538{
    537     m_hostImpl->initializeLayerRenderer(createContext(), adoptPtr(new FakeTextureUploader));
    538     m_hostImpl->setViewportSize(IntSize(10, 10));
    539 
    540539    // When the texture is not missing, we draw as usual.
    541540    m_hostImpl->setRootLayer(DidDrawCheckLayer::create(0));
    … …  
    662661TEST_F(CCLayerTreeHostImplTest, blendingOffWhenDrawingOpaqueLayers)
    663662{
    664     m_hostImpl->initializeLayerRenderer(createContext(), adoptPtr(new FakeTextureUploader));
    665     m_hostImpl->setViewportSize(IntSize(10, 10));
    666663
    667664    {
    … …  
    992989    RefPtr<GraphicsContext3D> context = GraphicsContext3DPrivate::createGraphicsContextFromWebContext(adoptPtr(reshapeTracker), GraphicsContext3D::RenderDirectlyToHostWindow);
    993990    m_hostImpl->initializeLayerRenderer(context, adoptPtr(new FakeTextureUploader));
    994     m_hostImpl->setViewportSize(IntSize(10, 10));
    995991
    996992    CCLayerImpl* root = new FakeDrawableCCLayerImpl(1);
    … …  
    11271123TEST_F(CCLayerTreeHostImplTest, contextLostAndRestoredNotificationSentToAllLayers)
    11281124{
    1129     m_hostImpl->initializeLayerRenderer(createContext(), adoptPtr(new FakeTextureUploader));
    1130     m_hostImpl->setViewportSize(IntSize(10, 10));
    1131 
    11321125    m_hostImpl->setRootLayer(ContextLostNotificationCheckLayer::create(0));
    11331126    ContextLostNotificationCheckLayer* root = static_cast<ContextLostNotificationCheckLayer*>(m_hostImpl->rootLayer());
    … …  
    11741167TEST_F(CCLayerTreeHostImplTest, scrollbarLayerLostContext)
    11751168{
    1176     m_hostImpl->initializeLayerRenderer(createContext(), adoptPtr(new FakeTextureUploader));
    1177     m_hostImpl->setViewportSize(IntSize(10, 10));
    1178 
    11791169    m_hostImpl->setRootLayer(ScrollbarLayerFakePaint::create(0));
    11801170    ScrollbarLayerFakePaint* scrollbar = static_cast<ScrollbarLayerFakePaint*>(m_hostImpl->rootLayer());
    … …  
    13241314TEST_F(CCLayerTreeHostImplTest, dontUseOldResourcesAfterLostContext)
    13251315{
    1326     m_hostImpl->initializeLayerRenderer(createContext(), adoptPtr(new FakeTextureUploader));
    1327     m_hostImpl->setViewportSize(IntSize(10, 10));
    1328 
    13291316    OwnPtr<CCLayerImpl> rootLayer(CCLayerImpl::create(0));
    13301317    rootLayer->setBounds(IntSize(10, 10));

  • trunk/Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp

    r116500 r116587  
    841841}
    842842
     843// If the layerTreeHost says it can't draw, then we should not try to draw.
     844// FIXME: Make this run in single threaded mode too. http://crbug.com/127481
     845class CCLayerTreeHostTestCanDrawBlocksDrawing : public CCLayerTreeHostTestThreadOnly {
     846public:
     847    CCLayerTreeHostTestCanDrawBlocksDrawing()
     848        : m_numCommits(0)
     849    {
     850    }
     851
     852    virtual void beginTest()
     853    {
     854    }
     855
     856    virtual void drawLayersOnCCThread(CCLayerTreeHostImpl* impl)
     857    {
     858        // Only the initial draw should bring us here.
     859        EXPECT_TRUE(impl->canDraw());
     860        EXPECT_EQ(0, impl->sourceFrameNumber());
     861    }
     862
     863    virtual void commitCompleteOnCCThread(CCLayerTreeHostImpl* impl)
     864    {
     865        if (m_numCommits >= 1) {
     866            // After the first commit, we should not be able to draw.
     867            EXPECT_FALSE(impl->canDraw());
     868        }
     869    }
     870
     871    virtual void didCommitAndDrawFrame()
     872    {
     873        m_numCommits++;
     874        if (m_numCommits == 1) {
     875            // Make the viewport empty so the host says it can't draw.
     876            m_layerTreeHost->setViewportSize(IntSize(0, 0));
     877
     878            OwnArrayPtr<char> pixels(adoptArrayPtr(new char[4]));
     879            m_layerTreeHost->compositeAndReadback(static_cast<void*>(pixels.get()), IntRect(0, 0, 1, 1));
     880        } else if (m_numCommits == 2) {
     881            m_layerTreeHost->setNeedsCommit();
     882            m_layerTreeHost->finishAllRendering();
     883            endTest();
     884        }
     885    }
     886
     887    virtual void afterTest()
     888    {
     889    }
     890
     891private:
     892    int m_numCommits;
     893};
     894
     895TEST_F(CCLayerTreeHostTestCanDrawBlocksDrawing, runMultiThread)
     896{
     897    runTestThreaded();
     898}
    843899
    844900// beginLayerWrite should prevent draws from executing until a commit occurs
Note: See TracChangeset for help on using the changeset viewer.