WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
36512
Expose constructor functions for instanceof checks of WebGL objects
https://bugs.webkit.org/show_bug.cgi?id=36512
Summary
Expose constructor functions for instanceof checks of WebGL objects
Kenneth Russell
Reported
2010-03-23 15:53:13 PDT
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.
Attachments
patch
(16.52 KB, patch)
2010-12-09 15:46 PST
,
Zhenyao Mo
no flags
Details
Formatted Diff
Diff
Patch
(17.84 KB, patch)
2010-12-09 17:08 PST
,
Zhenyao Mo
kbr
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Zhenyao Mo
Comment 1
2010-12-09 15:46:30 PST
Created
attachment 76128
[details]
patch
Chris Marrin
Comment 2
2010-12-09 16:27:42 PST
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.
Kenneth Russell
Comment 3
2010-12-09 16:34:23 PST
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.
Zhenyao Mo
Comment 4
2010-12-09 16:36:06 PST
(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.
Chris Marrin
Comment 5
2010-12-09 16:36:16 PST
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?
Chris Marrin
Comment 6
2010-12-09 16:37:38 PST
(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.
Zhenyao Mo
Comment 7
2010-12-09 17:08:15 PST
Created
attachment 76138
[details]
Patch
Kenneth Russell
Comment 8
2010-12-09 17:15:24 PST
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
Zhenyao Mo
Comment 9
2010-12-09 17:55:22 PST
(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.
Zhenyao Mo
Comment 10
2010-12-09 17:59:12 PST
Committed
r73669
: <
http://trac.webkit.org/changeset/73669
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug