Bad ASSERT() in RasterShapeIntervals::firstIncludedIntervalY()

[Hans Muller]

The changes for https://bugs.webkit.org/show_bug.cgi?id=120211 included an over-constrained ASSERT() in  RasterShapeIntervals::firstIncludedIntervalY():

bool RasterShapeIntervals::firstIncludedIntervalY(int minY, const IntSize& minSize, LayoutUnit& result) const
{
    minY = std::max<int>(bounds().y(), minY);

    ASSERT(minY >= 0 && minY + minSize.height() < size());
...
}

It's only necessary for the caller to ensure that minY is effectively < size(). Legitimate calls to firstIncluedIntervalY() may specify values of minY that are greater than size() - minSize.height().

[Hans Muller]

Created attachment 211840 [details]
Patch

Fixed the bad ASSERT() in RasterShapeIntervals::firstIncludedIntervalY().

[Darin Adler]

Comment on attachment 211840 [details]
Patch

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

> Source/WebCore/rendering/shapes/RasterShape.cpp:100
> -    ASSERT(minY >= 0 && minY + minSize.height() < size());
> +    ASSERT(minY >= 0 && minY < size());

Wouldn’t the right fix be to make the assertion "<=" rather than leaving out the height?

[Hans Muller]

(In reply to comment #2)
> (From update of attachment 211840 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=211840&action=review
> 
> > Source/WebCore/rendering/shapes/RasterShape.cpp:100
> > -    ASSERT(minY >= 0 && minY + minSize.height() < size());
> > +    ASSERT(minY >= 0 && minY < size());
> 
> Wouldn’t the right fix be to make the assertion "<=" rather than leaving out the height?

If minY + minSize.height() extends beyond the bounds of the shape, then this function is just supposed to return false.  And bounds().height() <= size().  The assertion is only supposed to insure that minY is in range.

Thanks for the quick review!

[WebKit Commit Bot]

Comment on attachment 211840 [details]
Patch

Clearing flags on attachment: 211840

Committed r155965: <http://trac.webkit.org/changeset/155965>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.