[Webkit-unassigned] [Bug 221339] [css-grid] Cache definite height detection

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=221339

--- Comment #4 from Javier Fernandez <jfernandez at igalia.com> ---
Comment on attachment 419167
  --> https://bugs.webkit.org/attachment.cgi?id=419167
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 ?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210215/bf75bac7/attachment.htm>