Use the return value to signal success for VTTParser helpers

Use the return value to signal success for VTTParser helpers

Convert functions on the form:

ValueType f(..., bool& success);

to the form:

bool f(..., ValueType& value);

Since the latter form tend to integrate more naturally in a
success/failure parsing scheme.
Make sure that the output value is not touched unless parsing was
successful.
Perform some (minor) simplifications due to this change.

Also simplify the check for a trailing '%' in parseFloatPercentageValue.

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

[jochen (gone - plz use gerrit), 2013-12-05 09:00:35]

https://codereview.chromium.org/104713002/diff/20001/Source/core/html/track/v...
File Source/core/html/track/vtt/VTTCue.cpp (right):

https://codereview.chromium.org/104713002/diff/20001/Source/core/html/track/v...
Source/core/html/track/vtt/VTTCue.cpp:679:
VTTParser::collectTimeStamp(timestamp, &position, currentTimestamp);
why not ASSERT() around this call?

[fs, 2013-12-05 09:29:14]

On 2013/12/05 09:00:35, jochen wrote:
>
https://codereview.chromium.org/104713002/diff/20001/Source/core/html/track/v...
> File Source/core/html/track/vtt/VTTCue.cpp (right):
> 
>
https://codereview.chromium.org/104713002/diff/20001/Source/core/html/track/v...
> Source/core/html/track/vtt/VTTCue.cpp:679:
> VTTParser::collectTimeStamp(timestamp, &position, currentTimestamp);
> why not ASSERT() around this call?

Is there a nice way to achieve that? I didn't consider something like:

#ifndef ASSERT_DISABLED
bool check =
#endif
collect...(...);
ASSERT(check);

worthwhile enough (considering that these Nodes are generated by the [VTT] tree
builder which only generate them if the same predicate is true.)

[jochen (gone - plz use gerrit), 2013-12-05 09:31:52]

On 2013/12/05 09:29:14, fs wrote:
> On 2013/12/05 09:00:35, jochen wrote:
> >
>
https://codereview.chromium.org/104713002/diff/20001/Source/core/html/track/v...
> > File Source/core/html/track/vtt/VTTCue.cpp (right):
> > 
> >
>
https://codereview.chromium.org/104713002/diff/20001/Source/core/html/track/v...
> > Source/core/html/track/vtt/VTTCue.cpp:679:
> > VTTParser::collectTimeStamp(timestamp, &position, currentTimestamp);
> > why not ASSERT() around this call?
> 
> Is there a nice way to achieve that? I didn't consider something like:
> 
> #ifndef ASSERT_DISABLED
> bool check =
> #endif
> collect...(...);
> ASSERT(check);
> 
> worthwhile enough (considering that these Nodes are generated by the [VTT]
tree
> builder which only generate them if the same predicate is true.)

What about

bool check = collect(...);
ASSERT_UNUSED(check, check);

[fs, 2013-12-05 09:42:42]

On 2013/12/05 09:31:52, jochen wrote:
> On 2013/12/05 09:29:14, fs wrote:
> > On 2013/12/05 09:00:35, jochen wrote:
> > >
> >
>
https://codereview.chromium.org/104713002/diff/20001/Source/core/html/track/v...
> > > File Source/core/html/track/vtt/VTTCue.cpp (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/104713002/diff/20001/Source/core/html/track/v...
> > > Source/core/html/track/vtt/VTTCue.cpp:679:
> > > VTTParser::collectTimeStamp(timestamp, &position, currentTimestamp);
> > > why not ASSERT() around this call?
> > 
> > Is there a nice way to achieve that? I didn't consider something like:
> > 
> > #ifndef ASSERT_DISABLED
> > bool check =
> > #endif
> > collect...(...);
> > ASSERT(check);
> > 
> > worthwhile enough (considering that these Nodes are generated by the [VTT]
> tree
> > builder which only generate them if the same predicate is true.)
> 
> What about
> 
> bool check = collect(...);
> ASSERT_UNUSED(check, check);

That looks like a good/nice way. Will do that.

[fs, 2013-12-05 09:44:32]

Now w/ ASSERT_UNUSED.

[jochen (gone - plz use gerrit), 2013-12-05 09:50:11]

lgtm

[commit-bot: I haz the power, 2013-12-05 09:51:08]

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

[commit-bot: I haz the power, 2013-12-05 10:51:00]

Change committed as 163248