[CSS Shapes] Negative raster shape height leads to crash

[CSS Shapes] Negative raster shape height leads to crash

This patch clamps the size of the margin box rect used to compute the
raster shape on a float so the width and height cannot be negative.
This makes it safe to use that rect to determine the size of the shape
intervals data structure.

This is a merge of http://trac.webkit.org/changeset/178054
by Bem Jones-Bey <bjonesbe@adobe.com>;.

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

[rwlbuis, 2015-01-08 22:10:57]

rob.buis@samsung.com changed reviewers:
+ bemjb@chromium.org, bjonesbe@adobe.com

[rwlbuis, 2015-01-08 22:10:58]

PTAL, I don't think the win_rel failure is related.

[Bem Jones-Bey (adobe), 2015-01-09 00:01:09]

lgtm

https://codereview.chromium.org/844733004/diff/1/LayoutTests/fast/shapes/shap...
File
LayoutTests/fast/shapes/shape-outside-floats/shape-outside-negative-height-crash.html
(right):

https://codereview.chromium.org/844733004/diff/1/LayoutTests/fast/shapes/shap...
LayoutTests/fast/shapes/shape-outside-floats/shape-outside-negative-height-crash.html:12:
margin-top: -29px;
The WebKit patch has -19px here. Did that not crash for you in Blink? IIRC, it
crashed for me in Canary.

[rwlbuis, 2015-01-09 00:48:48]

On 2015/01/09 00:01:09, Bem Jones-Bey (adobe) wrote:
> lgtm
> 
>
https://codereview.chromium.org/844733004/diff/1/LayoutTests/fast/shapes/shap...
> File
>
LayoutTests/fast/shapes/shape-outside-floats/shape-outside-negative-height-crash.html
> (right):
> 
>
https://codereview.chromium.org/844733004/diff/1/LayoutTests/fast/shapes/shap...
>
LayoutTests/fast/shapes/shape-outside-floats/shape-outside-negative-height-crash.html:12:
> margin-top: -29px;
> The WebKit patch has -19px here. Did that not crash for you in Blink? IIRC, it
> crashed for me in Canary.

You are right. The testcase with -19px crashed Debug content_shell on Linux but
not Debug run_webkit_tests.
Could it be Linux specific (I assume you tested on OS X)?

[Bem Jones-Bey (adobe), 2015-01-09 20:19:43]

On 2015/01/09 00:48:48, rwlbuis wrote:
> On 2015/01/09 00:01:09, Bem Jones-Bey (adobe) wrote:
> > lgtm
> > 
> >
>
https://codereview.chromium.org/844733004/diff/1/LayoutTests/fast/shapes/shap...
> > File
> >
>
LayoutTests/fast/shapes/shape-outside-floats/shape-outside-negative-height-crash.html
> > (right):
> > 
> >
>
https://codereview.chromium.org/844733004/diff/1/LayoutTests/fast/shapes/shap...
> >
>
LayoutTests/fast/shapes/shape-outside-floats/shape-outside-negative-height-crash.html:12:
> > margin-top: -29px;
> > The WebKit patch has -19px here. Did that not crash for you in Blink? IIRC,
it
> > crashed for me in Canary.
> 
> You are right. The testcase with -19px crashed Debug content_shell on Linux
but
> not Debug run_webkit_tests.
> Could it be Linux specific (I assume you tested on OS X)?

Yeah, I tested on OS X, but it's more likely to have something to do with the
viewport size or something that content_shell does when running in DRT mode.

[rwlbuis, 2015-01-14 22:55:49]

On 2015/01/09 20:19:43, Bem Jones-Bey (adobe) wrote:
> On 2015/01/09 00:48:48, rwlbuis wrote:
> > On 2015/01/09 00:01:09, Bem Jones-Bey (adobe) wrote:
> > > lgtm
> > > 
> > >
> >
>
https://codereview.chromium.org/844733004/diff/1/LayoutTests/fast/shapes/shap...
> > > File
> > >
> >
>
LayoutTests/fast/shapes/shape-outside-floats/shape-outside-negative-height-crash.html
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/844733004/diff/1/LayoutTests/fast/shapes/shap...
> > >
> >
>
LayoutTests/fast/shapes/shape-outside-floats/shape-outside-negative-height-crash.html:12:
> > > margin-top: -29px;
> > > The WebKit patch has -19px here. Did that not crash for you in Blink?
IIRC,
> it
> > > crashed for me in Canary.
> > 
> > You are right. The testcase with -19px crashed Debug content_shell on Linux
> but
> > not Debug run_webkit_tests.
> > Could it be Linux specific (I assume you tested on OS X)?
> 
> Yeah, I tested on OS X, but it's more likely to have something to do with the
> viewport size or something that content_shell does when running in DRT mode.

Actually I think it may be the line-height. I am now forcing it and hopefully it
will turn up green.

[rwlbuis, 2015-01-14 23:48:11]

rob.buis@samsung.com changed reviewers:
+ leviw@chromium.org

[rwlbuis, 2015-01-14 23:48:11]

PTAL

[leviw_travelin_and_unemployed, 2015-01-14 23:50:47]

lgtm.

[rwlbuis, 2015-01-14 23:55:53]

The CQ bit was checked by rob.buis@samsung.com

[commit-bot: I haz the power, 2015-01-14 23:56:08]

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

[commit-bot: I haz the power, 2015-01-15 00:45:03]

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