[webkit-reviews] review granted: [Bug 204255] Block layout invalidation logic triggers excessive layout on height percentage descendants : [Attachment 383762] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted zalan
<zalan at apple.com>'s request for review:
Bug 204255: Block layout invalidation logic triggers excessive layout on height
percentage descendants
https://bugs.webkit.org/show_bug.cgi?id=204255

Attachment 383762: Patch

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




--- Comment #2 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 383762
  --> https://bugs.webkit.org/attachment.cgi?id=383762
Patch

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

> Source/WebCore/ChangeLog:4
> +	   https://bugs.webkit.org/show_bug.cgi?id=204255

Radar

> Source/WebCore/ChangeLog:14
> +	   This patch attempts to optimize the height percentage invalidation
logic by skipping boxes with out-of-flow ancestors.
> +	   The normal box invalidation logic works on parent-child level (a.k.a
when the parent is dirty it walks its direct children list and marks them dirty
if needed).
> +	   However percent height values require containing block-child type of
invalidation which atm we manage using a "percentHeightDescendantsMap".
> +	   percentHeightDescendantsMap contains all the height percentage
descendants and as part of the box invalidation the block container always
marks all of these special descendants dirty.
> +	   This leads to excessive and unnecessary layouts in certain cases
when the "walk the ancestor chain and mark boxes dirty all the way to the
RenderView" code ends up descending back
> +	   to these height percentage boxes.
> +	   We could optimize it slightly by checking if there's a height
percentage containing block in the ancestor chain. In such cases, we know that
the descendant's height used value computation will eventually stop at this
out-of-flow containing block.

Tidy this up a bit!

> Source/WebCore/rendering/RenderBlock.cpp:825
> +	   auto descendantNeedsLayout = true;

Nooooo

>
LayoutTests/fast/block/height-percentage-descendants-with-absolute-pos-containi
ngblock.html:11
> +    <img style="background-color: black; filter: blur(20px); height: 100%;"
src="broken.jpg">

Don't need the blur.