[webkit-reviews] review denied: [Bug 70707] CSS Aspect Ratio Property Parsing Stage : [Attachment 113241] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has denied Fady Samuel <fsamuel at chromium.org>'s
request for review:
Bug 70707: CSS Aspect Ratio Property Parsing Stage
https://bugs.webkit.org/show_bug.cgi?id=70707

Attachment 113241: Patch
https://bugs.webkit.org/attachment.cgi?id=113241&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=113241&action=review


Just needs a little restructuring...I think it'll be fine to commit after that.


> Source/WebCore/rendering/style/StyleBoxData.h:86
> +    float m_aspectRatio;
> +
>      bool m_hasAutoZIndex : 1;
> +    bool m_hasAspectRatio : 1;

These should probably be on StyleRareNonInheritedData.h. See what the new
flexbox properties do as an example. As it is, every RenderStyle holds a
StyleBoxData, so this is adding a float to every RenderStyle. With
StyleRareNonInheritedData, it would only add the memory use if aspectRatio is
set.

It's safe to say that aspectRatio will be used rarely. Even on pages that use
it, they'll only use it on a small subset of the DOM.

> LayoutTests/fast/css/aspect-ratio-parsing-tests-expected.txt:6
> +PASS testParsing("aspectRatioTest", "2/1", "-webkit-aspect-ratio") is "2"

This serialization doesn't match http://www.xanthir.com/blog/b4810. It should
be "2/1".