we should pass the test

This patch should fix the test.

Check the size of texture and do finalize attachment before getting parameter from attachment.

Comment on attachment 8709852 [details] [diff] [review] Fix framebuffer-test problem. Review of attachment 8709852 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/canvas/WebGLContextGL.cpp @@ +557,5 @@ > + * level must be greater than or equal to zero and no larger than > + * log2 of the value of MAX_TEXTURE_SIZE. Otherwise, an > + * INVALID_VALUE error is generated. > + */ > + if (IsWebGL2()) { There is an `if (!IsWebGL2() && level != 0) {` that should be merged into an `else` or `else if` attached to this `if` block. @@ +560,5 @@ > + */ > + if (IsWebGL2()) { > + if (textarget == LOCAL_GL_TEXTURE_2D) { > + if (uint32_t(level) > FloorLog2(mImplMaxTextureSize)) { > + ErrorInvalidValue("framebufferTexture2D: level must be 0."); The text here is wrong. "framebufferTexture2D: `level` is too large." is fine. @@ +563,5 @@ > + if (uint32_t(level) > FloorLog2(mImplMaxTextureSize)) { > + ErrorInvalidValue("framebufferTexture2D: level must be 0."); > + return; > + } > + } else { Assert that textarget is a cubemap TexImageTarget. @@ +564,5 @@ > + ErrorInvalidValue("framebufferTexture2D: level must be 0."); > + return; > + } > + } else { > + uint32_t cudeSize = MINVALUE_GL_MAX_CUBE_MAP_TEXTURE_SIZE; 'cube' not 'cude', but this should just be: const auto maxSize = mImplMaxCubeMapTextureSize;

Address jgilbert's comment.

Re-upload patch.

Comment on attachment 8710232 [details] [diff] [review] framebuffer-test problem. Review of attachment 8710232 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/canvas/WebGLContextGL.cpp @@ +560,5 @@ > + ErrorInvalidValue("framebufferTexture2D: level is too large."); > + return; > + } > + } else if (textarget >= LOCAL_GL_TEXTURE_CUBE_MAP_POSITIVE_X && > + textarget <= LOCAL_GL_TEXTURE_CUBE_MAP_NEGATIVE_Z) { You shouldn't need to check this at runtime. You should only need to assert it. textarget should have been validated in ValidateFramebufferTarget or elsewhere. If it's not, we need to validate it. This means this code may be incomplete, but it should be a strict improvement over the present code.

The textarget check is behind the size check. I move the textarget check position and use assertion when checking size. Please help review again.

Comment on attachment 8710281 [details] [diff] [review] fix framebuffer-test problem. Review of attachment 8710281 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/canvas/WebGLContextGL.cpp @@ +564,5 @@ > + */ > + > + if (textarget == LOCAL_GL_TEXTURE_2D) { > + if (uint32_t(level) > FloorLog2(mImplMaxTextureSize)) { > + ErrorInvalidValue("framebufferTexture2D: level is too large."); 4-space indents please

Address jgilbert's comments.

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