Bug 23458 - Reintroduce CanvasPixelArray in ImageData.idl
Summary: Reintroduce CanvasPixelArray in ImageData.idl
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-01-21 10:39 PST by Ivan Posva
Modified: 2009-01-21 13:06 PST (History)
2 users (show)

See Also:


Attachments
proposed patch (8.42 KB, patch)
2009-01-21 10:45 PST, Ivan Posva
oliver: review-
Details | Formatted Diff | Diff
Return CPA to basically its old implementation, extra indirection and all (23.25 KB, patch)
2009-01-21 12:01 PST, Oliver Hunt
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ivan Posva 2009-01-21 10:39:31 PST
Reintroduce CanvasPixelArray in ImageData.idl:

Recent changes to the ImageData.idl broke non-JSC bindings. This change reintroduces the CanvasPixelArray and for the JSC build defines it to be equivalent to WTF::ByteArray.
Comment 1 Ivan Posva 2009-01-21 10:45:47 PST
Created attachment 26897 [details]
proposed patch

Ideally I would have wanted to use "#if defined(LANGUAGE_JAVASCRIPT) && !defined(JSC_BINDING)" as you suggested to Darin, but it appears that something like JSC_BINDING does not exist in the code generator.
Comment 2 Oliver Hunt 2009-01-21 10:51:46 PST
Comment on attachment 26897 [details]
proposed patch

r=me
Comment 3 Oliver Hunt 2009-01-21 10:58:33 PST
Comment on attachment 26897 [details]
proposed patch

Actually, on second thoughts, i think a better solution would be to make CanvasPixelArray be a subclass of ByteArray for those builds that need it -- all builds should be using WTF::ByteArray to ensure consistency, and just typedef-ing the type difference away could lead to platform inconsistencies
Comment 4 Oliver Hunt 2009-01-21 11:00:57 PST
Basically, my though is that ImageData should be in terms of WTF::ByteArray, but those builds that need to declare it as type CanvasPixelArray should have there own typedef
Comment 5 Oliver Hunt 2009-01-21 11:01:41 PST
Err, need to declare in IDL -- do we support typedefs in IDL files?
Comment 6 Oliver Hunt 2009-01-21 12:01:36 PST
Created attachment 26899 [details]
Return CPA to basically its old implementation, extra indirection and all

This builds, just doing final layout test run atm
Comment 7 Alexey Proskuryakov 2009-01-21 12:12:42 PST
Comment on attachment 26899 [details]
Return CPA to basically its old implementation, extra indirection and all

r=me

+#if !defined(LANGUAGE_JAVASCRIPT) || !defined(JSC_BINDING)
...
+#if !defined(LANGUAGE_JAVASCRIPT) || defined(V8_BINDING)

Is it intentional that the checks are different?
Comment 8 Oliver Hunt 2009-01-21 13:06:29 PST
Committed r40089
	M	WebCore/WebCore.pro
	M	WebCore/ChangeLog
	M	WebCore/WebCore.vcproj/WebCore.vcproj
	M	WebCore/GNUmakefile.am
	M	WebCore/platform/graphics/cg/ImageBufferCG.cpp
	M	WebCore/platform/graphics/cairo/ImageBufferCairo.cpp
	M	WebCore/bindings/js/JSImageDataCustom.cpp
	M	WebCore/html/ImageData.h
	A	WebCore/html/CanvasPixelArray.h
	M	WebCore/html/ImageData.idl
	M	WebCore/html/ImageData.cpp
	A	WebCore/html/CanvasPixelArray.idl
	A	WebCore/html/CanvasPixelArray.cpp
	M	WebCore/html/CanvasRenderingContext2D.cpp
	M	WebCore/WebCore.xcodeproj/project.pbxproj
	M	WebCore/WebCoreSources.bkl