Avoid int coercion of box-shadow args

Avoid int coercion of box-shadow args

Most of the pipeline is already operating with float values, so there's
no need to pass int args for shadow-related GraphicsContext methods.

This addresses non-int shadow TODOs, and fixes asserts triggered by int
overflows.

TEST=fast/box-shadow/degenerate-radius-crash.html

BUG=424119, 334828
R=reed@google.com,cbiesinger@chromium.org,junov@chromium.org

Committed: https://crrev.com/d6fc1b14536d95e13ca06bbdfe427bfd693b5c79
Cr-Commit-Position: refs/heads/master@{#360113}

[chrishtr, 2015-11-16 17:12:43]

chrishtr@chromium.org changed reviewers:
+ chrishtr@chromium.org

[chrishtr, 2015-11-16 17:12:44]

https://codereview.chromium.org/1438043012/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/BoxPainter.cpp (left):

https://codereview.chromium.org/1438043012/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/BoxPainter.cpp:705:
context->drawInnerShadow(border, shadowColor, flooredIntSize(shadowOffset),
shadowBlur, shadowSpread, clippedEdges);
Please also update the call site in ThemePainterMac.mm.

Also, not flooring here changes output. Is there a missing test for
floating-point-sized shadows?

[Justin Novosad, 2015-11-16 23:07:19]

https://codereview.chromium.org/1438043012/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/DrawLooperBuilder.cpp (right):

https://codereview.chromium.org/1438043012/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/DrawLooperBuilder.cpp:70:
ASSERT(blur >= 0);
Did a bit of digging... I find it hard to prove with 100% certanty, just by
reading the code, that blur is always pre-clamped.  Are you sure about this
ASSERT? 
SkBlurMaskFilter::Create already knows to hanlde blurs <= 0 by not blurring.

[f(malita), 2015-11-17 13:55:24]

https://codereview.chromium.org/1438043012/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/BoxPainter.cpp (left):

https://codereview.chromium.org/1438043012/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/BoxPainter.cpp:705:
context->drawInnerShadow(border, shadowColor, flooredIntSize(shadowOffset),
shadowBlur, shadowSpread, clippedEdges);
On 2015/11/16 17:12:44, chrishtr wrote:
> Please also update the call site in ThemePainterMac.mm.

Done.


> Also, not flooring here changes output. Is there a missing test for
> floating-point-sized shadows?

Right, no tests for inset subpixel shadows.  I've added one to capture the new
behavior.

https://codereview.chromium.org/1438043012/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/DrawLooperBuilder.cpp (right):

https://codereview.chromium.org/1438043012/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/DrawLooperBuilder.cpp:70:
ASSERT(blur >= 0);
On 2015/11/16 23:07:19, Justin Novosad wrote:
> Did a bit of digging... I find it hard to prove with 100% certanty, just by
> reading the code, that blur is always pre-clamped.  Are you sure about this
> ASSERT?

I can't spot the place where it gets sanitized either, but negative values are
not allowed for blur radius, per spec:
http://www.w3.org/TR/css3-background/#box-shadow

Both the test and my manual fiddling cannot produce a negative value at this
stage - so I presume the style engine must clamp it (or default to 0 for inval
negative values).


> SkBlurMaskFilter::Create already knows to hanlde blurs <= 0 by not blurring.

Yeah, but skBlurRadiusToSigma ASSERTS on negative values - see linked bug.

[Justin Novosad, 2015-11-17 14:19:03]

lgtm

https://codereview.chromium.org/1438043012/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/DrawLooperBuilder.cpp (right):

https://codereview.chromium.org/1438043012/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/DrawLooperBuilder.cpp:70:
ASSERT(blur >= 0);
On 2015/11/17 13:55:23, f(malita) wrote:

> I can't spot the place where it gets sanitized either, but negative values are
> not allowed for blur radius, per spec:
> http://www.w3.org/TR/css3-background/#box-shadow

This code is also used by 2d canvas, svg, and the values may be the result of an
animation curve interpolation. Canvas and animation curves are clamping, I
checked. Anyways, I think this is Okay.

[chrishtr, 2015-11-17 18:01:17]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2015-11-17 18:01:17]

lgtm

[commit-bot: I haz the power, 2015-11-17 18:03:35]

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

[commit-bot: I haz the power, 2015-11-17 18:11:27]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-11-17 18:12:48]

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