Bug 171961 – WebGLRenderingContext should implement WebGLRenderingContextBase

• Component: WebGL
• Status: RESOLVED FIXED

[Dean Jackson]

WebGLRenderingContext should implement WebGLRenderingContextBase

[Radar WebKit Bug Importer]

<rdar://problem/32124920>

[Dean Jackson]

Created attachment 309678 [details]
Patch

[Sam Weinig]

Comment on attachment 309678 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=309678&action=review

Looks good!

> Source/WebCore/bindings/js/JSWebGLRenderingContextBaseCustom.cpp:75
>  void JSWebGLRenderingContextBase::visitAdditionalChildren(SlotVisitor& visitor)
>  {
>      visitor.addOpaqueRoot(&wrapped());
> -    visitor.addOpaqueRoot(root(wrapped().canvas()));
> +    visitor.addOpaqueRoot(WebCore::root(wrapped().canvas()));

I don't think this file (and the generated ones) needs to be in the project at all anymore.  No one should ever be instantiating or referring to a JSWebGLRenderingContextBase.  For an example of an interface that is just used for "implements", see ChildNode.idl.  Perhaps we should have an extended IDL that means this is not a real interface.

[Dean Jackson]

Created attachment 309773 [details]
Patch

[Sam Weinig]

Comment on attachment 309773 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=309773&action=review

> Source/WebCore/bindings/js/JSDocumentCustom.cpp:160
> +    if (is<WebGL2RenderingContext>(*context))
> +        return toJS(&state, globalObject(), downcast<WebGL2RenderingContext>(*context));

Should this be in a WEBGL2 #ifdef? Do we need a WEBGL2 #ifdef at all?

> Source/WebCore/bindings/js/JSHTMLCanvasElementCustom.cpp:74
> +            if (is<WebGL2RenderingContext>(*context))
> +                return toJS<IDLNullable<IDLInterface<WebGL2RenderingContext>>>(state, *globalObject(), static_cast<WebGL2RenderingContext*>(context));

Should this be in a WEBGL2 #ifdef? Do we need a WEBGL2 #ifdef at all?

> Source/WebCore/html/canvas/WebGL2RenderingContext.idl:340
>      // WebGL2:

This comment can probably also go.

[Dean Jackson]

Created attachment 309785 [details]
Patch

[Dean Jackson]

Committed r216695: <http://trac.webkit.org/changeset/216695>