[webkit-reviews] review denied: [Bug 110815] [CSS Shaders] Parse the geometry descriptor : [Attachment 196201] Patch

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied Michelangelo De Simone
<michelangelo at webkit.org>'s request for review:
Bug 110815: [CSS Shaders] Parse the geometry descriptor
https://bugs.webkit.org/show_bug.cgi?id=110815

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

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=196201&action=review


I review the patch together with Max. The patch looks great already. Just some
small modifications. The tests are good.

> Source/WebCore/css/CSSParser.cpp:2887
> +	   return m_inFilterRule ? parseGeometry(propId, value, important) : 0;


s/0/false/

> Source/WebCore/css/CSSParser.cpp:8802
> +    // grid() function should have 1..3 arguments.

.. -> to

> Source/WebCore/css/CSSParser.cpp:8803
> +    unsigned size = value->function->args->size();

Can you add at least an assertion for args?

> Source/WebCore/css/CSSParser.cpp:8812
> +    bool allowFirstNumber = true;
> +    bool allowSecondNumber = false;
> +    bool allowAttachment = true;

How about breaking up the parsing of the mesh dimensions in a helper function
"parseMeshDimensions". That function will parse 1 or 2 numbers or return false.