[webkit-reviews] review granted: [Bug 192529] Add nodes to the scrolling tree in z-index order. : [Attachment 360510] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jan 29 17:41:37 PST 2019

Dean Jackson <dino at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 192529: Add nodes to the scrolling tree in z-index order.

Attachment 360510: Patch


--- Comment #9 from Dean Jackson <dino at apple.com> ---
Comment on attachment 360510
  --> https://bugs.webkit.org/attachment.cgi?id=360510

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,

oooh. I didn't know ASSERT_IMPLIES existed :|

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


> Source/WebCore/rendering/RenderLayerCompositor.cpp:3837
> +    bool isViewportConstained =

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.

> +	       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 :)

More information about the webkit-reviews mailing list