Bug 26059 - [SKIA] Crash in radial gradients with a radius of zero.
Summary: [SKIA] Crash in radial gradients with a radius of zero.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-28 04:49 PDT by Dean McNamee
Modified: 2009-06-10 17:24 PDT (History)
2 users (show)

See Also:


Attachments
patch (3.49 KB, patch)
2009-05-28 05:07 PDT, Dean McNamee
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.