[webkit-reviews] review granted: [Bug 202505] Element jumps to wrong position after perspective change on ancestor : [Attachment 384467] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 202505: Element jumps to wrong position after perspective change on
ancestor
https://bugs.webkit.org/show_bug.cgi?id=202505

Attachment 384467: Patch

https://bugs.webkit.org/attachment.cgi?id=384467&action=review




--- Comment #12 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 384467
  --> https://bugs.webkit.org/attachment.cgi?id=384467
Patch

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

> Source/WebCore/page/FrameView.cpp:812
> +void FrameView::styleDidChange()
> +{
> +    ScrollView::styleDidChange();
> +    RenderView* renderView = this->renderView();
> +    if (!renderView)
> +	   return;
> +
> +    RenderLayer* layerTreeMutationRoot =
renderView->takeStyleChangeLayerTreeMutationRoot();
> +    if (layerTreeMutationRoot && !needsLayout())
> +	   layerTreeMutationRoot->updateLayerPositionsAfterStyleChange();
> +}

Can this code go to RenderView::styleDidChange where this function is invoked?
You'll need to expose fewer functions (no
takeStyleChangeLayerTreeMutationRoot).

> Source/WebCore/platform/ScrollView.h:143
> +    virtual void styleDidChange();

...and you avoid making this virtual.

> Source/WebCore/rendering/RenderView.cpp:880
> +void RenderView::layerChildrenChangedDuringStyleChange(RenderLayer& layer)

It feels like there is a need for a new root type where this sort of stuff can
live.

> Source/WebCore/rendering/RenderView.cpp:891
> +RenderLayer* RenderView::takeStyleChangeLayerTreeMutationRoot()

Non-ownership transferring take() is bit surprising.