If releaseContentsTextures gets called between CCThreadProxy::scheduledActionBeginFrame and CCThreadProxy::beginFrameCompleteOnImplThread, we evict all the resources, and set the needsCommit flag, but the main thread won't know about it and paint the tiles as if the resources were not deleted, and queues a bunch of updates. When beginFrameCompleteOnImplThread gets called, we execute those updates on deleted resources, causing ASSERTs in debug and very bad things in release. Also the needsCommit flag is essentially lost in the dust, meaning we won't repaint those missing textures. Quick solution: just delay the eviction until after the commit. However we may not want to delay the eviction in which case the proposed fix is: - if the m_contentsTexturesWerePurgedSinceLastCommit flag is set in beginFrameCompleteOnImplThread, drop the updates that touch any managed texture - keep track of whether we set m_contentsTexturesWerePurgedSinceLastCommit since scheduledActionBeginFrame, in which case we don't want to reset m_contentsTexturesWerePurgedSinceLastCommit in commitComplete, and post another setNeedsCommit then. The way I reproed this is by running chrome/aura with --force-compositing-mode --enable-threaded-compositing --enable-ui-release-front-surface, open 3 tabs and switch rapidly between them.
Created attachment 157850 [details] Patch
I'll write tests before landing, but please check the approach and/or if I'm missing something. My assumption if the frame is "aborted" (because we got invisible in the mean time) is that I can keep the state as is, and a commit will be kicked when the tab becomes visible again, causing a new beginFrame post that will validate the textures then.
Comment on attachment 157850 [details] Patch Can this get unit tested? This might fit with some of the CCLayerTreeHostTest tests that test other proxy interactions.
Yes, that's the plan (as I wrote in #2).
Comment on attachment 157850 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=157850&action=review > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.h:272 > + bool m_beginFramePosted; > + bool m_contentsTexturesWerePurgedSinceLastBeginFrame; > + bool m_contentsTexturesPurged; could you keep the more stateful bits up on CCThreadProxy? It's the class responsible for handling state transitions (which it mostly delegates to the scheduler) and the details of the commit flow, so I think it should keep track of what it has posted to the main thread and what it needs to preserve. Also, the single thread proxy doesn't have to worry about any of this stuff.
Created attachment 158195 [details] Patch
(In reply to comment #3) > (From update of attachment 157850 [details]) > Can this get unit tested? Done (In reply to comment #5) > (From update of attachment 157850 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=157850&action=review > > > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.h:272 > > + bool m_beginFramePosted; > > + bool m_contentsTexturesWerePurgedSinceLastBeginFrame; > > + bool m_contentsTexturesPurged; > > could you keep the more stateful bits up on CCThreadProxy? It's the class responsible for handling state transitions (which it mostly delegates to the scheduler) and the details of the commit flow, so I think it should keep track of what it has posted to the main thread and what it needs to preserve. Also, the single thread proxy doesn't have to worry about any of this stuff. Done.
Comment on attachment 158195 [details] Patch R=me - nice test!
Comment on attachment 158195 [details] Patch Clearing flags on attachment: 158195 Committed r125577: <http://trac.webkit.org/changeset/125577>
All reviewed patches have been landed. Closing bug.