Bug 23627

Summary: PLATFORM(SKIA) build bustage in SVGPaintServer.cpp
Product: WebKit Reporter: Darin Fisher (:fishd, Google) <fishd>
Component: PlatformAssignee: Darin Fisher (:fishd, Google) <fishd>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
v1 patch
darin: review-
v2 patch
none
v3 patch darin: review+

Description Darin Fisher (:fishd, Google) 2009-01-29 15:18:47 PST
PLATFORM(SKIA) build bustage in SVGPaintServer.cpp

Simple patch coming up...
Comment 1 Darin Fisher (:fishd, Google) 2009-01-29 15:19:31 PST
Created attachment 27164 [details]
v1 patch
Comment 2 mitz 2009-01-29 15:20:53 PST
Comment on attachment 27164 [details]
v1 patch

I wonder if this won't trigger the unused parameters warning on non-Skia platforms.
Comment 3 Darin Fisher (:fishd, Google) 2009-01-29 15:22:39 PST
Oh, I didn't realize that warning was enabled.  I guess I could move the entire function into the #ifdef.
Comment 4 Darin Adler 2009-01-29 15:28:10 PST
Comment on attachment 27164 [details]
v1 patch

This will break the build on Mac OS X unless you use UNUSED_PARAM when it's not PLATFORM(SKIA).
Comment 5 Darin Fisher (:fishd, Google) 2009-01-29 15:32:01 PST
Created attachment 27165 [details]
v2 patch
Comment 6 Darin Adler 2009-01-29 15:33:43 PST
Comment on attachment 27165 [details]
v2 patch

> +#else
> +void SVGPaintServer::teardown(GraphicsContext*&, const RenderObject*, SVGPaintTargetType, bool) const
> +{
> +#endif

Missing a brace here. Please fix that!
Comment 7 Darin Fisher (:fishd, Google) 2009-01-29 15:35:47 PST
Created attachment 27167 [details]
v3 patch

3rd times a charm :)
Comment 8 Eric Seidel (no email) 2009-01-29 16:08:53 PST
I don't understand why this is necessary?  Why doesn't Skia want the standard "teardown" function?  Skia should be using the standard PatternSkia.cpp and GradientSkia.cpp abstractions instead of any custom SVGPaintServer* classes these days...
Comment 9 Darin Fisher (:fishd, Google) 2009-01-29 16:30:27 PST
That's a great question.  I think the FIXME implies a desire to clean that up.  I was just hoping to "fix bustage" in existing code.  Can I postpone cleanup to a subsequent bug?
Comment 10 Eric Seidel (no email) 2009-01-29 16:34:06 PST
Comment on attachment 27167 [details]
v3 patch

Oh, I'm sorry.  I thought you were adding the #ifdefs, I didn't realize they were already there.  Yes, this is totally fine.
Comment 11 Darin Fisher (:fishd, Google) 2009-01-29 16:40:07 PST
http://trac.webkit.org/changeset/40385