Bug 221339 – [css-grid] Cache definite height detection

• Component: CSS
• Status: RESOLVED LATER

It might be worthy of storing the definite/indefinite height detection in a new attribute
in RenderGrid. By doing so, we only need to call RenderBlock::hasDefiniteLogicalHeight() once 
from RenderGrid::RenderBlock(). Then we can use the cached value in other places.

Correcting above comment: RenderGrid::RenderBlock() -> RenderGrid::layoutBlock()

Created attachment 419167 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/74191831>

[Javier Fernandez]

Comment on attachment 419167 [details]
Patch

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

The change looks good in general, but since it only makes sense to improve performance, I'd rather justify it with some data from the perf bots. I think we've recently added some changes in the height definiteness logic that could have impacted negatively, so we could monitor the perf tests we have since that change and evaluate the benefit of this patch.

> Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:1119
> +    const bool indefiniteHeight = m_direction == ForRows && !m_renderGrid->cachedHasDefiniteLogicalHeight();

Theoretically, it's possible to call the GridTrackSizingAlgorithm method of RenderGrid instance during the execution of its LayoutBlock method (eg, computing the grid's intrinsic size). If I understood correctly this patch, we are initializing the cache during the Layout, so it'd be possible that we call the cachedHasDefiniteLogicalHeight() when the cache Optional value is uninitialized. Shouldn't we add some defensive code to prevent this scenario ?