Summary: | Add nodes to the scrolling tree in z-index order. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||||
Component: | Layout and Rendering | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | bfulgham, dino, ews-watchlist, fred.wang, simon.fraser, webkit-bug-importer, zalan | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2018-12-08 13:02:03 PST
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. Created attachment 360318 [details]
Patch
Created attachment 360322 [details]
Patch
Comment on attachment 360322 [details]
Patch
WIP
Created attachment 360510 [details]
Patch
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 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
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 :) |