23809 2009-02-06 16:35:55 -0800 Return CanvasRenderingContext2D instead of DOMObject in IDL to avoid V8 #ifdefs 2009-03-02 20:04:18 -0800 1 1 1 Unclassified WebKit WebCore JavaScript 528+ (Nightly build) All All RESOLVED WONTFIX P2 Normal --- 1 fishd fishd darin eric sam oldest_to_newest 108764 0 fishd 2009-02-06 16:35:55 -0800 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. 108765 1 27432 fishd 2009-02-06 16:44:48 -0800 Created attachment 27432 v1 patch 108767 2 27432 darin 2009-02-06 16:47:57 -0800 Comment on attachment 27432 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. 108768 3 27432 eric 2009-02-06 16:49:53 -0800 Comment on attachment 27432 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) 108794 4 fishd 2009-02-06 19:16:18 -0800 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 :-) 108795 5 fishd 2009-02-06 19:21:54 -0800 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 :) 108808 6 27432 eric 2009-02-07 02:11:32 -0800 Comment on attachment 27432 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. 109033 7 fishd 2009-02-09 14:34:02 -0800 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. 109094 8 fishd 2009-02-10 00:17:40 -0800 Re-opening since Sam didn't like this fix. Reverted as: http://trac.webkit.org/changeset/40810 111957 9 fishd 2009-03-02 20:04:18 -0800 Closing this out in favor of the solution in bug 24311. 27432 2009-02-06 16:44:48 -0800 2009-02-10 00:18:07 -0800 v1 patch explicit_1.diff text/plain 2163 fishd SW5kZXg6IENoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBDaGFuZ2VMb2cJKHJldmlzaW9uIDQwNzQw KQorKysgQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTQgQEAKKzIwMDktMDIt MDYgIERhcmluIEZpc2hlciAgPGRhcmluQGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9MjM4MDkKKyAgICAgICAgUmV0dXJuIENhbnZhc1JlbmRlcmluZ0NvbnRl eHQyRCBpbnN0ZWFkIG9mIERPTU9iamVjdCBpbiBJREwgdG8gYXZvaWQgVjggI2lmZGVmcworCisg ICAgICAgICogYmluZGluZ3Mvc2NyaXB0cy9Db2RlR2VuZXJhdG9ySlMucG06CisgICAgICAgICog ZG9tL0RvY3VtZW50LmlkbDoKKyAgICAgICAgKiBodG1sL0hUTUxDYW52YXNFbGVtZW50LmlkbDoK KwogMjAwOS0wMi0wNiAgR2VvZmZyZXkgR2FyZW4gIDxnZ2FyZW5AYXBwbGUuY29tPgogCiAgICAg ICAgIFJldmlld2VkIGJ5IFNhbSBXZWluaWcuCkluZGV4OiBiaW5kaW5ncy9zY3JpcHRzL0NvZGVH ZW5lcmF0b3JKUy5wbQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBiaW5kaW5ncy9zY3JpcHRzL0NvZGVHZW5lcmF0 b3JKUy5wbQkocmV2aXNpb24gNDA3MzQpCisrKyBiaW5kaW5ncy9zY3JpcHRzL0NvZGVHZW5lcmF0 b3JKUy5wbQkod29ya2luZyBjb3B5KQpAQCAtMTY5OSw5ICsxNjk5LDcgQEAgc3ViIE5hdGl2ZVRv SlNWYWx1ZQogICAgICAgICAkaW1wbEluY2x1ZGVzeyJOYW1lTm9kZUxpc3QuaCJ9ID0gMTsKICAg ICB9CiAKLSAgICBpZiAoJHR5cGUgZXEgIkRPTU9iamVjdCIpIHsKLSAgICAgICAgJGltcGxJbmNs dWRlc3siSlNDYW52YXNSZW5kZXJpbmdDb250ZXh0MkQuaCJ9ID0gMTsKLSAgICB9IGVsc2lmICgk dHlwZSA9fiAvU1ZHUGF0aFNlZy8pIHsKKyAgICBpZiAoJHR5cGUgPX4gL1NWR1BhdGhTZWcvKSB7 CiAgICAgICAgICRpbXBsSW5jbHVkZXN7IkpTJHR5cGUuaCJ9ID0gMTsKICAgICAgICAgJGpvaW5l ZE5hbWUgPSAkdHlwZTsKICAgICAgICAgJGpvaW5lZE5hbWUgPX4gcy9BYnN8UmVsLy87CkluZGV4 OiBkb20vRG9jdW1lbnQuaWRsCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIGRvbS9Eb2N1bWVudC5pZGwJKHJldmlz aW9uIDQwNzM0KQorKysgZG9tL0RvY3VtZW50LmlkbAkod29ya2luZyBjb3B5KQpAQCAtMjI2LDcg KzIyNiw3IEBAIG1vZHVsZSBjb3JlIHsKIAogI2lmICFkZWZpbmVkKExBTkdVQUdFX0NPTSkKICNp ZiAhZGVmaW5lZChMQU5HVUFHRV9PQkpFQ1RJVkVfQykKLSAgICAgICAgRE9NT2JqZWN0IGdldENT U0NhbnZhc0NvbnRleHQoaW4gRE9NU3RyaW5nIGNvbnRleHRJZCwgaW4gRE9NU3RyaW5nIG5hbWUs IGluIGxvbmcgd2lkdGgsIGluIGxvbmcgaGVpZ2h0KTsKKyAgICAgICAgQ2FudmFzUmVuZGVyaW5n Q29udGV4dDJEIGdldENTU0NhbnZhc0NvbnRleHQoaW4gRE9NU3RyaW5nIGNvbnRleHRJZCwgaW4g RE9NU3RyaW5nIG5hbWUsIGluIGxvbmcgd2lkdGgsIGluIGxvbmcgaGVpZ2h0KTsKICNlbmRpZgog I2VuZGlmCiAKSW5kZXg6IGh0bWwvSFRNTENhbnZhc0VsZW1lbnQuaWRsCj09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0t IGh0bWwvSFRNTENhbnZhc0VsZW1lbnQuaWRsCShyZXZpc2lvbiA0MDczNCkKKysrIGh0bWwvSFRN TENhbnZhc0VsZW1lbnQuaWRsCSh3b3JraW5nIGNvcHkpCkBAIC0zOCw3ICszOCw3IEBAIG1vZHVs ZSBodG1sIHsKICAgICAgICAgICAgIHJhaXNlcyhET01FeGNlcHRpb24pOwogCiAjaWYgIWRlZmlu ZWQoTEFOR1VBR0VfT0JKRUNUSVZFX0MpCi0gICAgICAgIERPTU9iamVjdCBnZXRDb250ZXh0KGlu IERPTVN0cmluZyBjb250ZXh0SWQpOworICAgICAgICBDYW52YXNSZW5kZXJpbmdDb250ZXh0MkQg Z2V0Q29udGV4dChpbiBET01TdHJpbmcgY29udGV4dElkKTsKICNlbmRpZgogCiAgICAgfTsK