Summary: | PLATFORM(SKIA) build bustage in SVGPaintServer.cpp | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Darin Fisher (:fishd, Google) <fishd> | ||||||||
Component: | Platform | Assignee: | Darin Fisher (:fishd, Google) <fishd> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | ||||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Darin Fisher (:fishd, Google)
2009-01-29 15:18:47 PST
Created attachment 27164 [details]
v1 patch
Comment on attachment 27164 [details]
v1 patch
I wonder if this won't trigger the unused parameters warning on non-Skia platforms.
Oh, I didn't realize that warning was enabled. I guess I could move the entire function into the #ifdef. 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).
Created attachment 27165 [details]
v2 patch
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! Created attachment 27167 [details]
v3 patch
3rd times a charm :)
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... 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 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.
|