[webkit-reviews] review granted: [Bug 67958] Fix out-of-bounds access in Gradient::sortStopsIfNecessary : [Attachment 107091] More detailed ChangeLog

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Adam Klein <adamk at chromium.org>'s
request for review:
Bug 67958: Fix out-of-bounds access in Gradient::sortStopsIfNecessary
https://bugs.webkit.org/show_bug.cgi?id=67958

Attachment 107091: More detailed ChangeLog
https://bugs.webkit.org/attachment.cgi?id=107091&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=107091&action=review


> Source/WebCore/ChangeLog:14
> +	   No new tests (OOPS!).

You won’t be able to land a patch with this line in it.

> Source/WebCore/platform/graphics/Gradient.cpp:128
> -    if (m_stops.size() == 2 && compareStops(*m_stops.begin(),
*m_stops.end()))
> +    if (m_stops.size() == 2 && compareStops(m_stops.first(),
m_stops.last()))

Since this optimization has been failing, it would be better to remove it
entirely. Clearly we didn’t need it. No one noticed it was not working. It’s
possible that it even sorted incorrectly in some cases, and we could probably
construct a test case to show that incorrect behavior.

I’m going to say review+ for fixing it, but I think it’s not good to land a bug
fix without a test. If this is an important performance optimization, we should
create a performance test that demonstrates its benefit.