Bug 192529 – Add nodes to the scrolling tree in z-index order.

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Simon Fraser (smfr)]

Bug 176914 adds the ability to add nodes to the scrolling tree in the right z-order, but we actually need to make use of that.

[Simon Fraser (smfr)]

Marking fast/visual-viewport/tiled-drawing/zoomed-fixed-scrolling-layers-state.html as flakey until this is fixed; the current code attaches nodes to the scrolling tree by traversing a HashSet, which is not ordered.

[Radar WebKit Bug Importer]

<rdar://problem/47402708>

[Simon Fraser (smfr)]

Created attachment 360318 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 360322 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 360322 [details]
Patch

WIP

[Simon Fraser (smfr)]

Created attachment 360510 [details]
Patch

[EWS Watchlist]

Comment on attachment 360510 [details]
Patch

Attachment 360510 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10944721

New failing tests:
fast/scrolling/ios/hit-testing-iframe.html

[EWS Watchlist]

Created attachment 360530 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Dean Jackson]

Comment on attachment 360510 [details]
Patch

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

> Source/WebCore/ChangeLog:21
> +        was hard because the backing was already disconnect from its owning RenderLayer, so I added RenderLayerBacking::willBeDestroyed()

typo: disconnected

> Source/WebCore/rendering/RenderLayerCompositor.cpp:627
> +        return { };

Is this the new nullopt?

> Source/WebCore/rendering/RenderLayerCompositor.cpp:3773
> +    ASSERT_IMPLIES(nodeType == ScrollingNodeType::MainFrame, !treeState.parentNodeID.value());

oooh. I didn't know ASSERT_IMPLIES existed :|

> Source/WebCore/rendering/RenderLayerCompositor.cpp:3807
> +        // FIXME

oops!

> Source/WebCore/rendering/RenderLayerCompositor.cpp:3837
> +    bool isViewportConstained = roles.contains(ScrollCoordinationRole::ViewportConstrained);

I don't know what a con stain is.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:3890
> +        ASSERT(layer.renderer().isFixedPositioned());

huh. i assumed this wouldn't compile in release.

> LayoutTests/scrollingcoordinator/scrolling-tree/scrolling-tree-is-z-order.html:45
> +            z-index: 3;

It might be worth explaining in a comment (assuming I'm correct) that this z-index value causes the third Fixed Node in the results to be the one with the top-left-most layout position.

i.e. the result shows that the scrolling layers are #second, #third, #first, because z-index: 3, auto, 1.

Also, I guess you could make this value 2 :)

[Simon Fraser (smfr)]

https://trac.webkit.org/changeset/240698/webkit