avoid unnecessary layouts of flex items during the flex pass

• Component: New Bugs
• Status: RESOLVED FIXED

[Tony Chang]

avoid unnecessary layouts of flex items during the flex pass

[Tony Chang]

Created attachment 111862 [details]
Patch

[Ojan Vafai]

Comment on attachment 111862 [details]
Patch

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

Patch looks good to me.

> Source/WebCore/rendering/RenderFlexibleBox.cpp:496
> +    Length fixedWidth = isHorizontalFlow() ? child->style()->width() : child->style()->height();

Nit: I think the code would be simpler if you added flowAwareWidthStyleForChild instead of isFlowAwareLogicalWidthAutoForChild.

[Dave Hyatt]

Comment on attachment 111862 [details]
Patch

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

> Source/WebCore/rendering/RenderFlexibleBox.cpp:224
> +    Length width = isHorizontalFlow() ? child->style()->width() : child->style()->height();
> +    return width.isAuto();

Please name the local variable logicalWidth and not just width.

> Source/WebCore/rendering/RenderFlexibleBox.cpp:497
> +    return fixedWidth.calcMinValue(flowAwareContentLogicalWidth());

Adding flowAwareLogicalWidthLengthForChild as Ojan suggests seems cleaner.

[Tony Chang]

Created attachment 112368 [details]
Patch

[Ojan Vafai]

Comment on attachment 112368 [details]
Patch

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

> Source/WebCore/rendering/RenderFlexibleBox.cpp:506
> +    if (flowAwareLogicalWidthLengthForChild(child).isAuto()) {
>          LayoutUnit logicalWidth = hasOrthogonalFlow(child) ? child->logicalHeight() : child->maxPreferredLogicalWidth();
>          return logicalWidth - logicalBorderAndPaddingWidthForChild(child) - logicalScrollbarHeightForChild(child);
>      }
> -    return isHorizontalFlow() ? child->contentWidth() : child->contentHeight();
> +    Length fixedWidth = isHorizontalFlow() ? child->style()->width() : child->style()->height();
> +    return fixedWidth.calcMinValue(flowAwareContentLogicalWidth());

nit: i would do this like so:
Length logicalWidthLength = flowAwareLogicalWidthLengthForChild(child);
if (logicalWidthLength.isAuto()) {
...
}
return logicalWidthLength.calcMinValue(...);

[Tony Chang]

Created attachment 112372 [details]
Patch for landing

[Tony Chang]

Committed r98373: <http://trac.webkit.org/changeset/98373>