Bug 26061 - [SKIA] Crash when filling an empty pattern
Summary: [SKIA] Crash when filling an empty pattern
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
Depends on:
Reported: 2009-05-28 06:08 PDT by Dean McNamee
Modified: 2009-06-10 11:05 PDT (History)
2 users (show)

See Also:

patch (3.31 KB, patch)
2009-05-28 06:10 PDT, Dean McNamee
eric: review+
Details | Formatted Diff | Diff
Patch with indention fixed (3.37 KB, patch)
2009-06-10 05:54 PDT, Dean McNamee
no flags 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 06:08:25 PDT
Skia-specific crash, where we assume that the bitmap for a pattern is valid.  In reality it can return NULL.  For example:

createPattern(new Image, "repeat")

Original Chromium bug:

Comment 1 Dean McNamee 2009-05-28 06:10:57 PDT
Created attachment 30735 [details]
Comment 2 Eric Seidel (no email) 2009-06-04 09:05:16 PDT
Comment on attachment 30735 [details]

The patch looks fine.

I would have used a named constant for the color.  I'm surprised that skia doesn't have a "transparent" constant that you can return instead of having to manually construct a color.

Color::transparent is identical, and is guaranteed to be the right format due to COMPILE_ASSERT checks in ColorSkia.cpp.

Tab indent should be 4 spaces, not 2.

Those are both nits someone could fix when landing.
Comment 3 Dean McNamee 2009-06-10 05:54:27 PDT
Created attachment 31129 [details]
Patch with indention fixed

Fixed the indention line.  I don't think making a constant (since Skia doesn't already have one) is worthwhile, it's pretty clear from the comment that I'm creating a transparent color.  I don't like using Color in place of SkColor, even though I realize they have the same layout.
Comment 4 Brent Fulgham 2009-06-10 11:05:29 PDT
Please don't remove "Reviewed by (OOPS)" comment created by prepare-ChangeLog.

Confirmed p-1.txt and p-2.txt differed only by spacing per Eric's comment and landed.

Landed in @r44575.