The ASSERT in case ACTION_DRAW of CCSchedulerStateMachine::updateState(Action action) fails. A potential fix is to change the assert to ASSERT(m_needsCommit || !m_visible). The issue arises because updateState is being called right after m_needsCommit has been set to true and m_visible has been set to false. This will set m_commitState = COMMIT_STATE_WAITING_FOR_FIRST_DRAW in the ACTION_COMMIT case. Then on the draw action the assert is triggered and fails because it asserts only that m_needsCommit is true. Other solutions may be that m_needsCommit should not be set to true, m_visible may be incorrectly being set to false or m_needsForcedRedraw should not be set. The main question is what should the ASSERT statement be asserting in the ACTION_DRAW case of updateState.
Are you talking about this assertion: case ACTION_DRAW: m_needsRedraw = false; m_needsForcedRedraw = false; if (m_insideVSync) m_lastFrameNumberWhereDrawWasCalled = m_currentFrameNumber; if (m_commitState == COMMIT_STATE_WAITING_FOR_FIRST_DRAW) { ASSERT(m_needsCommit); <----here m_commitState = COMMIT_STATE_IDLE; } return; ? If so I'm not sure how we are hitting this in the scenario you describe - if m_needsCommit is true then that assertion shouldn't fire. Or are you referring to a different assertion?
Armand, are you planning on putting up your patch, or is your expectation that someone else fixes this?
Yes that is the failing ASSERT. I'm sorry, my initial report had a typo. The state we get into is one in which m_needsCommit is *false* and m_visible is *false*. In such a state, ACTION_COMMIT in updateState will set m_commitState = COMMIT_STATE_WAITING_FOR_FIRST_TO_DRAW (since !m_visible is true). Then, an ACTION_DRAW happens where m_commitState = COMMIT_STATE_WAITING_FOR_FIRST_TO_DRAW and m_needsCommit is false (and m_visible is false). Here the assertion fails. Nat: I was not planning on putting up my patch because I am not confident it is the correct solution. The patch I had was to change the assertion to ASSERT(m_needsCommit || !m_visible) in ACTION_DRAW. While this would solve this particular failure, I am not familiar enough with this code to know if this is correct. One question is, should ACTION_DRAW even be triggered when m_visible is false? If not, then the assertion in question is not the problem, but the problem is that ACTION_DRAW is being called even though m_visible is false.
I liked your original patch, I think you were correct. I suggest putting it up here. I'll double check it, but in general, I think its good.
Armand, I've tentatively assigned this bug to you as you have a proposed fix for the issue.
Great. I will submit my patch for this. I'll try to get to it soon (next couple days).
Created attachment 117716 [details] Text file of patch (ChangeLog and diff) First patch I am submitting. This is for the following bug: https://bugs.webkit.org/show_bug.cgi?id=73351 Please let me know if I took the correct steps for future patches I submit.
Comment on attachment 117716 [details] Text file of patch (ChangeLog and diff) View in context: https://bugs.webkit.org/attachment.cgi?id=117716&action=review > Source/WebCore/ChangeLog:10 > + Changed ASSERT in CCSchedulerStateMachine to include ( || !m_visible) as discussed in bug > + report. m_commitState is set to COMMIT_STATE_WAITING_FOR_FIRST_DRAW if m_needsCommit or > + !m_visible, so in ACTION_DRAW the assert should have both conditions. the intendentation here is off - see other entries in this file
(In reply to comment #8) > Created an attachment (id=117716) [details] > Text file of patch (ChangeLog and diff) > > First patch I am submitting. This is for the following bug: > https://bugs.webkit.org/show_bug.cgi?id=73351 > > Please let me know if I took the correct steps for future patches I submit. I left one comment inline. Also, new patches should have the "review?" flag set. "webkit-patch upload" should take care of that automatically.
Created attachment 117724 [details] Patch
Comment on attachment 117724 [details] Patch Great! That's perfect.
Comment on attachment 117724 [details] Patch Attachment 117724 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10695457 New failing tests: svg/custom/linking-uri-01-b.svg
Comment on attachment 117724 [details] Patch Flake, let's try again.
Comment on attachment 117724 [details] Patch Clearing flags on attachment: 117724 Committed r101911: <http://trac.webkit.org/changeset/101911>
All reviewed patches have been landed. Closing bug.