Gregg Tavares points out that WebGLRenderingContext::drawElements currently generates an INVALID_OPERATION error if the UNSIGNED_INT type is passed in, because the index validation code catches the invalid enum but always causes INVALID_OPERATION to be generated. Per the OpenGL ES 2.0 spec, INVALID_ENUM should be generated in this case.
Several cases should return INVALID_VALUE instead of INVALID_OPERATION.
Created attachment 58315 [details] patch
Comment on attachment 58315 [details] patch Mostly looks good. A few issues. WebCore/html/canvas/WebGLRenderingContext.cpp:824 + if (!validateDrawMode(mode)) From recent emails on the public-webgl mailing list it sounds like they may want to strictly specify the order in which errors are returned if the inputs would generate multiple errors. For example, INVALID_ENUM would be preferred over INVALID_VALUE. Because of this, even though it may mean doing some redundant validation, I think we should remove the isGLES2Compliant() check here and always validate the draw mode. LayoutTests/fast/canvas/webgl/resources/webgl-test.js:110 + testFailed(evalStr + " expected: " + getGLErrorAsString(ctx, glError) + ". Was " + getGLErrorAsString(ctx, err) + "."); Can you say here "expected GL error:" and ". Generated: " for clarity? LayoutTests/fast/canvas/webgl/resources/webgl-test.js:112 + testPassed(evalStr + " was expected value: " + getGLErrorAsString(ctx, glError) + "."); Can you say here " generated expected GL error: " for clarity? LayoutTests/fast/canvas/webgl/draw-elements-out-of-bounds.html:44 + shouldGenerateGLError(context, context.INVALID_ENUM, "context.drawElements(0x0009, 3, context.UNSIGNED_INT, 0)"); // GL_POLYGON You should use UNSIGNED_BYTE here to make sure you're producing the error for the first enum error and not the second.
Created attachment 58421 [details] revised patch: responding to kbr's review
Comment on attachment 58421 [details] revised patch: responding to kbr's review Looks good.
Comment on attachment 58421 [details] revised patch: responding to kbr's review ok.
Comment on attachment 58421 [details] revised patch: responding to kbr's review Clearing flags on attachment: 58421 Committed r61017: <http://trac.webkit.org/changeset/61017>
All reviewed patches have been landed. Closing bug.