Bug 26059

Summary: [SKIA] Crash in radial gradients with a radius of zero.
Product: WebKit Reporter: Dean McNamee <deanm>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, skylined
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
patch eric: review+

Description Dean McNamee 2009-05-28 04:49:34 PDT
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.
Comment 1 Dean McNamee 2009-05-28 05:07:00 PDT
Created attachment 30734 [details]
patch
Comment 2 Eric Seidel (no email) 2009-05-29 11:04:07 PDT
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?
Comment 3 Dean McNamee 2009-06-02 02:27:51 PDT
(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?

> 
Comment 4 Eric Seidel (no email) 2009-06-02 13:18:30 PDT
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.
Comment 5 Brent Fulgham 2009-06-10 10:57:43 PDT
Added ASSERT(m_gradient).  Corrected tab characters in LayoutTests/ChangeLog.

Landed in @r44574.
Comment 6 Brent Fulgham 2009-06-10 16:23:21 PDT
Landed expected results in @r44595.
Comment 7 Brent Fulgham 2009-06-10 17:24:37 PDT
Landed proper pixel-level expected results in @r44596.