NEW 221252
Assertion failure in Debug build blocks webpage rendering
https://bugs.webkit.org/show_bug.cgi?id=221252
Summary Assertion failure in Debug build blocks webpage rendering
Eleni Maria Stea
Reported 2021-02-02 04:46:55 PST
Overview: Assertion failure from: ASSERT(willBeComposited == needsToBeComposited(layer, queryData)); in rendering/RenderLayerCompositor.cpp stops the rendering of some webpages in MiniBrowser in Debug builds. The reason seems to be that the layers are asynchronously updated since the time willBeComposited is set and needsToBeComposited is called, and so events like scrolling overflow can cause assertion failure. The assertion should be moved right after willBeComposited is set. Steps to reproduce: Build WPE/Webkit in Debug mode and run MiniBrowser with https://www.igalia.com. Results: An empty page will be displayed. Expected Result: The webpage to be displayed. Proposed Fix: If the assertion is moved right after: if (layer.needsPostLayoutCompositingUpdate() || compositingState.fullPaintOrderTraversalRequired || compositingState.descendantsRequireCompositingUpdate) { layer.setIndirectCompositingReason(IndirectCompositingReason::None); willBeComposited = needsToBeComposited(layer, queryData); } the page will appear fine in MiniBrowser. Additional Information: Bug was spotted on Linux/X11 using nested Weston and running MiniBrowser like this: ./run-wpe-x11 <path_to_minibrowser_installdir>MiniBrowser https://www.igalia.com
Attachments
proposed patch (2.01 KB, patch)
2021-02-02 05:03 PST, Eleni Maria Stea
simon.fraser: review-
Eleni Maria Stea
Comment 1 2021-02-02 05:03:54 PST
Created attachment 418983 [details] proposed patch Proposed patch: move the assertion before all other checks that might change the needsToBeComposited(layer, queryData) value.
Simon Fraser (smfr)
Comment 2 2021-02-02 09:58:50 PST
Comment on attachment 418983 [details] proposed patch The whole point of this assertion is to check that willBeComposited has the correct value.
Eleni Maria Stea
Comment 3 2021-02-02 14:23:07 PST
(In reply to Simon Fraser (smfr) from comment #2) > Comment on attachment 418983 [details] > proposed patch > > The whole point of this assertion is to check that willBeComposited has the > correct value. Thank you for your reply, What I tried to prevent by moving the assertion earlier was the case that needsToBeComposited() is different from willBeComposited value without that being a bug. I noticed that willBeComposited and needsToBeComposited have the same value at the beginning of the function but further down there are cases that change the value of the variable and so, the same check fails. I thought that the willBeComposited changes were correctly made and so I assumed that the problem is that the assertion failure takes place too late after the changes. My description was not so clear, sorry. What are your thoughts on this? Thanks in advance!
Simon Fraser (smfr)
Comment 4 2021-02-02 14:37:38 PST
If this assertion fires, it means that some state changed that affected the answer to needsToBeComposited() inside this function, which implies there's some side-effect. If willBeComposited does not match needsToBeComposited() then the behavior is wrong. You should go looking for why needsToBeComposited() changes its answer unexpectedly.
Eleni Maria Stea
Comment 5 2021-02-02 15:05:34 PST
(In reply to Simon Fraser (smfr) from comment #4) > If this assertion fires, it means that some state changed that affected the > answer to needsToBeComposited() inside this function, which implies there's > some side-effect. If willBeComposited does not match needsToBeComposited() > then the behavior is wrong. > > You should go looking for why needsToBeComposited() changes its answer > unexpectedly. Alright, thank you very much! needsToBeComposited() was true because compositing was required for overflow scrolling (although I saw the assertion failure in some other cases too). I hadn't realized this is unexpected, I will investigate it further.
Radar WebKit Bug Importer
Comment 6 2021-02-09 04:47:54 PST
Note You need to log in before you can comment on or make changes to this bug.