Skip to content

GitLab

T
tangled
Commit cf115265 authored by rune's avatar rune Committed by Commit bot
Browse files
Options

Better overflow handling for aspect-ratio MQ. 

This change fixes two issues:

1. Clamp instead of casting double values from parser to internal
   unsigned storage.
2. Promote width/height/numerator/denominator multiplications to double
   to avoid integer overflow for large numerator/denominators.

R=yoav@yoav.ws,suzyh@chromium.org
BUG=676709,714079

Review-Url: https://codereview.chromium.org/2836613002
Cr-Commit-Position: refs/heads/master@{#467615}
parent 44062958
Hide whitespace changes
Inline Side-by-side
Showing with 27 additions and 6 deletions
third_party/WebKit/LayoutTests/fast/media/mq-aspect-ratio-overflow.html 0 → 100644
View file @ cf115265
<!DOCTYPE html>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<style>
@media (max-aspect-ratio: 4294967295/1) { #t1 { color:green } }
@media (max-aspect-ratio: 4294967296/1) { #t2 { color:green } }
</style>
<div id="t1">This text should be green.</div>
<div id="t2">This text should be green.</div>
<script>
test(() => {
assert_equals(getComputedStyle(t1).color, "rgb(0, 128, 0)", "#t1 should be green.");
}, "Aspect ratio with numerator = 2^32-1.");
test(() => {
assert_equals(getComputedStyle(t2).color, "rgb(0, 128, 0)", "#t2 should be green.");
}, "Aspect ratio with numerator = 2^32.");
</script>
third_party/WebKit/Source/core/css/MediaQueryEvaluator.cpp
View file @ cf115265
...@@ -192,10 +192,10 @@ static bool CompareAspectRatioValue(const MediaQueryExpValue& value, ...@@ -192,10 +192,10 @@ static bool CompareAspectRatioValue(const MediaQueryExpValue& value,
int width, int width,
int height, int height,
MediaFeaturePrefix op) { MediaFeaturePrefix op) {
if (value.is_ratio) if (value.is_ratio) {
return CompareValue(width * static_cast<int>(value.denominator), return CompareValue(static_cast<double>(width) * value.denominator,
height * static_cast<int>(value.numerator), op); static_cast<double>(height) * value.numerator, op);
}
return false; return false;
} }
......
third_party/WebKit/Source/core/css/MediaQueryEvaluatorTest.cpp
View file @ cf115265
...@@ -66,6 +66,8 @@ TestCase g_screen_test_cases[] = { ...@@ -66,6 +66,8 @@ TestCase g_screen_test_cases[] = {
{"(display-mode: @junk browser)", 0}, {"(display-mode: @junk browser)", 0},
{"(shape: rect)", 1}, {"(shape: rect)", 1},
{"(shape: round)", 0}, {"(shape: round)", 0},
{"(max-device-aspect-ratio: 4294967295/1)", 1},
{"(min-device-aspect-ratio: 1/4294967296)", 1},
{0, 0} // Do not remove the terminator line. {0, 0} // Do not remove the terminator line.
}; };
...@@ -91,6 +93,8 @@ TestCase g_viewport_test_cases[] = { ...@@ -91,6 +93,8 @@ TestCase g_viewport_test_cases[] = {
{"(width)", 1}, {"(width)", 1},
{"(width: whatisthis)", 0}, {"(width: whatisthis)", 0},
{"screen and (min-width: 400px) and (max-width: 700px)", 1}, {"screen and (min-width: 400px) and (max-width: 700px)", 1},
{"(max-aspect-ratio: 4294967296/1)", 1},
{"(min-aspect-ratio: 1/4294967295)", 1},
{0, 0} // Do not remove the terminator line. {0, 0} // Do not remove the terminator line.
}; };
......
third_party/WebKit/Source/core/css/MediaQueryExp.cpp
View file @ cf115265
...@@ -284,8 +284,8 @@ MediaQueryExp MediaQueryExp::Create( ...@@ -284,8 +284,8 @@ MediaQueryExp MediaQueryExp::Create(
denominator.GetNumericValueType() != kIntegerValueType) denominator.GetNumericValueType() != kIntegerValueType)
return Invalid(); return Invalid();
exp_value.numerator = (unsigned)numerator.NumericValue(); exp_value.numerator = clampTo<unsigned>(numerator.NumericValue());
exp_value.denominator = (unsigned)denominator.NumericValue(); exp_value.denominator = clampTo<unsigned>(denominator.NumericValue());
exp_value.is_ratio = true; exp_value.is_ratio = true;
} else { } else {
return Invalid(); return Invalid();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment