Bug 231832

Summary: Make sure child layers of top layer elements are rendered and correctly z-ordered (top-layer-stacking.html fails)
Product: WebKit Reporter: Tim Nguyen (:ntim) <ntim>
Component: Layout and RenderingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, changseok, esprehn+autocc, ews-watchlist, fred.wang, glenn, graouts, kondapallykalyan, pdr, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 84635    
Attachments:
Description Flags
Patch
none
Patch none

Description Tim Nguyen (:ntim) 2021-10-15 14:21:35 PDT
I suspect something is wrong in RenderLayer::rebuildZOrderLists
Comment 1 Radar WebKit Bug Importer 2021-10-15 14:21:44 PDT
<rdar://problem/84316561>
Comment 2 Tim Nguyen (:ntim) 2021-10-15 17:20:03 PDT
Created attachment 441461 [details]
Patch
Comment 3 Simon Fraser (smfr) 2021-10-15 17:30:10 PDT
Comment on attachment 441461 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=441461&action=review

I think the real bug here is that RenderLayer::collectLayers() just early returns when establishesTopLayer() is true, so top layer layers never get any children.

> Source/WebCore/rendering/RenderLayer.cpp:768
> +    collectLayersForChildren(includeHiddenLayers, false, posZOrderList, negZOrderList, accumulatedDirtyFlags);

This is called the "boolean trap": a true/false argument that is hard to understand here at the call site. We generally avoid this by using enums.

> Source/WebCore/rendering/RenderLayer.cpp:803
> +                auto startOfSorting = posZOrderList->end();
> +                layer->collectLayersForChildren(includeHiddenLayers, true, posZOrderList, negZOrderList, accumulatedDirtyFlags);
> +                std::stable_sort(startOfSorting, posZOrderList->end(), compareZIndex);

I don't think this is right. This will append all descendant layers to 'posZOrderList' but what needs to happen is that 'layer' here, which is a stacking context, builds its own positive and negative z-order lists.

> Source/WebCore/rendering/RenderLayer.cpp:827
> +        auto& layerList = (zIndex() >= 0 || hasTopLayerAncestor) ? positiveZOrderList : negativeZOrderList;

Here all descendants of top layer layers append to the positiveZOrderList but this is wrong.
Comment 4 Simon Fraser (smfr) 2021-10-15 18:35:06 PDT
Created attachment 441466 [details]
Patch
Comment 5 EWS 2021-10-16 03:35:27 PDT
Committed r284314 (243110@main): <https://commits.webkit.org/243110@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 441466 [details].