[webkit-reviews] review denied: [Bug 49350] bufferData/bufferSubData should not crash with null data input : [Attachment 73660] revised patch: responding to spec change

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has denied Zhenyao Mo <zmo at google.com>'s
request for review:
Bug 49350: bufferData/bufferSubData should not crash with null data input
https://bugs.webkit.org/show_bug.cgi?id=49350

Attachment 73660: revised patch: responding to spec change
https://bugs.webkit.org/attachment.cgi?id=73660&action=review

------- Additional Comments from Kenneth Russell <kbr at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=73660&action=review

> WebCore/html/canvas/WebGLRenderingContext.cpp:475
> +    if (!data) {
> +	   m_context->synthesizeGLError(GraphicsContext3D::INVALID_VALUE);
> +	   return;
> +    }

The bufferSubData variants shouldn't generate INVALID_VALUE, only the
bufferData variants. The bufferSubData variants can safely return early.

> WebCore/html/canvas/WebGLRenderingContext.cpp:498
> +    if (!data) {
> +	   m_context->synthesizeGLError(GraphicsContext3D::INVALID_VALUE);
> +	   return;
> +    }

See above.

> LayoutTests/fast/canvas/webgl/buffer-data-array-buffer.html:26
> +glErrorShouldBe(gl, gl.INVALID_OPERATION);

Unfortunately per discussion on the public_webgl list I do not think we can
mandate this behavior in the Khronos conformance suite. The overloading
behavior in Web IDL is ambiguous in this case
(http://dev.w3.org/2006/webapi/WebIDL/Overview.html#call) and Firefox does
something different than WebKit. You could require that the GL error is either
INVALID_OPERATION or NO_ERROR (and, implicitly, not a crash).