Bug 102735 - Clamp out-of-range numbers in CSS
: Clamp out-of-range numbers in CSS
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: CSS
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To: Emil A Eklund
: WebExposed
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-19 16:18 PST by Emil A Eklund
Modified: 2012-12-11 14:58 PST (History)
15 users (show)

See Also:


Attachments
Patch (23.42 KB, patch)
2012-11-19 16:54 PST, Emil A Eklund
no flags Details | Formatted Diff | Diff
Patch (22.82 KB, patch)
2012-11-20 12:06 PST, Emil A Eklund
no flags Details | Formatted Diff | Diff
Patch (23.35 KB, patch)
2012-11-20 14:41 PST, Emil A Eklund
no flags Details | Formatted Diff | Diff
Patch (24.37 KB, patch)
2012-11-21 09:57 PST, Emil A Eklund
no flags Details | Formatted Diff | Diff
Patch (23.51 KB, patch)
2012-11-26 09:56 PST, Emil A Eklund
no flags Details | Formatted Diff | Diff
Patch (25.78 KB, patch)
2012-11-30 14:15 PST, Emil A Eklund
no flags Details | Formatted Diff | Diff
Patch (26.01 KB, patch)
2012-12-07 14:45 PST, Emil A Eklund
no flags Details | Formatted Diff | Diff
Patch (25.94 KB, patch)
2012-12-10 11:44 PST, Emil A Eklund
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Emil A Eklund 2012-11-19 16:18:29 PST
Currently when a large number (outside of the supported range) is applied from a style rule or set from javascript using Element.style we check if it is within the supported range and set it to zero if it is not. This is incorrect according to the CSS3 specification which instead mandates that the value be ignored. This patch changes the behavior to ignore out-of-range values.

"Properties may restrict numeric values to some range. If the value is outside the allowed range, the declaration is invalid and must be ignored." - http://www.w3.org/TR/css3-values/#numeric-types
Comment 1 Glenn Adams 2012-11-19 16:21:46 PST
do you have specific properties in mind, or is this a meta-requirement
Comment 2 Emil A Eklund 2012-11-19 16:43:48 PST
(In reply to comment #1)
> do you have specific properties in mind, or is this a meta-requirement

Length properties, specifically width, height, left and top as these have been causing some problems with a couple of google properties.

Downstream chromium bug: http://code.google.com/p/chromium/issues/detail?id=145798
Comment 3 Emil A Eklund 2012-11-19 16:54:40 PST
Created attachment 175081 [details]
Patch
Comment 4 Glenn Adams 2012-11-19 17:03:11 PST
Comment on attachment 175081 [details]
Patch

lgtm fwiw
Comment 5 Brent Fulgham 2012-11-19 17:22:32 PST
Comment on attachment 175081 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=175081&action=review

Looms good tome, too.

Please correct my minor nit on the a/an when landing.

R=me.

> Source/WebCore/ChangeLog:26
> +        Return an Length with the type set to Undefined for unsupported

Nit: return *a* length with ...

> Source/WebCore/css/CSSPrimitiveValue.cpp:-483
> -    return Length(static_cast<float>(value > intMaxForLayoutUnit || value < intMinForLayoutUnit ? 0.0 : value), Fixed);

Based on the naming of these limits, it almost seems like these bounds should be inclusive. However, they have obviously not been used this way historically...

> Source/WebCore/css/CSSPrimitiveValue.cpp:483
> +    if (value > intMaxForLayoutUnit || value < intMinForLayoutUnit)

Whoops! They are inclusive. Never mind! :)
Comment 6 Emil A Eklund 2012-11-19 17:26:09 PST
(In reply to comment #5)
> (From update of attachment 175081 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=175081&action=review
> 
> Looms good tome, too.
> 
> Please correct my minor nit on the a/an when landing.

Thanks for the review!
I'll wait until tomorrow to commit it to give others a chance to comment further or object.


> > Source/WebCore/ChangeLog:26
> > +        Return an Length with the type set to Undefined for unsupported
> 
> Nit: return *a* length with ...

Good catch, thanks!


> Whoops! They are inclusive. Never mind! :)

I tried to make it clearer by reversing the check.
Comment 7 Glenn Adams 2012-11-19 17:33:59 PST
Comment on attachment 175081 [details]
Patch

one more thought... it might be worth generating a warning in the inspector log when an out of bounds value is ignored, but that could be a follow-up patch
Comment 8 Emil A Eklund 2012-11-19 17:38:35 PST
(In reply to comment #7)
> (From update of attachment 175081 [details])
> one more thought... it might be worth generating a warning in the inspector log when an out of bounds value is ignored, but that could be a follow-up patch

That is not a bad idea. Filed bug 102750 to track it.
Comment 9 Tab Atkins 2012-11-20 09:28:18 PST
(In reply to comment #0)
> Currently when a large number (outside of the supported range) is applied from a style rule or set from javascript using Element.style we check if it is within the supported range and set it to zero if it is not. This is incorrect according to the CSS3 specification which instead mandates that the value be ignored. This patch changes the behavior to ignore out-of-range values.
> 
> "Properties may restrict numeric values to some range. If the value is outside the allowed range, the declaration is invalid and must be ignored." - http://www.w3.org/TR/css3-values/#numeric-types

You're reading this slightly wrong.  *Properties* can restrict their range, which should cause rejection at parse-time if you violate that range.  Authors can know ahead of time what the allowed range is, and violating that range generally means you're supplying a nonsensical value.

The Chromium bug in question is related to *implementation-defined* limits on the allowed range.  The behavior here is undefined by CSS.  (We wanted to define it in Values & Units 3, but had to punt it to get the spec to CR.  It'll reappear in level 4.)  The most author-friendly behavior is to clamp at the limit of our range.  That gives the author something as close as we can deliver to what they asked for.

Rejecting the rule at parse-time is author-hostile, as authors have no way of predicting how large is "too large", since it's impl-defined and can change over time, and predicting wrongly will cause properties to be ignored in a seemingly random fashion, perhaps in a non-interoperable way.
Comment 10 Emil A Eklund 2012-11-20 09:42:26 PST
(In reply to comment #9)
> (In reply to comment #0)
> > Currently when a large number (outside of the supported range) is applied from a style rule or set from javascript using Element.style we check if it is within the supported range and set it to zero if it is not. This is incorrect according to the CSS3 specification which instead mandates that the value be ignored. This patch changes the behavior to ignore out-of-range values.
> > 
> > "Properties may restrict numeric values to some range. If the value is outside the allowed range, the declaration is invalid and must be ignored." - http://www.w3.org/TR/css3-values/#numeric-types
> 
> You're reading this slightly wrong.  *Properties* can restrict their range, which should cause rejection at parse-time if you violate that range.  Authors can know ahead of time what the allowed range is, and violating that range generally means you're supplying a nonsensical value.
> 
> The Chromium bug in question is related to *implementation-defined* limits on the allowed range.  The behavior here is undefined by CSS.  (We wanted to define it in Values & Units 3, but had to punt it to get the spec to CR.  It'll reappear in level 4.)  The most author-friendly behavior is to clamp at the limit of our range.  That gives the author something as close as we can deliver to what they asked for.

Thanks for clarifying, it would be really helpful if we could add that back to the next version of the spec.

> Rejecting the rule at parse-time is author-hostile, as authors have no way of predicting how large is "too large", since it's impl-defined and can change over time, and predicting wrongly will cause properties to be ignored in a seemingly random fashion, perhaps in a non-interoperable way.

Ok, I'll change it to clamp values instead (which matches the gecko behavior).
Comment 11 Tab Atkins 2012-11-20 10:07:46 PST
(In reply to comment #10)
> Ok, I'll change it to clamp values instead (which matches the gecko behavior).

Thanks, Emil!  That'll make it a lot easier to specify in the next go-round of the spec, too.
Comment 12 Emil A Eklund 2012-11-20 12:06:03 PST
Created attachment 175260 [details]
Patch
Comment 13 Emil A Eklund 2012-11-20 12:06:51 PST
Made the changes suggested by Tab. Please take another look (cleared the r+ as this is effectively a different change now).
Comment 14 Glenn Adams 2012-11-20 12:11:20 PST
Just an FYI, but CSS specs are not presently consistent regarding clamping versus ignoring. For example, CSS3 Animation requires a keyframe declaration outside of 0-100% to be ignored and not merely clamped to 0 or 100. See https://bugs.webkit.org/show_bug.cgi?id=96844.
Comment 15 Tab Atkins 2012-11-20 12:21:43 PST
(In reply to comment #14)
> Just an FYI, but CSS specs are not presently consistent regarding clamping versus ignoring. For example, CSS3 Animation requires a keyframe declaration outside of 0-100% to be ignored and not merely clamped to 0 or 100. See https://bugs.webkit.org/show_bug.cgi?id=96844.

Again, as I explained, the rule is that *language-defined limits* cause things to be ignored.  It's a syntax error, since you know about it beforehand and everyone is the same (barring bugs).

We clamp when we encounter *implementation-defined limits*, because they're unspecified and variable across time and browsers, so there's no way for authors to robustly protect themselves from violating the limit.

The behavior of keyframes is consistent with this.  (If you do know of any inconsistencies, please report them on www-style so we can try to fix them!)
Comment 16 WebKit Review Bot 2012-11-20 13:50:21 PST
Comment on attachment 175260 [details]
Patch

Attachment 175260 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14907712

New failing tests:
css3/flexbox/flex-algorithm.html
Comment 17 Build Bot 2012-11-20 14:34:14 PST
Comment on attachment 175260 [details]
Patch

Attachment 175260 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14907720

New failing tests:
css3/flexbox/flex-algorithm.html
Comment 18 Emil A Eklund 2012-11-20 14:41:38 PST
Created attachment 175283 [details]
Patch
Comment 19 Tony Chang 2012-11-20 14:46:27 PST
Comment on attachment 175283 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=175283&action=review

> LayoutTests/css3/flexbox/flex-algorithm.html:133
> -  <div data-expected-width="600" style="-webkit-flex: 100000000000000000000000000000000000000 0 600px; -moz-flex: 100000000000000000000000000000000000000 0 600px"></div>
> +  <div data-expected-width="0" style="-webkit-flex: 100000000000000000000000000000000000000 0 600px; -moz-flex: 100000000000000000000000000000000000000 0 600px"></div>
>    <div data-expected-width="600" style="-webkit-flex: 0 100000000000000000000000000000000000000 600px; -moz-flex: 0 100000000000000000000000000000000000000 600px"></div>
> -  <div data-expected-width="0" style="-webkit-flex: 1 1 100000000000000000000000000000000000000px; -moz-flex: 1 1 100000000000000000000000000000000000000px"></div>
> +  <div data-expected-width="33554428" style="-webkit-flex: 1 1 100000000000000000000000000000000000000px; -moz-flex: 1 1 100000000000000000000000000000000000000px"></div>

FWIW, this change seems fine to me.  The test is just making sure that we don't crash or infinite loop when there are really big input values.
Comment 20 Emil A Eklund 2012-11-20 14:47:17 PST
(In reply to comment #19)
> (From update of attachment 175283 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=175283&action=review
> 
> > LayoutTests/css3/flexbox/flex-algorithm.html:133
> > -  <div data-expected-width="600" style="-webkit-flex: 100000000000000000000000000000000000000 0 600px; -moz-flex: 100000000000000000000000000000000000000 0 600px"></div>
> > +  <div data-expected-width="0" style="-webkit-flex: 100000000000000000000000000000000000000 0 600px; -moz-flex: 100000000000000000000000000000000000000 0 600px"></div>
> >    <div data-expected-width="600" style="-webkit-flex: 0 100000000000000000000000000000000000000 600px; -moz-flex: 0 100000000000000000000000000000000000000 600px"></div>
> > -  <div data-expected-width="0" style="-webkit-flex: 1 1 100000000000000000000000000000000000000px; -moz-flex: 1 1 100000000000000000000000000000000000000px"></div>
> > +  <div data-expected-width="33554428" style="-webkit-flex: 1 1 100000000000000000000000000000000000000px; -moz-flex: 1 1 100000000000000000000000000000000000000px"></div>
> 
> FWIW, this change seems fine to me.  The test is just making sure that we don't crash or infinite loop when there are really big input values.

Ah, was just about to ask you to validate that. Thanks!
I tested with other large (but in-range) numbers and got the same results/behavior.
Comment 21 Benjamin Poulain 2012-11-20 18:02:08 PST
Comment on attachment 175283 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=175283&action=review

The conversion to zero seems arbitrary and confusing, it looks like a good idea to clamp it. I have a couple of questions:

> Source/WebCore/css/CSSPrimitiveValue.cpp:485
>      double value = computeLengthDouble(style, rootStyle, multiplier, computingFontSize);
> -    return Length(static_cast<float>(value > intMaxForLayoutUnit || value < intMinForLayoutUnit ? 0.0 : value), Fixed);
> +    return Length(clampToLayoutUnitRange(value), Fixed);
>  #else
>      return Length(roundForImpreciseConversion<float>(computeLengthDouble(style, rootStyle, multiplier, computingFontSize)), Fixed);

Why is the Length clamped here and not when the conversion double->int occurs?

To avoid fragmentation, could we have the exact same clamping rules with and without ENABLE(SUBPIXEL_LAYOUT)?

> Source/WebCore/platform/LayoutUnit.h:869
> +    // Adjust values somewhat to allow for float imprecision and rounding without overflowing.
> +    return clampTo<float>(value, intMinForLayoutUnit + 2, intMaxForLayoutUnit - 2);

Can you please explain why you use the value 2 here?
Comment 22 Emil A Eklund 2012-11-20 18:10:25 PST
(In reply to comment #21)
> (From update of attachment 175283 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=175283&action=review
> 
> The conversion to zero seems arbitrary and confusing, it looks like a good idea to clamp it. I have a couple of questions:
> 
> > Source/WebCore/css/CSSPrimitiveValue.cpp:485
> >      double value = computeLengthDouble(style, rootStyle, multiplier, computingFontSize);
> > -    return Length(static_cast<float>(value > intMaxForLayoutUnit || value < intMinForLayoutUnit ? 0.0 : value), Fixed);
> > +    return Length(clampToLayoutUnitRange(value), Fixed);
> >  #else
> >      return Length(roundForImpreciseConversion<float>(computeLengthDouble(style, rootStyle, multiplier, computingFontSize)), Fixed);
> 
> Why is the Length clamped here and not when the conversion double->int occurs?

We don't convert it to an int, it is represented as a float in Length and then covered to a LayoutUnit.

> To avoid fragmentation, could we have the exact same clamping rules with and without ENABLE(SUBPIXEL_LAYOUT)?

We could but then we'd have to hardcode the MIN/MAX values. That would probably be fine.

> 
> > Source/WebCore/platform/LayoutUnit.h:869
> > +    // Adjust values somewhat to allow for float imprecision and rounding without overflowing.
> > +    return clampTo<float>(value, intMinForLayoutUnit + 2, intMaxForLayoutUnit - 2);
> 
> Can you please explain why you use the value 2 here?

To ensure that we have a number that can be correctly represented as a float and having enough room for rounding without overflowing. Usig intMinForLayoutUnit and intMaxForLayoutUnit as is would overflow when rounding, adding/subtracting one and the values returned by computed style differs from the values returned by the offsetTop/Width/* properties due to float<->LayoutUnit conversions.

I suppose just hardcoding the min/max values might be a better idea and would also allow the same behavior regardless of the ENABLE_SUBPIXEL flag.
Comment 23 Benjamin Poulain 2012-11-20 20:07:58 PST
> > > Source/WebCore/css/CSSPrimitiveValue.cpp:485
> > >      double value = computeLengthDouble(style, rootStyle, multiplier, computingFontSize);
> > > -    return Length(static_cast<float>(value > intMaxForLayoutUnit || value < intMinForLayoutUnit ? 0.0 : value), Fixed);
> > > +    return Length(clampToLayoutUnitRange(value), Fixed);
> > >  #else
> > >      return Length(roundForImpreciseConversion<float>(computeLengthDouble(style, rootStyle, multiplier, computingFontSize)), Fixed);
> > 
> > Why is the Length clamped here and not when the conversion double->int occurs?
> 
> We don't convert it to an int, it is represented as a float in Length and then covered to a LayoutUnit.

I know...but if you look at what is LayoutUnit: an int ;)

I believed the conversion in LayoutUnit was already clamping the input, but that's actually only true with SATURATED_LAYOUT_ARITHMETIC.
This makes me believe the original code using intMaxForLayoutUnit && intMinForLayoutUnit may have been bogus to begin with, and SATURATED_LAYOUT_ARITHMETIC is the right solution.

> > > Source/WebCore/platform/LayoutUnit.h:869
> > > +    // Adjust values somewhat to allow for float imprecision and rounding without overflowing.
> > > +    return clampTo<float>(value, intMinForLayoutUnit + 2, intMaxForLayoutUnit - 2);
> > 
> > Can you please explain why you use the value 2 here?
> 
> To ensure that we have a number that can be correctly represented as a float and having enough room for rounding without overflowing. Usig intMinForLayoutUnit and intMaxForLayoutUnit as is would overflow when rounding, adding/subtracting one and the values returned by computed style differs from the values returned by the offsetTop/Width/* properties due to float<->LayoutUnit conversions.
> 
> I suppose just hardcoding the min/max values might be a better idea and would also allow the same behavior regardless of the ENABLE_SUBPIXEL flag.

Yep, that'd be great.

I think the code must use the same path as float<->LayoutUnit in order to be more readable.
Comment 24 Emil A Eklund 2012-11-21 09:56:35 PST
(In reply to comment #23)
> > > > Source/WebCore/css/CSSPrimitiveValue.cpp:485
> > > >      double value = computeLengthDouble(style, rootStyle, multiplier, computingFontSize);
> > > > -    return Length(static_cast<float>(value > intMaxForLayoutUnit || value < intMinForLayoutUnit ? 0.0 : value), Fixed);
> > > > +    return Length(clampToLayoutUnitRange(value), Fixed);
> > > >  #else
> > > >      return Length(roundForImpreciseConversion<float>(computeLengthDouble(style, rootStyle, multiplier, computingFontSize)), Fixed);
> > > 
> > > Why is the Length clamped here and not when the conversion double->int occurs?
> > 
> > We don't convert it to an int, it is represented as a float in Length and then covered to a LayoutUnit.
> 
> I know...but if you look at what is LayoutUnit: an int ;)
> 
> I believed the conversion in LayoutUnit was already clamping the input, but that's actually only true with SATURATED_LAYOUT_ARITHMETIC.
> This makes me believe the original code using intMaxForLayoutUnit && intMinForLayoutUnit may have been bogus to begin with, and SATURATED_LAYOUT_ARITHMETIC is the right solution.

Right, I've been trying to turn on SATURATED_LAYOUT_ARITHMETIC for awhile but there are still some lingering performance concerns. Once (if) it has been turned on everywhere we can revisit this and remove the clampTo call here.

> 
> > > > Source/WebCore/platform/LayoutUnit.h:869
> > > > +    // Adjust values somewhat to allow for float imprecision and rounding without overflowing.
> > > > +    return clampTo<float>(value, intMinForLayoutUnit + 2, intMaxForLayoutUnit - 2);
> > > 
> > > Can you please explain why you use the value 2 here?
> > 
> > To ensure that we have a number that can be correctly represented as a float and having enough room for rounding without overflowing. Usig intMinForLayoutUnit and intMaxForLayoutUnit as is would overflow when rounding, adding/subtracting one and the values returned by computed style differs from the values returned by the offsetTop/Width/* properties due to float<->LayoutUnit conversions.
> > 
> > I suppose just hardcoding the min/max values might be a better idea and would also allow the same behavior regardless of the ENABLE_SUBPIXEL flag.
> 
> Yep, that'd be great.
> 
> I think the code must use the same path as float<->LayoutUnit in order to be more readable.

I changed both to use the same clampTo call. The non-subpixel path still needs do some some other rounding though.
Comment 25 Emil A Eklund 2012-11-21 09:57:26 PST
Created attachment 175470 [details]
Patch
Comment 26 Emil A Eklund 2012-11-26 09:56:20 PST
Created attachment 176023 [details]
Patch
Comment 27 Emil A Eklund 2012-11-27 15:33:44 PST
Cleaned it up a bit and made sure both code paths (subpixel on/off) uses the same logic. Please take another look.
Comment 28 Emil A Eklund 2012-11-30 10:29:27 PST
Please take another look when you get a chance, thanks!
Comment 29 Glenn Adams 2012-11-30 13:36:01 PST
>LayoutTests/platform/chromium/fast/css/large-number-round-trip-expected.txt
>1 PASS: read 90010000px back as 0px, read again as 0px
> 1PASS: read 90010000px back as 33554428px, read again as 33554428px

I wonder if the above should also include a test for the large negative number clamp (-33554430)?

>Source/WebCore/css/CSSPrimitiveValue.cpp
>56 // Max/min values for CSS, needs to slightly smaller/larger than the true max/min values to allow for rounding without overflowing.
> 57 const int maxValueForCssLength = 33554428;
> 58 const int minValueForCssLength = -33554430;

One wonders where these magic numbers come from. It might be nice to add something to the comment about how these are derived.
Comment 30 Emil A Eklund 2012-11-30 14:15:06 PST
Created attachment 177023 [details]
Patch
Comment 31 Emil A Eklund 2012-11-30 14:15:49 PST
Made the changes you suggested, please take another look.
Comment 32 Glenn Adams 2012-11-30 15:52:35 PST
lgtm tnx!
Comment 33 Brent Fulgham 2012-12-03 16:41:14 PST
Comment on attachment 177023 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=177023&action=review

I had one suggestion about the two magic numbers you use, but otherwise it looks good to me.

> Source/WebCore/css/CSSPrimitiveValue.cpp:59
> +const int minValueForCssLength = -33554430;

If that is the case, why can't we reference LayoutUnit::nearlyMax or nearlyMin?  Or if they aren't in scope for some reason, just inline them as:

#if ENABLE(SUBPIXEL_LAYOUT)
static const int kFixedPointDenominator = 64;
#else
static const int kFixedPointDenominator = 1;
#endif

const int maxValueForCssLength = std::numeric_limits<int>::max() - kFixedPointDenominator / 2;
const int minValueForCssLength = std::numeric_limits<int>::min() + kFixedPointDenominator / 2;

Instead of the magic number 33554428, we now have the kFixedPointDenominator of 1 or 64, which are easy to understand and defend.
Comment 34 Emil A Eklund 2012-12-03 17:55:59 PST
(In reply to comment #33)
> Instead of the magic number 33554428, we now have the kFixedPointDenominator of 1 or 64, which are easy to understand and defend.

It is not quite that easy. I can't use nearlyMax/nearlyMin as that would result in different limits depending on the SUBPIXEL flag. Similarly I can't use "std::numeric_limits<int>::max() / 64 - 1" due to loosing precision when converting to a float and back.

INT_MAX / 64 == 33554431
static_cast<int>(static_cast<float>(INT_MAX / 64) - 1) + 1 == 33554433

The largest value I've found that works is 33554429 (INT_MAX / 64 - 2).

The cleanest solution would be to use (1 << 24) as the max value as it is guaranteed to have a stable representation but that would in effective reduce the the usable range of numbers by a factor of two.
Comment 35 Emil A Eklund 2012-12-04 14:25:43 PST
Any suggestions as to which is preferable?
Comment 36 Emil A Eklund 2012-12-06 17:41:28 PST
Simon, what do you think we should do here? I don't really want to hard-code the limits.
Comment 37 Simon Fraser (smfr) 2012-12-06 17:48:34 PST
(In reply to comment #36)
> Simon, what do you think we should do here? I don't really want to hard-code the limits.

Can't you define it as (INT_MAX / kFixedPointDenominator - 2)? That seems close enough.
Comment 38 Emil A Eklund 2012-12-06 17:55:30 PST
(In reply to comment #37)
> (In reply to comment #36)
> > Simon, what do you think we should do here? I don't really want to hard-code the limits.
> 
> Can't you define it as (INT_MAX / kFixedPointDenominator - 2)? That seems close enough.

Yeah, that's probably the best we can do. Thanks!
Comment 39 Emil A Eklund 2012-12-07 14:45:11 PST
Created attachment 178284 [details]
Patch
Comment 40 Build Bot 2012-12-07 21:51:53 PST
Comment on attachment 178284 [details]
Patch

Attachment 178284 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15191519
Comment 41 Emil A Eklund 2012-12-10 11:44:52 PST
Created attachment 178603 [details]
Patch
Comment 42 Emil A Eklund 2012-12-11 14:08:38 PST
Brent/Simon, would either of you mind taking another look?
Comment 43 WebKit Review Bot 2012-12-11 14:57:54 PST
Comment on attachment 178603 [details]
Patch

Clearing flags on attachment: 178603

Committed r137365: <http://trac.webkit.org/changeset/137365>
Comment 44 WebKit Review Bot 2012-12-11 14:58:00 PST
All reviewed patches have been landed.  Closing bug.