Clamp CSS numbers to float range at parsing.

Clamp CSS numbers to float range at parsing.

The CSS parser previously did late checks that all number-type tokens
were in the range of a float, and treated them as invalid if not.
Instead, this CL clamps CSS number-type values to float range at parse-
time, and removes the now-extraneous checks.

This should be faster that iterating over already-parsed tokens,
and treats a wider range of values as valid (anything that successfully
parses as a double is now valid).

[Tab Atkins, 2016-04-22 20:57:39]

Description was changed from

==========
Clamp CSS numbers to float range at parsing.

The CSS parser previously did late checks that all number-type tokens
were in the range of a float, and treated them as invalid if not.
Instead, this CL clamps CSS number-type values to float range at parse-
time, and removes the now-extraneous checks.

This should be faster that iterating over already-parsed tokens,
and treats a wider range of values as valid (anything that successfully
parses as a double is now valid).
==========

to

==========
Clamp CSS numbers to float range at parsing.

The CSS parser previously did late checks that all number-type tokens
were in the range of a float, and treated them as invalid if not.
Instead, this CL clamps CSS number-type values to float range at parse-
time, and removes the now-extraneous checks.

This should be faster that iterating over already-parsed tokens,
and treats a wider range of values as valid (anything that successfully
parses as a double is now valid).
==========

[Tab Atkins, 2016-04-22 20:57:39]

tabatkins@chromium.org changed reviewers:
+ alancutter@chromium.org, timloh@chromium.org

[Tab Atkins, 2016-04-22 21:07:14]

While I'm here in this code, I'm not 100% certain why we were limiting it to
float ranges in the first place. m_numericValue is a double, so is there a good
reason to not just let CSS numerics fill out the double range?  They get clamped
to much smaller ranges in practice anyway.

[rune, 2016-04-25 12:24:28]

rune@opera.com changed reviewers:
+ rune@opera.com

[rune, 2016-04-25 12:24:29]

This CL needs tests. timloh@ should have a look as well.

https://codereview.chromium.org/1914693002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/parser/CSSParserFastPaths.cpp (right):

https://codereview.chromium.org/1914693002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/parser/CSSParserFastPaths.cpp:86: if
(!std::isfinite(number)) {
Should this be std::isnan instead?

https://codereview.chromium.org/1914693002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/parser/CSSParserToken.cpp (right):

https://codereview.chromium.org/1914693002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/parser/CSSParserToken.cpp:46: m_numericValue
= 0.0;
Shouldn't +/-inf be clamped too?

Why is 0 a good choice here?

[Timothy Loh, 2016-04-28 08:11:50]

On 2016/04/22 21:07:14, Tab Atkins wrote:
> While I'm here in this code, I'm not 100% certain why we were limiting it to
> float ranges in the first place. m_numericValue is a double, so is there a
good
> reason to not just let CSS numerics fill out the double range?  They get
clamped
> to much smaller ranges in practice anyway.

IIRC because clusterfuzz kept finding different places where they didn't
actually get clamped, so we put this in to make clusterfuzz be quiet (not
clamping usually means we'd end up with Infinite or NaN in random places and hit
asserts). The checks here are not exactly comprehensive of course as you can
probably hit the same asserts with calc(big * big * big).

[rune, 2016-06-28 10:52:34]

This stalled. Tab, do you want to follow up, or should someone take over?

[rwlbuis, 2016-06-29 14:25:08]

On 2016/06/28 10:52:34, rune wrote:
> This stalled. Tab, do you want to follow up, or should someone take over?

I am willing to work on this, as I already kind of touching this area:
https://codereview.chromium.org/2056593005/

[rwlbuis, 2016-08-15 15:09:20]

On 2016/06/29 14:25:08, rwlbuis wrote:
> On 2016/06/28 10:52:34, rune wrote:
> > This stalled. Tab, do you want to follow up, or should someone take over?
> 
> I am willing to work on this, as I already kind of touching this area:
> https://codereview.chromium.org/2056593005/

I think this CL can be closed now since the comparable patch went in:
https://codereview.chromium.org/2115923002/