Support aspect-ratio on flexbox children.
Created attachment 415748 [details] Patch
Created attachment 415761 [details] Patch
<rdar://problem/72382311>
Created attachment 417080 [details] Patch
Created attachment 417261 [details] Patch
Created attachment 417841 [details] Patch
Created attachment 417966 [details] Patch
Created attachment 417971 [details] Patch
Created attachment 418553 [details] Patch
Created attachment 418734 [details] Patch
Created attachment 420005 [details] Patch
Created attachment 420456 [details] Patch
Created attachment 420624 [details] Patch
Created attachment 420628 [details] Patch
Created attachment 420814 [details] Patch
Created attachment 420937 [details] Patch
Comment on attachment 420937 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420937&action=review Looking great, just one question > Source/WebCore/rendering/RenderBox.cpp:682 > + if (logicalMinHeight.isAuto() && shouldComputeLogicalHeightFromAspectRatio() && intrinsicContentHeight && intrinsicContentHeight != LayoutUnit::max() && styleToUse.overflowBlockDirection() == Overflow::Visible) In which cases intrinsicContentHeight is max() ? Shouldn't it just use nullopt to mean an indefinite size? Looks like we're kind of duplicating checks
Comment on attachment 420937 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420937&action=review >> Source/WebCore/rendering/RenderBox.cpp:682 >> + if (logicalMinHeight.isAuto() && shouldComputeLogicalHeightFromAspectRatio() && intrinsicContentHeight && intrinsicContentHeight != LayoutUnit::max() && styleToUse.overflowBlockDirection() == Overflow::Visible) > > In which cases intrinsicContentHeight is max() ? Shouldn't it just use nullopt to mean an indefinite size? Looks like we're kind of duplicating checks This is initiated by RenderFlexibleBox::layoutFlexItems calling mainAxisContentExtent(LayoutUnit::max()). You have a great point however looking at the code I think it will be difficult to introduce Optional here.
Created attachment 420958 [details] Patch
Comment on attachment 420958 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420958&action=review Awesome addition! Just one nit. > Source/WebCore/rendering/RenderBox.cpp:2947 > + if (computedValues.m_extent != LayoutUnit::max()) Mind adding a comment here explaining that using max() is the way callers have to tell this method to use a nullopt intrinsicHeight? Maybe even pointing to the RenderFlexibleBox code. Code should be self-explanatory but in this case the context is not enough and I think it pays off to add a comment and ease the job of future reviewers :) > Source/WebCore/rendering/RenderFlexibleBox.cpp:550 > +} I guess it's OK to have this here now. I hope we could eventually add the style().hasAspectRatio() check to the RenderBox::hasAspectRatio() in the future, but that's a far more intrusive change indeed.
Created attachment 420979 [details] Patch
Committed r273193: <https://commits.webkit.org/r273193> All reviewed patches have been landed. Closing bug and clearing flags on attachment 420979 [details].