⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 100054 in webkit


Ignore:
Timestamp:
Nov 11, 2011, 5:52:27 PM (15 years ago)
Author:
commit-queue@webkit.org
Message:

[chromium] CCThreadProxy::finishAllRendering hangs if !visible
​https://bugs.webkit.org/show_bug.cgi?id=71920

Patch by Iain Merrick <​husky@google.com> on 2011-11-11
Reviewed by James Robinson.

Source/WebCore:

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

(WebCore::CCScheduler::setNeedsForcedRedraw):

  • platform/graphics/chromium/cc/CCScheduler.h:
  • platform/graphics/chromium/cc/CCSchedulerStateMachine.cpp:

(WebCore::CCSchedulerStateMachine::CCSchedulerStateMachine):
(WebCore::CCSchedulerStateMachine::nextAction):
(WebCore::CCSchedulerStateMachine::updateState):
(WebCore::CCSchedulerStateMachine::setNeedsForcedRedraw):

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

(WebCore::CCThreadProxy::requestReadbackOnImplThread):
(WebCore::CCThreadProxy::finishAllRenderingOnImplThread):

Source/WebKit/chromium:

  • tests/CCSchedulerStateMachineTest.cpp:

(WebCore::TEST):

Location:
trunk/Source
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r100052 r100054  
     12011-11-11  Iain Merrick  <husky@google.com>
     2
     3        [chromium] CCThreadProxy::finishAllRendering hangs if !visible
     4        https://bugs.webkit.org/show_bug.cgi?id=71920
     5
     6        Reviewed by James Robinson.
     7
     8        * platform/graphics/chromium/cc/CCScheduler.cpp:
     9        (WebCore::CCScheduler::setNeedsForcedRedraw):
     10        * platform/graphics/chromium/cc/CCScheduler.h:
     11        * platform/graphics/chromium/cc/CCSchedulerStateMachine.cpp:
     12        (WebCore::CCSchedulerStateMachine::CCSchedulerStateMachine):
     13        (WebCore::CCSchedulerStateMachine::nextAction):
     14        (WebCore::CCSchedulerStateMachine::updateState):
     15        (WebCore::CCSchedulerStateMachine::setNeedsForcedRedraw):
     16        * platform/graphics/chromium/cc/CCSchedulerStateMachine.h:
     17        * platform/graphics/chromium/cc/CCThreadProxy.cpp:
     18        (WebCore::CCThreadProxy::requestReadbackOnImplThread):
     19        (WebCore::CCThreadProxy::finishAllRenderingOnImplThread):
     20
    1212011-11-11  John Knottenbelt  <jknotten@chromium.org>
    222

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

    r99253 r100054  
    6666{
    6767    m_stateMachine.setNeedsRedraw();
     68    processScheduledActions();
     69}
     70
     71void CCScheduler::setNeedsForcedRedraw()
     72{
     73    m_stateMachine.setNeedsForcedRedraw();
    6874    processScheduledActions();
    6975}

  • trunk/Source/WebCore/platform/graphics/chromium/cc/CCScheduler.h

    r99253 r100054  
    6565    void setNeedsRedraw();
    6666
     67    // As setNeedsRedraw(), but ensures the draw will definitely happen even if we are not visible.
     68    void setNeedsForcedRedraw();
     69
    6770    void beginFrameComplete();
    6871

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

    r99100 r100054  
    3232    : m_commitState(COMMIT_STATE_IDLE)
    3333    , m_needsRedraw(false)
     34    , m_needsForcedRedraw(false)
    3435    , m_needsCommit(false)
    3536    , m_updateMoreResourcesPending(false)
    … …  
    3940CCSchedulerStateMachine::Action CCSchedulerStateMachine::nextAction() const
    4041{
     42    bool shouldDraw = (m_needsRedraw && m_insideVSync && m_visible) || m_needsForcedRedraw;
    4143    switch (m_commitState) {
    4244    case COMMIT_STATE_IDLE:
    43         if (m_needsRedraw && m_insideVSync && m_visible)
     45        if (shouldDraw)
    4446            return ACTION_DRAW;
    4547        if (m_needsCommit && m_visible)
    … …  
    4850
    4951    case COMMIT_STATE_FRAME_IN_PROGRESS:
    50         if (m_needsRedraw && m_insideVSync && m_visible)
     52        if (shouldDraw)
    5153            return ACTION_DRAW;
    5254        return ACTION_NONE;
    5355
    5456    case COMMIT_STATE_UPDATING_RESOURCES:
    55         if (m_needsRedraw && m_insideVSync && m_visible)
     57        if (shouldDraw)
    5658            return ACTION_DRAW;
    5759        if (!m_updateMoreResourcesPending)
    … …  
    6365
    6466    case COMMIT_STATE_WAITING_FOR_FIRST_DRAW:
    65         if (m_needsRedraw && m_insideVSync && m_visible)
     67        if (shouldDraw)
    6668            return ACTION_DRAW;
    6769        return ACTION_NONE;
    … …  
    98100    case ACTION_DRAW:
    99101        m_needsRedraw = false;
     102        m_needsForcedRedraw = false;
    100103        if (m_commitState == COMMIT_STATE_WAITING_FOR_FIRST_DRAW) {
    101104            ASSERT(m_needsCommit);
    … …  
    121124}
    122125
     126void CCSchedulerStateMachine::setNeedsForcedRedraw()
     127{
     128    m_needsForcedRedraw = true;
     129}
     130
    123131void CCSchedulerStateMachine::setNeedsCommit()
    124132{

  • trunk/Source/WebCore/platform/graphics/chromium/cc/CCSchedulerStateMachine.h

    r99100 r100054  
    7979    void setNeedsRedraw();
    8080
     81    // As setNeedsRedraw(), but ensures the draw will definitely happen even if
     82    // we are not visible.
     83    void setNeedsForcedRedraw();
     84
    8185    // Indicates that a new commit flow needs to be performed, either to pull
    8286    // updates from the main thread to the impl, or to push deltas from the impl
    … …  
    9094
    9195    // Call this only in response to receiving an ACTION_UPDATE_MORE_RESOURCES
    92     // from nextState. Indicatest that the specific update request completed.
     96    // from nextState. Indicates that the specific update request completed.
    9397    void beginUpdateMoreResourcesComplete(bool morePending);
    9498
    … …  
    97101
    98102    bool m_needsRedraw;
     103    bool m_needsForcedRedraw;
    99104    bool m_needsCommit;
    100105    bool m_updateMoreResourcesPending;

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

    r99942 r100054  
    129129    }
    130130    m_readbackRequestOnImplThread = request;
    131     m_schedulerOnImplThread->setNeedsRedraw();
     131    m_schedulerOnImplThread->setNeedsForcedRedraw();
    132132}
    133133
    … …  
    309309    ASSERT(isImplThread());
    310310    ASSERT(!m_finishAllRenderingCompletionEventOnImplThread);
    311     m_schedulerOnImplThread->setNeedsRedraw();
    312311    m_finishAllRenderingCompletionEventOnImplThread = completion;
     312
     313    m_schedulerOnImplThread->setNeedsForcedRedraw();
    313314}
    314315

  • trunk/Source/WebKit/chromium/ChangeLog

    r100047 r100054  
     12011-11-11  Iain Merrick  <husky@google.com>
     2
     3        [chromium] CCThreadProxy::finishAllRendering hangs if !visible
     4        https://bugs.webkit.org/show_bug.cgi?id=71920
     5
     6        Reviewed by James Robinson.
     7
     8        * tests/CCSchedulerStateMachineTest.cpp:
     9        (WebCore::TEST):
     10
    1112011-11-11  Antoine Labour  <piman@chromium.org>
    212

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

    r99100 r100054  
    5656    bool needsRedraw() const { return m_needsRedraw; }
    5757
     58    void setNeedsForcedRedraw(bool b) { m_needsForcedRedraw = b; }
     59    bool needsForcedRedraw() const { return m_needsForcedRedraw; }
     60
    5861    bool insideVSync() const { return m_insideVSync; }
    5962    bool visible() const { return m_visible; }
    … …  
    100103}
    101104
     105TEST(CCSchedulerStateMachineTest, TestSetForcedRedrawDoesNotSetsNormalRedraw)
     106{
     107    CCSchedulerStateMachine state;
     108    state.setNeedsForcedRedraw();
     109    EXPECT_FALSE(state.redrawPending());
     110}
     111
    102112TEST(CCSchedulerStateMachineTest, TestNextActionDrawsOnVSync)
    103113{
    104     // When not on vsync, don't draw.
     114    // When not on vsync, or on vsync but not visible, don't draw.
     115    size_t numCommitStates = sizeof(allCommitStates) / sizeof(CCSchedulerStateMachine::CommitState);
     116    for (size_t i = 0; i < numCommitStates; ++i) {
     117        for (unsigned j = 0; j < 2; ++j) {
     118            StateMachine state;
     119            state.setCommitState(allCommitStates[i]);
     120            if (!j) {
     121                state.setInsideVSync(true);
     122                state.setVisible(false);
     123            } else
     124                state.setInsideVSync(false);
     125
     126            // Case 1: needsCommit=false updateMoreResourcesPending=false.
     127            state.setNeedsCommit(false);
     128            state.setUpdateMoreResourcesPending(false);
     129            EXPECT_NE(CCSchedulerStateMachine::ACTION_DRAW, state.nextAction());
     130
     131            // Case 2: needsCommit=false updateMoreResourcesPending=true.
     132            state.setNeedsCommit(false);
     133            state.setUpdateMoreResourcesPending(true);
     134            EXPECT_NE(CCSchedulerStateMachine::ACTION_DRAW, state.nextAction());
     135
     136            // Case 3: needsCommit=true updateMoreResourcesPending=false.
     137            state.setNeedsCommit(true);
     138            state.setUpdateMoreResourcesPending(false);
     139            EXPECT_NE(CCSchedulerStateMachine::ACTION_DRAW, state.nextAction());
     140
     141            // Case 4: needsCommit=true updateMoreResourcesPending=true.
     142            state.setNeedsCommit(true);
     143            state.setUpdateMoreResourcesPending(true);
     144            EXPECT_NE(CCSchedulerStateMachine::ACTION_DRAW, state.nextAction());
     145        }
     146    }
     147
     148    // When on vsync, or not on vsync but needsForcedRedraw set, should always draw expect if you're ready to commit, in which case commit.
     149    for (size_t i = 0; i < numCommitStates; ++i) {
     150        for (unsigned j = 0; j < 2; ++j) {
     151            StateMachine state;
     152            state.setCommitState(allCommitStates[i]);
     153            if (!j) {
     154                state.setInsideVSync(true);
     155                state.setNeedsRedraw(true);
     156                state.setVisible(true);
     157            } else
     158                state.setNeedsForcedRedraw(true);
     159
     160            CCSchedulerStateMachine::Action expectedAction;
     161            if (allCommitStates[i] != CCSchedulerStateMachine::COMMIT_STATE_READY_TO_COMMIT)
     162                expectedAction = CCSchedulerStateMachine::ACTION_DRAW;
     163            else
     164                expectedAction = CCSchedulerStateMachine::ACTION_COMMIT;
     165
     166            // Case 1: needsCommit=false updateMoreResourcesPending=false.
     167            state.setNeedsCommit(false);
     168            state.setUpdateMoreResourcesPending(false);
     169            EXPECT_EQ(expectedAction, state.nextAction());
     170
     171            // Case 2: needsCommit=false updateMoreResourcesPending=true.
     172            state.setNeedsCommit(false);
     173            state.setUpdateMoreResourcesPending(true);
     174            EXPECT_EQ(expectedAction, state.nextAction());
     175
     176            // Case 3: needsCommit=true updateMoreResourcesPending=false.
     177            state.setNeedsCommit(true);
     178            state.setUpdateMoreResourcesPending(false);
     179            EXPECT_EQ(expectedAction, state.nextAction());
     180
     181            // Case 4: needsCommit=true updateMoreResourcesPending=true.
     182            state.setNeedsCommit(true);
     183            state.setUpdateMoreResourcesPending(true);
     184            EXPECT_EQ(expectedAction, state.nextAction());
     185        }
     186    }
     187}
     188
     189TEST(CCSchedulerStateMachineTest, TestNoCommitStatesRedrawWhenInvisible)
     190{
    105191    size_t numCommitStates = sizeof(allCommitStates) / sizeof(CCSchedulerStateMachine::CommitState);
    106192    for (size_t i = 0; i < numCommitStates; ++i) {
    107193        StateMachine state;
    108194        state.setCommitState(allCommitStates[i]);
     195        state.setVisible(false);
    109196        state.setNeedsRedraw(true);
    110         state.setVisible(true);
    111         state.setInsideVSync(false);
    112 
    113         // Case 1: needsCommit=false updateMoreResourcesPending=false.
    114         state.setNeedsCommit(false);
    115         state.setUpdateMoreResourcesPending(false);
    116         EXPECT_NE(CCSchedulerStateMachine::ACTION_DRAW, state.nextAction());
    117 
    118         // Case 2: needsCommit=false updateMoreResourcesPending=true.
    119         state.setNeedsCommit(false);
    120         state.setUpdateMoreResourcesPending(true);
    121         EXPECT_NE(CCSchedulerStateMachine::ACTION_DRAW, state.nextAction());
    122 
    123         // Case 3: needsCommit=true updateMoreResourcesPending=false.
    124         state.setNeedsCommit(true);
    125         state.setUpdateMoreResourcesPending(false);
    126         EXPECT_NE(CCSchedulerStateMachine::ACTION_DRAW, state.nextAction());
    127 
    128         // Case 4: needsCommit=true updateMoreResourcesPending=true.
    129         state.setNeedsCommit(true);
    130         state.setUpdateMoreResourcesPending(true);
    131         EXPECT_NE(CCSchedulerStateMachine::ACTION_DRAW, state.nextAction());
    132     }
    133 
    134     // When on vsync, you should always draw. Expect if you're ready to commit, in which case commit.
    135     for (size_t i = 0; i < numCommitStates; ++i) {
    136         StateMachine state;
    137         state.setCommitState(allCommitStates[i]);
    138         state.setNeedsRedraw(true);
    139         state.setVisible(true);
    140         state.setInsideVSync(true);
    141         CCSchedulerStateMachine::Action expectedAction;
    142         if (allCommitStates[i] != CCSchedulerStateMachine::COMMIT_STATE_READY_TO_COMMIT)
    143             expectedAction = CCSchedulerStateMachine::ACTION_DRAW;
    144         else
    145             expectedAction = CCSchedulerStateMachine::ACTION_COMMIT;
    146 
    147         // Case 1: needsCommit=false updateMoreResourcesPending=false.
    148         state.setNeedsCommit(false);
    149         state.setUpdateMoreResourcesPending(false);
    150         EXPECT_EQ(expectedAction, state.nextAction());
    151 
    152         // Case 2: needsCommit=false updateMoreResourcesPending=true.
    153         state.setNeedsCommit(false);
    154         state.setUpdateMoreResourcesPending(true);
    155         EXPECT_EQ(expectedAction, state.nextAction());
    156 
    157         // Case 3: needsCommit=true updateMoreResourcesPending=false.
    158         state.setNeedsCommit(true);
    159         state.setUpdateMoreResourcesPending(false);
    160         EXPECT_EQ(expectedAction, state.nextAction());
    161 
    162         // Case 4: needsCommit=true updateMoreResourcesPending=true.
    163         state.setNeedsCommit(true);
    164         state.setUpdateMoreResourcesPending(true);
    165         EXPECT_EQ(expectedAction, state.nextAction());
    166     }
    167 }
    168 
    169 TEST(CCSchedulerStateMachineTest, TestNoCommitStatesRedrawWhenInvisible)
    170 {
    171     // If not visible, never redraw.
    172     size_t numCommitStates = sizeof(allCommitStates) / sizeof(CCSchedulerStateMachine::CommitState);
    173     for (size_t i = 0; i < numCommitStates; ++i) {
    174         StateMachine state;
    175         state.setCommitState(allCommitStates[i]);
    176         state.setNeedsRedraw(true);
    177         state.setInsideVSync(false);
    178         state.setVisible(false);
    179 
    180         // Case 1: needsCommit=false updateMoreResourcesPending=false.
    181         state.setNeedsCommit(false);
    182         state.setUpdateMoreResourcesPending(false);
    183         EXPECT_NE(CCSchedulerStateMachine::ACTION_DRAW, state.nextAction());
    184 
    185         // Case 2: needsCommit=false updateMoreResourcesPending=true.
    186         state.setNeedsCommit(false);
    187         state.setUpdateMoreResourcesPending(true);
    188         EXPECT_NE(CCSchedulerStateMachine::ACTION_DRAW, state.nextAction());
    189 
    190         // Case 3: needsCommit=true updateMoreResourcesPending=false.
    191         state.setNeedsCommit(true);
    192         state.setUpdateMoreResourcesPending(false);
    193         EXPECT_NE(CCSchedulerStateMachine::ACTION_DRAW, state.nextAction());
    194 
    195         // Case 4: needsCommit=true updateMoreResourcesPending=true.
    196         state.setNeedsCommit(true);
    197         state.setUpdateMoreResourcesPending(true);
    198         EXPECT_NE(CCSchedulerStateMachine::ACTION_DRAW, state.nextAction());
     197        state.setNeedsForcedRedraw(false);
     198
     199        // There shouldn't be any drawing regardless of vsync.
     200        for (unsigned j = 0; j < 2; ++j) {
     201            state.setInsideVSync(j == 1);
     202
     203            // Case 1: needsCommit=false updateMoreResourcesPending=false.
     204            state.setNeedsCommit(false);
     205            state.setUpdateMoreResourcesPending(false);
     206            EXPECT_NE(CCSchedulerStateMachine::ACTION_DRAW, state.nextAction());
     207
     208            // Case 2: needsCommit=false updateMoreResourcesPending=true.
     209            state.setNeedsCommit(false);
     210            state.setUpdateMoreResourcesPending(true);
     211            EXPECT_NE(CCSchedulerStateMachine::ACTION_DRAW, state.nextAction());
     212
     213            // Case 3: needsCommit=true updateMoreResourcesPending=false.
     214            state.setNeedsCommit(true);
     215            state.setUpdateMoreResourcesPending(false);
     216            EXPECT_NE(CCSchedulerStateMachine::ACTION_DRAW, state.nextAction());
     217
     218            // Case 4: needsCommit=true updateMoreResourcesPending=true.
     219            state.setNeedsCommit(true);
     220            state.setUpdateMoreResourcesPending(true);
     221            EXPECT_NE(CCSchedulerStateMachine::ACTION_DRAW, state.nextAction());
     222        }
    199223    }
    200224}
    … …  
    217241    state.updateState(CCSchedulerStateMachine::ACTION_BEGIN_UPDATE_MORE_RESOURCES);
    218242
    219     // Veriify we don't do anything, both for vsync and not vsync.
     243    // Verify we don't do anything, both for vsync and not vsync.
    220244    state.setInsideVSync(false);
    221245    EXPECT_EQ(CCSchedulerStateMachine::ACTION_NONE, state.nextAction());
Note: See TracChangeset for help on using the changeset viewer.