Rework float value/percentage scanning for VTTRegion

Rework float value/percentage scanning for VTTRegion

Next step in the transition to VTTScanner for VTTRegion parsing.
Add a new method scanFloat(...) to VTTScanner (basing the implementation
on VTTParser::parseFloatPercentageValue).

Use this new method scanFloat in VTTParser::parseFloatPercentageValue,
and change (remove) the end-of-input/value requirement (hoisting it into
the callers).

Let the above propagate into VTTParser::parseFloatPercentageValuePair,
and make it use VTTScanner internally.

BUG=305317
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=165442

[jochen (gone - plz use gerrit), 2014-01-20 19:22:26]

and... I defer to Mike! :)

[Mike West, 2014-01-21 07:59:23]

Two questions, looks fine otherwise.

https://codereview.chromium.org/139343006/diff/1/Source/core/html/track/vtt/V...
File Source/core/html/track/vtt/VTTScanner.cpp (right):

https://codereview.chromium.org/139343006/diff/1/Source/core/html/track/vtt/V...
Source/core/html/track/vtt/VTTScanner.cpp:156: return false;
Here you return false without setting `number`. At the bottom, you set invalid
numbers to float::max() and return true. Is that intentional?

https://codereview.chromium.org/139343006/diff/1/Source/core/html/track/vtt/V...
File Source/core/html/track/vtt/VTTScanner.h (right):

https://codereview.chromium.org/139343006/diff/1/Source/core/html/track/vtt/V...
Source/core/html/track/vtt/VTTScanner.h:129: // Scan a floating point value on
one of the forms: \d+\.? \d+\.\d+ \.\d+
Is "0." a valid floating point value? It doesn't seem like it should be, but
maybe there's a good reason for that strangeness?

[fs, 2014-01-21 08:19:57]

https://codereview.chromium.org/139343006/diff/1/Source/core/html/track/vtt/V...
File Source/core/html/track/vtt/VTTScanner.cpp (right):

https://codereview.chromium.org/139343006/diff/1/Source/core/html/track/vtt/V...
Source/core/html/track/vtt/VTTScanner.cpp:156: return false;
On 2014/01/21 07:59:24, Mike West wrote:
> Here you return false without setting `number`. At the bottom, you set invalid
> numbers to float::max() and return true. Is that intentional?

Intentional, yes. At this point, we have not managed to scan a valid float, so
we leave |number| as is and return false to signal failure (and don't consume
input).

Below however, we've verified that the input is wellformed - charactersToFloat
does that too, but has a differing definition of wellformed. Hence we interpret
|validNumber| - or rather, it's negation - as meaning that the number was too
large. We still return true though because the input was wellformed and we did
consume it (and |number| is set to the "best approximation" of the value). (If
we needed to detect the overflow as well, then we'd probably return some other
value for !validNumber.)

https://codereview.chromium.org/139343006/diff/1/Source/core/html/track/vtt/V...
File Source/core/html/track/vtt/VTTScanner.h (right):

https://codereview.chromium.org/139343006/diff/1/Source/core/html/track/vtt/V...
Source/core/html/track/vtt/VTTScanner.h:129: // Scan a floating point value on
one of the forms: \d+\.? \d+\.\d+ \.\d+
On 2014/01/21 07:59:24, Mike West wrote:
> Is "0." a valid floating point value? It doesn't seem like it should be, but
> maybe there's a good reason for that strangeness?

It is a valid floating point value. (Compare to other definitions - like C++
[2.13.3]).

[Mike West, 2014-01-21 08:23:37]

Sold. LGTM, thanks.

On 2014/01/21 08:19:57, fs wrote:
>
https://codereview.chromium.org/139343006/diff/1/Source/core/html/track/vtt/V...
> File Source/core/html/track/vtt/VTTScanner.cpp (right):
> 
>
https://codereview.chromium.org/139343006/diff/1/Source/core/html/track/vtt/V...
> Source/core/html/track/vtt/VTTScanner.cpp:156: return false;
> On 2014/01/21 07:59:24, Mike West wrote:
> > Here you return false without setting `number`. At the bottom, you set
invalid
> > numbers to float::max() and return true. Is that intentional?
> 
> Intentional, yes. At this point, we have not managed to scan a valid float, so
> we leave |number| as is and return false to signal failure (and don't consume
> input).
> 
> Below however, we've verified that the input is wellformed - charactersToFloat
> does that too, but has a differing definition of wellformed. Hence we
interpret
> |validNumber| - or rather, it's negation - as meaning that the number was too
> large. We still return true though because the input was wellformed and we did
> consume it (and |number| is set to the "best approximation" of the value). (If
> we needed to detect the overflow as well, then we'd probably return some other
> value for !validNumber.)
> 
>
https://codereview.chromium.org/139343006/diff/1/Source/core/html/track/vtt/V...
> File Source/core/html/track/vtt/VTTScanner.h (right):
> 
>
https://codereview.chromium.org/139343006/diff/1/Source/core/html/track/vtt/V...
> Source/core/html/track/vtt/VTTScanner.h:129: // Scan a floating point value on
> one of the forms: \d+\.? \d+\.\d+ \.\d+
> On 2014/01/21 07:59:24, Mike West wrote:
> > Is "0." a valid floating point value? It doesn't seem like it should be, but
> > maybe there's a good reason for that strangeness?
> 
> It is a valid floating point value. (Compare to other definitions - like C++
> [2.13.3]).

[commit-bot: I haz the power, 2014-01-21 08:29:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fs@opera.com/139343006/1

[commit-bot: I haz the power, 2014-01-21 08:40:19]

Change committed as 165442