Bug 26059 – [SKIA] Crash in radial gradients with a radius of zero.

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Dean McNamee]

This is a Skia-specific bug, originally from the Chromium bug tracker:

http://code.google.com/p/chromium/issues/detail?id=10731&can=3&colspec=ID%20Stars%20Pri%20Area%20Type%20Status%20Summary%20Modified%20Owner%20Mstone

The problem is that if we ask Skia to create a radial gradient with a radius of zero, it will fail and return NULL.  I don't see a way to return a failure from this code, so I will just map zero to the smallest possible radius.

Patch and layout test coming.

[Dean McNamee]

Created attachment 30734 [details]
patch

[Eric Seidel (no email)]

Comment on attachment 30734 [details]
patch

Why don't we just null-check m_gradiant in other places?  That would avoid this whole class of crashers, no?

[Dean McNamee]

(In reply to comment #2)
> (From update of attachment 30734 [details] [review])
> Why don't we just null-check m_gradiant in other places?  That would avoid this
> whole class of crashers, no?

What's the whole class of crashers?  The problem I see is that other ports don't expect this to ever fail.  See Gradient::platformGradient in the CG port or in the Cairo port.  The thing that seemed smartest to me was the match the other ports, and have this never fail.

What were you proposing?

> 

[Eric Seidel (no email)]

Comment on attachment 30734 [details]
patch

We should probably ASSERT(m_gradient) after this block then.  If m_gradient should never fail to create.

Someone can add an ASSERT when landing.

[Brent Fulgham]

Added ASSERT(m_gradient).  Corrected tab characters in LayoutTests/ChangeLog.

Landed in @r44574.

[Brent Fulgham]

Landed expected results in @r44595.

[Brent Fulgham]

Landed proper pixel-level expected results in @r44596.