WebKit Bugzilla
Currently the constructor functions for types like WebGLBuffer and WebGLRenderBuffer are not exposed on the DOMWindow object. This presents problems when trying to determine the type of these objects. The JavaScript instanceof operator would support type checks against these objects, if the constructor functions were exposed on the DOMWindow under the appropriate type names. I haven't checked the Web IDL specification to understand whether support for these type checks is supposed to be implicit, in which case this would simply be a bug in WebKit's JavaScript bindings for WebGL. If not then this issue may need to be discussed with the WebGL working group.
Created attachment 76128 [details] patch
This worried me at first, because it seems to indicate that this change will allow you to construct and object with "new WebGLBuffer()", which would be anything from useless to very bad. But Ollie helped me understand that the word "constructor" in this context is a complete misnomer and these new objects will properly throw exceptions if something that is attempted. Still might be useful to create a test that attempts to create them and see that it properly fails.
Comment on attachment 76128 [details] patch As Chris points out the test really does need to ensure that attempts to call these constructors throw exceptions. r- for this reason. Looks good otherwise.
(In reply to comment #2) > This worried me at first, because it seems to indicate that this change will allow you to construct and object with "new WebGLBuffer()", which would be anything from useless to very bad. But Ollie helped me understand that the word "constructor" in this context is a complete misnomer and these new objects will properly throw exceptions if something that is attempted. > > Still might be useful to create a test that attempts to create them and see that it properly fails. CanBeConstructed needs to be in the idl interface and probably some extra hook-up if you wanna do new T(). I'll add the test cases.
Comment on attachment 76128 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=76128&action=review > WebCore/bindings/generic/RuntimeEnabledFeatures.h:122 > + static bool webGLUniformLocationEnabled() { return isWebGLEnabled; } Why are you not handling WebGLObject and WebGLContextAttributes here (and throughout the rest of the patch)? Also should obsolete objects like WebGLFloatArray be removed?
(In reply to comment #4) > (In reply to comment #2) > > This worried me at first, because it seems to indicate that this change will allow you to construct and object with "new WebGLBuffer()", which would be anything from useless to very bad. But Ollie helped me understand that the word "constructor" in this context is a complete misnomer and these new objects will properly throw exceptions if something that is attempted. > > > > Still might be useful to create a test that attempts to create them and see that it properly fails. > > CanBeConstructed needs to be in the idl interface and probably some extra hook-up if you wanna do new T(). Just to be clear, we definitely don't want to be able to do new T() on any of these objects. The tests are just to make sure that is the case.
Created attachment 76138 [details] Patch
Comment on attachment 76138 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=76138&action=review Looks good. One minor grammatical correction, fix on landing. > LayoutTests/fast/canvas/webgl/instanceof-test.html:67 > + testPassed('new ' + objectName + ' throw an error'); throw -> threw
(In reply to comment #5) > (From update of attachment 76128 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=76128&action=review > > > WebCore/bindings/generic/RuntimeEnabledFeatures.h:122 > > + static bool webGLUniformLocationEnabled() { return isWebGLEnabled; } > > Why are you not handling WebGLObject and WebGLContextAttributes here (and throughout the rest of the patch)? Also should obsolete objects like WebGLFloatArray be removed? WebGLObject is not exposed. As for WebGLContextAttributes and WebGLContextEvent, I am not sure if it's worth exposing the constructors. If yes, we can add them in a follow-up patch, so is the clean-up.
Committed r73669: <http://trac.webkit.org/changeset/73669>