Bug 221252

Summary:

Assertion failure in Debug build blocks webpage rendering

Product:

WebKit

Reporter:

Eleni Maria Stea <estea>

Component:

Layout and Rendering

Assignee:

Nobody <webkit-unassigned>

Status:

NEW

Severity:

Minor

CC:

bfulgham, estea, simon.fraser, webkit-bug-importer, zalan

Priority:

Keywords:

InRadar

Version:

WebKit Local Build

Hardware:

OS:

Linux

See Also:

https://bugs.webkit.org/show_bug.cgi?id=191962
https://bugs.webkit.org/show_bug.cgi?id=166008

Attachments:

Description	Flags
proposed patch	simon.fraser: review-

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-	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

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

<rdar://problem/74138081>

Note You need to log in before you can comment on or make changes to this bug.