Fix float parsing in srcset

Fix float parsing in srcset

The HTMLSrcsetParser was using a non-compliant float parser (the one used by
WTFString::charactersToFloat which is in wtf/dtoa/double-conversion).
As a result some tests were failing.
This CL switches it to use the decimal parser in HTMLParserIdioms.

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

[Yoav Weiss, 2015-08-25 09:24:25]

yoav@yoav.ws changed reviewers:
+ noel@chromium.org, tkent@chromium.org

[tkent, 2015-08-25 09:34:46]

The CQ bit was checked by tkent@chromium.org

[tkent, 2015-08-25 09:34:46]

lgtm

[commit-bot: I haz the power, 2015-08-25 09:35:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1306743003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1306743003/1

[Yoav Weiss, 2015-08-25 09:37:18]

On 2015/08/25 09:34:46, tkent wrote:
> lgtm

Thanks! :) I've updated the CL to properly clamp the double to a float

[tkent, 2015-08-25 10:02:47]

https://codereview.chromium.org/1306743003/diff/20001/Source/core/html/parser...
File Source/core/html/parser/HTMLSrcsetParser.cpp (right):

https://codereview.chromium.org/1306743003/diff/20001/Source/core/html/parser...
Source/core/html/parser/HTMLSrcsetParser.cpp:104: return
clampTo<float>(result.toDouble());
This looks to have a behavior change for numbers greater than the float limit.
Do we have a test?

[Yoav Weiss, 2015-08-25 10:07:58]

https://codereview.chromium.org/1306743003/diff/20001/Source/core/html/parser...
File Source/core/html/parser/HTMLSrcsetParser.cpp (right):

https://codereview.chromium.org/1306743003/diff/20001/Source/core/html/parser...
Source/core/html/parser/HTMLSrcsetParser.cpp:104: return
clampTo<float>(result.toDouble());
On 2015/08/25 10:02:46, tkent wrote:
> This looks to have a behavior change for numbers greater than the float limit.
> Do we have a test?

I didn't mean for that to be a behavior change. Is there a difference between
casting a double to a float (which was the previous behavior) and clampTo?
Should this be a static_case instead?

[tkent, 2015-08-25 10:14:23]

https://codereview.chromium.org/1306743003/diff/20001/Source/core/html/parser...
File Source/core/html/parser/HTMLSrcsetParser.cpp (right):

https://codereview.chromium.org/1306743003/diff/20001/Source/core/html/parser...
Source/core/html/parser/HTMLSrcsetParser.cpp:104: return
clampTo<float>(result.toDouble());
On 2015/08/25 10:07:58, Yoav Weiss wrote:
> On 2015/08/25 10:02:46, tkent wrote:
> > This looks to have a behavior change for numbers greater than the float
limit.
> > Do we have a test?
> 
> I didn't mean for that to be a behavior change. Is there a difference between
> casting a double to a float (which was the previous behavior) and clampTo?
> Should this be a static_case instead?

AFAIK, static_cast<float>(greater-than-float-max) returns Infinity, and
clampTo<float>() returns the maximum value of float.

[Yoav Weiss, 2015-08-25 11:05:05]

On 2015/08/25 10:14:23, tkent wrote:
>
https://codereview.chromium.org/1306743003/diff/20001/Source/core/html/parser...
> File Source/core/html/parser/HTMLSrcsetParser.cpp (right):
> 
>
https://codereview.chromium.org/1306743003/diff/20001/Source/core/html/parser...
> Source/core/html/parser/HTMLSrcsetParser.cpp:104: return
> clampTo<float>(result.toDouble());
> On 2015/08/25 10:07:58, Yoav Weiss wrote:
> > On 2015/08/25 10:02:46, tkent wrote:
> > > This looks to have a behavior change for numbers greater than the float
> limit.
> > > Do we have a test?
> > 
> > I didn't mean for that to be a behavior change. Is there a difference
between
> > casting a double to a float (which was the previous behavior) and clampTo?
> > Should this be a static_case instead?
> 
> AFAIK, static_cast<float>(greater-than-float-max) returns Infinity, and
> clampTo<float>() returns the maximum value of float.

in that case, static_cast is the way to go. Thanks for catching that :)

[tkent, 2015-08-25 11:05:56]

lgtm

[Yoav Weiss, 2015-08-25 11:08:45]

The CQ bit was checked by yoav@yoav.ws

[commit-bot: I haz the power, 2015-08-25 11:08:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1306743003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1306743003/40001

[commit-bot: I haz the power, 2015-08-25 12:05:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-25 12:05:22]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Yoav Weiss, 2015-08-25 12:13:44]

The CQ bit was checked by yoav@yoav.ws

[commit-bot: I haz the power, 2015-08-25 12:13:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1306743003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1306743003/40001

[Noel Gordon, 2015-08-25 12:51:00]

LGTM

[commit-bot: I haz the power, 2015-08-25 13:01:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-25 13:01:06]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tkent, 2015-08-25 13:22:05]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2015-08-25 13:22:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1306743003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1306743003/40001

[commit-bot: I haz the power, 2015-08-25 14:50:52]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201132