Test getActiveUniforms PASS gl.getProgramParameter(program, gl.LINK_STATUS) is true PASS gl.getError() is gl.NO_ERROR FAIL typeof rowMajors[i] should be boolean. Was number. FAIL typeof rowMajors[i] should be boolean. Was number. FAIL typeof rowMajors[i] should be boolean. Was number. FAIL getActiveUniforms returned -454761244,-454761244,-454761244 instead of null for invalid pname enum: NO_ERROR

By spec [1], the getActiveUniforms return type will vary with different pname. So I change the return type to any and set correct gl error when the pname is not in the table. [1] https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7.16

Comment on attachment 8704618 [details] [diff] [review] Fix getActiveUniforms return type Review of attachment 8704618 [details] [diff] [review]: ----------------------------------------------------------------- Does this JS look right? ::: dom/canvas/WebGL2Context.h @@ +360,5 @@ > void GetUniformIndices(WebGLProgram* program, > const dom::Sequence<nsString>& uniformNames, > dom::Nullable< nsTArray<GLuint> >& retval); > + void GetActiveUniforms(JSContext* cx, > + WebGLProgram* program, Fix the indents here. ::: dom/canvas/WebGLContext.h @@ +1287,5 @@ > uint32_t* const out_width, uint32_t* const out_height); > > bool ValidateQueryEnum(GLenum pname, const char* info); > > + bool ValidateUniformEnum(GLenum pname, const char* info); Why does this need to be a member of WebGLContext, and not just a static that takes a WebGLContext?

Comment on attachment 8704618 [details] [diff] [review] Fix getActiveUniforms return type Review of attachment 8704618 [details] [diff] [review]: ----------------------------------------------------------------- All the JSAPI stuff looks fine to me.

Comment on attachment 8704618 [details] [diff] [review] Fix getActiveUniforms return type Review of attachment 8704618 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/canvas/WebGL2Context.h @@ +360,5 @@ > void GetUniformIndices(WebGLProgram* program, > const dom::Sequence<nsString>& uniformNames, > dom::Nullable< nsTArray<GLuint> >& retval); > + void GetActiveUniforms(JSContext* cx, > + WebGLProgram* program, Okay. ::: dom/canvas/WebGLContext.h @@ +1287,5 @@ > uint32_t* const out_width, uint32_t* const out_height); > > bool ValidateQueryEnum(GLenum pname, const char* info); > > + bool ValidateUniformEnum(GLenum pname, const char* info); Originally I thought it's some kind of rule to put all ValidateSomething here. I will move this to WebGL2ContextUniforms.cpp as a static function.

apply jgilbert's comments.

try server result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6ad28c837a74

this needs dom peer review

Comment on attachment 8705487 [details] [diff] [review] Fix getActiveUniforms return type (carry r+: jgilbert)(carry r+: efaust) Could you help review WebIDL changes? Thanks.

Comment on attachment 8705487 [details] [diff] [review] Fix getActiveUniforms return type (carry r+: jgilbert)(carry r+: efaust) >+ JS::Rooted<JSObject*> array(cx, JS_NewArrayObject(cx, count)); >+ if (!array) { >+ return; >+ } >+ >+ switch (pname) { >+ case LOCAL_GL_UNIFORM_TYPE: >+ case LOCAL_GL_UNIFORM_SIZE: >+ case LOCAL_GL_UNIFORM_BLOCK_INDEX: >+ case LOCAL_GL_UNIFORM_OFFSET: >+ case LOCAL_GL_UNIFORM_ARRAY_STRIDE: >+ case LOCAL_GL_UNIFORM_MATRIX_STRIDE: >+ for (uint32_t i = 0; i < count; ++i) { >+ JS::RootedValue value(cx); >+ value = JS::Int32Value(samples[i]); >+ if (!JS_DefineElement(cx, array, i, value, JSPROP_ENUMERATE)) { >+ return; >+ } >+ } >+ break; >+ case LOCAL_GL_UNIFORM_IS_ROW_MAJOR: >+ for (uint32_t i = 0; i < count; ++i) { >+ JS::RootedValue value(cx); >+ value = JS::BooleanValue(samples[i]); >+ if (!JS_DefineElement(cx, array, i, value, JSPROP_ENUMERATE)) { >+ return; >+ } Sad to see so much JSAPI usage here. But I guess ToJSValue wouldn't have worked here since samples contains different type of values than what LOCAL_GL_UNIFORM_IS_ROW_MAJOR expects for the js array. r+ to the .webidl

caused bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=20551706&repo=mozilla-inbound and so got backedout

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/2f509b7156c7

Fix the warning problem.

try server: https://treeherder.mozilla.org/#/jobs?repo=try&revision=37e3163614e4

Transfer the patch to hg format.