Return CanvasRenderingContext2D instead of DOMObject in IDL to avoid V8 #ifdefs The code generator for JSC and V8 is a bit simpler when the return type of a method matches the actual type in the code. I know that getContext and getCSSCanvasContext are both spec'd to return DOMObject, but presently our code generators go through some extra steps to force that to mean CanvasRenderingContext2D. We might as well just change the IDL.
Created attachment 27432 [details] v1 patch
Comment on attachment 27432 [details] v1 patch Long term, this is wrong. The context could be other classes depending on the passed-in context ID. So we may want to make a different type of change here.
Comment on attachment 27432 [details] v1 patch Agreed. This would make it impossible to implement Context3D for example (which I figure we'll eventually do to match moz and opera)
Yeah, that's correct, but we don't have any Context3D yet. When we do, we can figure out how to abstract this return value. Why figure that out now? Seems like premature effort to me. My change just moves "hard coded logic" from the code generator into the IDL. That doesn't seem like a step back to me :-)
Would you guys prefer if the getContext return type were changed to CanvasRenderingContext instead of CanvasRenderingContext2D? I could then add a CanvasRenderingContext header that includes the typedef that currently lives in CanvasRenderingContext2D.h. Would that be better? I'm striving for avoiding hacks in the code generator :)
Comment on attachment 27432 [details] v1 patch I agree, it's overkill. The header with a separate CanvasRenderingContext would be cleanest, but we can solve that when we get a Context3d.
http://trac.webkit.org/changeset/40797 I left the patch as-is. It turns out that toString() currently returns "[object CanvasRenderingContext2D]", so the v1 patch also has the property of making the IDL match the JS better.
Re-opening since Sam didn't like this fix. Reverted as: http://trac.webkit.org/changeset/40810
Closing this out in favor of the solution in bug 24311.