Bug 221252 – Assertion failure in Debug build blocks webpage rendering

• Component: Layout and Rendering
• Status: NEW

[Eleni Maria Stea]

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

[Eleni Maria Stea]

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 on attachment 418983 [details]
proposed patch

The whole point of this assertion is to check that willBeComposited has the correct value.

[Eleni Maria Stea]

(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)]

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]

(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]

<rdar://problem/74138081>