Fix double comparisons for Media Queries

Fix double comparisons for Media Queries

Media queries that require double-based length comparisons weren't handled correctly.
As a result, some media queries weren't matching when they should have.

This CL fixes this issue by:
* Performing the length calculation in a more double-friendly way, by multiplying before dividing rather than the other way around.
* Apply a certain precision to length calculations, so that we could compare double equality.

BUG=604289
Committed: https://crrev.com/ee8bfd6290293a1b5dbf0813b6b3987f0e1ca545
Cr-Commit-Position: refs/heads/master@{#388464}

[Yoav Weiss, 2016-04-19 20:00:56]

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

[Yoav Weiss, 2016-04-19 20:00:56]

Hey Rune :)

I followed your advice in the related issue, and added precision based
comparison for lengths. Let me know what you think.

[Yoav Weiss, 2016-04-19 20:01:29]

On 2016/04/19 20:00:56, Yoav Weiss wrote:
> Hey Rune :)
> 
> I followed your advice in the related issue, and added precision based
> comparison for lengths. Let me know what you think.

s/and/as well as/ :)

[Yoav Weiss, 2016-04-19 20:05:16]

Description was changed from

==========
Fix float precision for Media Queries

Media queries that require float precision weren't handled correctly.
As a result, some media queries weren't matching when they should have.
This CL fixes this issue by:
* Perform the length calculation is a more float-friendly way, by multiplying
before dividing rather than the other way around.
* Apply a certain precision to length calculations, so that we could compare
floats.

BUG=604289
==========

to

==========
Fix float precision for Media Queries

Media queries that require float precision weren't handled correctly.
As a result, some media queries weren't matching when they should have.
This CL fixes this issue by:
* Performing the length calculation in a more float-friendly way, by multiplying
before dividing rather than the other way around.
* Apply a certain precision to length calculations, so that we could compare
float equality.

BUG=604289
==========

[Yoav Weiss, 2016-04-19 20:06:47]

Description was changed from

==========
Fix float precision for Media Queries

Media queries that require float precision weren't handled correctly.
As a result, some media queries weren't matching when they should have.
This CL fixes this issue by:
* Performing the length calculation in a more float-friendly way, by multiplying
before dividing rather than the other way around.
* Apply a certain precision to length calculations, so that we could compare
float equality.

BUG=604289
==========

to

==========
Fix double comparisons for Media Queries

Media queries that require double-based length comparisons weren't handled
correctly.
As a result, some media queries weren't matching when they should have.

This CL fixes this issue by:
* Performing the length calculation in a more double-friendly way, by
multiplying before dividing rather than the other way around.
* Apply a certain precision to length calculations, so that we could compare
double equality.

BUG=604289
==========

[rune, 2016-04-20 07:04:46]

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

https://codereview.chromium.org/1904523002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/MediaQueryEvaluator.cpp:178: const double
precision = 0.000001;
We use std::numeric_limits<T>::epsilon() other places. Would that be an
appropriate constant to use here?

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

https://codereview.chromium.org/1904523002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/MediaValues.cpp:171: nominator = 1;
Wouldn't it be simpler to just set "value" for each case in this switch and just
drop nominator/denominator?

And even return true instead of break and drop the code after the switch?

[Yoav Weiss, 2016-04-20 07:26:57]

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

https://codereview.chromium.org/1904523002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/MediaQueryEvaluator.cpp:178: const double
precision = 0.000001;
On 2016/04/20 07:04:46, rune wrote:
> We use std::numeric_limits<T>::epsilon() other places. Would that be an
> appropriate constant to use here?

yup

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

https://codereview.chromium.org/1904523002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/MediaValues.cpp:171: nominator = 1;
On 2016/04/20 07:04:46, rune wrote:
> Wouldn't it be simpler to just set "value" for each case in this switch and
just
> drop nominator/denominator?
> 
> And even return true instead of break and drop the code after the switch?

Yeah, changed it to suggested form

[rune, 2016-04-20 07:39:25]

https://codereview.chromium.org/1904523002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/MediaValues.cpp (left):

https://codereview.chromium.org/1904523002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/MediaValues.cpp:216: return true;
If you just remove NOTREACHED() and the return statement, the compiler will take
care of the NOTREACHED() part.

[Yoav Weiss, 2016-04-20 09:50:23]

https://codereview.chromium.org/1904523002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/MediaValues.cpp (left):

https://codereview.chromium.org/1904523002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/MediaValues.cpp:216: return true;
On 2016/04/20 07:39:25, rune wrote:
> If you just remove NOTREACHED() and the return statement, the compiler will
take
> care of the NOTREACHED() part.

done

[rune, 2016-04-20 10:55:06]

lgtm

[Yoav Weiss, 2016-04-20 10:56:38]

The CQ bit was checked by yoav@yoav.ws

[commit-bot: I haz the power, 2016-04-20 10:56:48]

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

[commit-bot: I haz the power, 2016-04-20 11:18:40]

Description was changed from

==========
Fix double comparisons for Media Queries

Media queries that require double-based length comparisons weren't handled
correctly.
As a result, some media queries weren't matching when they should have.

This CL fixes this issue by:
* Performing the length calculation in a more double-friendly way, by
multiplying before dividing rather than the other way around.
* Apply a certain precision to length calculations, so that we could compare
double equality.

BUG=604289
==========

to

==========
Fix double comparisons for Media Queries

Media queries that require double-based length comparisons weren't handled
correctly.
As a result, some media queries weren't matching when they should have.

This CL fixes this issue by:
* Performing the length calculation in a more double-friendly way, by
multiplying before dividing rather than the other way around.
* Apply a certain precision to length calculations, so that we could compare
double equality.

BUG=604289
==========

[commit-bot: I haz the power, 2016-04-20 11:18:41]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-04-22 19:22:22]

Description was changed from

==========
Fix double comparisons for Media Queries

Media queries that require double-based length comparisons weren't handled
correctly.
As a result, some media queries weren't matching when they should have.

This CL fixes this issue by:
* Performing the length calculation in a more double-friendly way, by
multiplying before dividing rather than the other way around.
* Apply a certain precision to length calculations, so that we could compare
double equality.

BUG=604289
==========

to

==========
Fix double comparisons for Media Queries

Media queries that require double-based length comparisons weren't handled
correctly.
As a result, some media queries weren't matching when they should have.

This CL fixes this issue by:
* Performing the length calculation in a more double-friendly way, by
multiplying before dividing rather than the other way around.
* Apply a certain precision to length calculations, so that we could compare
double equality.

BUG=604289

Committed: https://crrev.com/ee8bfd6290293a1b5dbf0813b6b3987f0e1ca545
Cr-Commit-Position: refs/heads/master@{#388464}
==========

[commit-bot: I haz the power, 2016-04-22 19:22:23]

Patchset 4 (id:??) landed as
https://crrev.com/ee8bfd6290293a1b5dbf0813b6b3987f0e1ca545
Cr-Commit-Position: refs/heads/master@{#388464}