Validate internalformat/format/type for uploading DOM elements to texture

third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp

Index: third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
diff --git a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
index 70d9cf301bc3dc51a20b8a18a76ffbd420f4fdeb..a250922e25baa323f69c082a17b3456a2f2a34b0 100644
--- a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
+++ b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
@@ -774,6 +774,33 @@ static const GLenum kSupportedInternalFormatsTexImageES3[] = {
     GL_DEPTH32F_STENCIL8,
 };
 
+// ES3 enums supported by TexImageSource
+static const GLenum kSupportedInternalFormatsTexImageSourceES3[] = {
+    GL_R8,
+    GL_R16F,
+    GL_R32F,
+    GL_R8UI,
+    GL_RG8,
+    GL_RG16F,
+    GL_RG32F,
+    GL_RG8UI,
+    GL_RGB8,
+    GL_SRGB8,
+    GL_RGB565,
+    GL_R11F_G11F_B10F,
+    GL_RGB9_E5,
+    GL_RGB16F,
+    GL_RGB32F,
+    GL_RGB8UI,
+    GL_RGBA8,
+    GL_SRGB8_ALPHA8,
+    GL_RGB5_A1,
+    GL_RGBA4,
+    GL_RGBA16F,
+    GL_RGBA32F,
+    GL_RGBA8UI,
+};
+
 // ES2 enums
 static const GLenum kSupportedFormatsES2[] = {
     GL_RGB,
@@ -809,6 +836,18 @@ static const GLenum kSupportedFormatsES3[] = {
     GL_DEPTH_STENCIL,
 };
 
+// ES3 enums supported by TexImageSource
+static const GLenum kSupportedFormatsTexImageSourceES3[] = {
+    GL_RED,
+    GL_RED_INTEGER,
+    GL_RG,
+    GL_RG_INTEGER,
+    GL_RGB,
+    GL_RGB_INTEGER,
+    GL_RGBA,
+    GL_RGBA_INTEGER,
+};
+
 // ES2 enums
 static const GLenum kSupportedTypesES2[] = {
     GL_UNSIGNED_BYTE,
@@ -850,6 +889,17 @@ static const GLenum kSupportedTypesES3[] = {
     GL_FLOAT_32_UNSIGNED_INT_24_8_REV,
 };
 
+// ES3 enums supported by TexImageSource
+static const GLenum kSupportedTypesTexImageSourceES3[] = {
+    GL_UNSIGNED_SHORT,
+    GL_SHORT,
+    GL_UNSIGNED_INT,
+    GL_INT,

[Ken Russell (switch to Gerrit), 2016/10/01 03:52:05]

From the table for texImage2D taking TexImageSource in
https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7.6 it looks to me
like GL_UNSIGNED_SHORT and GL_SHORT, GL_UNSIGNED_INT and GL_INT are deliberately
excluded from this list. Can you double-check?

[qiankun, 2016/10/01 14:27:30]

Yes. These types are excluded. Thanks.

+    GL_HALF_FLOAT,
+    GL_FLOAT,
+    GL_UNSIGNED_INT_10F_11F_11F_REV,
+};
+
 bool isUnsignedIntegerFormat(GLenum internalformat)
 {
     switch (internalformat) {
@@ -953,6 +1003,7 @@ WebGLRenderingContextBase::WebGLRenderingContextBase(HTMLCanvasElement* passedCa
     , m_numGLErrorsToConsoleAllowed(maxGLErrorsAllowedToConsole)
     , m_onePlusMaxNonDefaultTextureUnit(0)
     , m_isWebGL2FormatsTypesAdded(false)
+    , m_isWebGL2TexImageSourceFormatsTypesAdded(false)
     , m_isWebGL2InternalFormatsCopyTexImageAdded(false)
     , m_isOESTextureFloatFormatsTypesAdded(false)
     , m_isOESTextureHalfFloatFormatsTypesAdded(false)
@@ -985,9 +1036,12 @@ WebGLRenderingContextBase::WebGLRenderingContextBase(HTMLCanvasElement* passedCa
     }
 
     ADD_VALUES_TO_SET(m_supportedInternalFormats, kSupportedInternalFormatsES2);
+    ADD_VALUES_TO_SET(m_supportedTexImageSourceInternalFormats, kSupportedInternalFormatsES2);

[Ken Russell (switch to Gerrit), 2016/10/01 03:52:05]

Minor nit: kSupportedInternalFormatsES2 is redundant because ES2 required that
format==internalformat. It could be removed, and replaced everywhere with
kSupportedFormatsES2.

[qiankun, 2016/10/01 14:27:30]

Right, they are same. Removed kSupportedInternalFormatsES2.

     ADD_VALUES_TO_SET(m_supportedInternalFormatsCopyTexImage, kSupportedInternalFormatsES2);
     ADD_VALUES_TO_SET(m_supportedFormats, kSupportedFormatsES2);
+    ADD_VALUES_TO_SET(m_supportedTexImageSourceFormats, kSupportedFormatsES2);
     ADD_VALUES_TO_SET(m_supportedTypes, kSupportedTypesES2);
+    ADD_VALUES_TO_SET(m_supportedTexImageSourceTypes, kSupportedTypesES2);
 }
 
 PassRefPtr<DrawingBuffer> WebGLRenderingContextBase::createDrawingBuffer(std::unique_ptr<WebGraphicsContext3DProvider> contextProvider)
@@ -1120,6 +1174,7 @@ void WebGLRenderingContextBase::initializeNewContext()
         m_extensionEnabled[i] = false;
 
     m_isWebGL2FormatsTypesAdded = false;
+    m_isWebGL2TexImageSourceFormatsTypesAdded = false;
     m_isWebGL2InternalFormatsCopyTexImageAdded = false;
     m_isOESTextureFloatFormatsTypesAdded = false;
     m_isOESTextureHalfFloatFormatsTypesAdded = false;
@@ -1128,12 +1183,18 @@ void WebGLRenderingContextBase::initializeNewContext()
 
     m_supportedInternalFormats.clear();
     ADD_VALUES_TO_SET(m_supportedInternalFormats, kSupportedInternalFormatsES2);
+    m_supportedTexImageSourceInternalFormats.clear();
+    ADD_VALUES_TO_SET(m_supportedTexImageSourceInternalFormats, kSupportedInternalFormatsES2);
     m_supportedInternalFormatsCopyTexImage.clear();
     ADD_VALUES_TO_SET(m_supportedInternalFormatsCopyTexImage, kSupportedInternalFormatsES2);
     m_supportedFormats.clear();
     ADD_VALUES_TO_SET(m_supportedFormats, kSupportedFormatsES2);
+    m_supportedTexImageSourceFormats.clear();
+    ADD_VALUES_TO_SET(m_supportedTexImageSourceFormats, kSupportedFormatsES2);
     m_supportedTypes.clear();
     ADD_VALUES_TO_SET(m_supportedTypes, kSupportedTypesES2);
+    m_supportedTexImageSourceTypes.clear();
+    ADD_VALUES_TO_SET(m_supportedTexImageSourceTypes, kSupportedTypesES2);
 
     activateContext(this);
 }
@@ -4030,7 +4091,7 @@ bool WebGLRenderingContextBase::validateTexFunc(const char* functionName, TexIma
     if (!validateTexFuncLevel(functionName, target, level))
         return false;
 
-    if (!validateTexFuncParameters(functionName, functionType, target, level, internalformat, width, height, depth, border, format, type))
+    if (!validateTexFuncParameters(functionName, functionType, sourceType, target, level, internalformat, width, height, depth, border, format, type))
         return false;
 
     if (functionType == TexSubImage) {
@@ -5526,6 +5587,72 @@ bool WebGLRenderingContextBase::validateShaderSource(const String& string)
     return true;
 }
 
+void WebGLRenderingContextBase::addExtensionSupportedFormatsTypes()
+{
+    if (!m_isOESTextureFloatFormatsTypesAdded && extensionEnabled(OESTextureFloatName)) {
+        ADD_VALUES_TO_SET(m_supportedTypes, kSupportedTypesOESTexFloat);
+        ADD_VALUES_TO_SET(m_supportedTexImageSourceTypes, kSupportedTypesOESTexFloat);
+        m_isOESTextureFloatFormatsTypesAdded = true;
+    }
+
+    if (!m_isOESTextureHalfFloatFormatsTypesAdded && extensionEnabled(OESTextureHalfFloatName)) {
+        ADD_VALUES_TO_SET(m_supportedTypes, kSupportedTypesOESTexHalfFloat);
+        ADD_VALUES_TO_SET(m_supportedTexImageSourceTypes, kSupportedTypesOESTexHalfFloat);
+        m_isOESTextureHalfFloatFormatsTypesAdded = true;
+    }
+
+    if (!m_isWebGLDepthTextureFormatsTypesAdded && extensionEnabled(WebGLDepthTextureName)) {
+        ADD_VALUES_TO_SET(m_supportedInternalFormats, kSupportedInternalFormatsOESDepthTex);
+        ADD_VALUES_TO_SET(m_supportedTexImageSourceInternalFormats, kSupportedInternalFormatsOESDepthTex);
+        ADD_VALUES_TO_SET(m_supportedFormats, kSupportedFormatsOESDepthTex);
+        ADD_VALUES_TO_SET(m_supportedTexImageSourceFormats, kSupportedFormatsOESDepthTex);
+        ADD_VALUES_TO_SET(m_supportedTypes, kSupportedTypesOESDepthTex);
+        ADD_VALUES_TO_SET(m_supportedTexImageSourceTypes, kSupportedTypesOESDepthTex);
+        m_isWebGLDepthTextureFormatsTypesAdded = true;
+    }
+
+    if (!m_isEXTsRGBFormatsTypesAdded && extensionEnabled(EXTsRGBName)) {
+        ADD_VALUES_TO_SET(m_supportedInternalFormats, kSupportedInternalFormatsEXTsRGB);
+        ADD_VALUES_TO_SET(m_supportedTexImageSourceInternalFormats, kSupportedInternalFormatsEXTsRGB);
+        ADD_VALUES_TO_SET(m_supportedFormats, kSupportedFormatsEXTsRGB);
+        ADD_VALUES_TO_SET(m_supportedTexImageSourceFormats, kSupportedFormatsEXTsRGB);
+        m_isEXTsRGBFormatsTypesAdded = true;
+    }
+}
+
+bool WebGLRenderingContextBase::validateTexImageSourceFormatAndType(const char* functionName, TexImageFunctionType functionType, GLenum internalformat, GLenum format, GLenum type)
+{
+    if (!m_isWebGL2TexImageSourceFormatsTypesAdded && isWebGL2OrHigher()) {
+        ADD_VALUES_TO_SET(m_supportedTexImageSourceInternalFormats, kSupportedInternalFormatsTexImageSourceES3);
+        ADD_VALUES_TO_SET(m_supportedTexImageSourceFormats, kSupportedFormatsTexImageSourceES3);
+        ADD_VALUES_TO_SET(m_supportedTexImageSourceTypes, kSupportedTypesTexImageSourceES3);
+        m_isWebGL2TexImageSourceFormatsTypesAdded = true;
+    }
+
+    if (!isWebGL2OrHigher()) {
+        addExtensionSupportedFormatsTypes();
+    }
+
+    if (internalformat != 0 && m_supportedTexImageSourceInternalFormats.find(internalformat) == m_supportedTexImageSourceInternalFormats.end()) {
+        if (functionType == TexImage) {
+            synthesizeGLError(GL_INVALID_VALUE, functionName, "invalid internalformat");
+        } else {
+            synthesizeGLError(GL_INVALID_ENUM, functionName, "invalid internalformat");
+        }
+        return false;
+    }
+    if (m_supportedTexImageSourceFormats.find(format) == m_supportedTexImageSourceFormats.end()) {
+        synthesizeGLError(GL_INVALID_ENUM, functionName, "invalid format");
+        return false;
+    }
+    if (m_supportedTexImageSourceTypes.find(type) == m_supportedTexImageSourceTypes.end()) {
+        synthesizeGLError(GL_INVALID_ENUM, functionName, "invalid type");
+        return false;
+    }

[Ken Russell (switch to Gerrit), 2016/10/01 03:52:05]

Only some of these internalformat/format/type combinations are valid; have you
added enough negative tests to ensure that the code in
Source/platform/graphics/gpu/WebGLImageConversion.cpp won't crash if an invalid
combination is specified? I'm thinking something like:

gl.texImage2D(gl.TEXTURE_2D, 0, gl.RG8, width, height, 0, gl.RG, gl.FLOAT,
image_element);

I think this will work OK -- the image conversion code seems to only look at
format and type, and the command buffer will catch the invalid internalformat
usage. I can't see any problems with the combinations listed in the
format/type/internalformat table, but wanted to ask.

[qiankun, 2016/10/01 14:27:30]

Thanks for catching this potential issue.
Code in WebGLImageConversion.cpp won't crash for (RG8, RG, FLOAT) combination.
Command buffer will catch the INVALID_OPERATION error.

I can add such negative case to conformance test: internalformat, format and
type are valid, but their combination is invalid.

+
+    return true;
+}
+
 bool WebGLRenderingContextBase::validateTexFuncFormatAndType(const char* functionName, TexImageFunctionType functionType, GLenum internalformat, GLenum format, GLenum type, GLint level)
 {
     if (!m_isWebGL2FormatsTypesAdded && isWebGL2OrHigher()) {
@@ -5537,28 +5664,7 @@ bool WebGLRenderingContextBase::validateTexFuncFormatAndType(const char* functio
     }
 
     if (!isWebGL2OrHigher()) {
-        if (!m_isOESTextureFloatFormatsTypesAdded && extensionEnabled(OESTextureFloatName)) {
-            ADD_VALUES_TO_SET(m_supportedTypes, kSupportedTypesOESTexFloat);
-            m_isOESTextureFloatFormatsTypesAdded = true;
-        }
-
-        if (!m_isOESTextureHalfFloatFormatsTypesAdded && extensionEnabled(OESTextureHalfFloatName)) {
-            ADD_VALUES_TO_SET(m_supportedTypes, kSupportedTypesOESTexHalfFloat);
-            m_isOESTextureHalfFloatFormatsTypesAdded = true;
-        }
-
-        if (!m_isWebGLDepthTextureFormatsTypesAdded && extensionEnabled(WebGLDepthTextureName)) {
-            ADD_VALUES_TO_SET(m_supportedInternalFormats, kSupportedInternalFormatsOESDepthTex);
-            ADD_VALUES_TO_SET(m_supportedFormats, kSupportedFormatsOESDepthTex);
-            ADD_VALUES_TO_SET(m_supportedTypes, kSupportedTypesOESDepthTex);
-            m_isWebGLDepthTextureFormatsTypesAdded = true;
-        }
-
-        if (!m_isEXTsRGBFormatsTypesAdded && extensionEnabled(EXTsRGBName)) {
-            ADD_VALUES_TO_SET(m_supportedInternalFormats, kSupportedInternalFormatsEXTsRGB);
-            ADD_VALUES_TO_SET(m_supportedFormats, kSupportedFormatsEXTsRGB);
-            m_isEXTsRGBFormatsTypesAdded = true;
-        }
+        addExtensionSupportedFormatsTypes();
     }
 
     if (internalformat != 0 && m_supportedInternalFormats.find(internalformat) == m_supportedInternalFormats.end()) {
@@ -5678,14 +5784,20 @@ bool WebGLRenderingContextBase::validateTexFuncDimensions(const char* functionNa
     return true;
 }
 
-bool WebGLRenderingContextBase::validateTexFuncParameters(const char* functionName, TexImageFunctionType functionType, GLenum target,
+bool WebGLRenderingContextBase::validateTexFuncParameters(const char* functionName, TexImageFunctionType functionType, TexFuncValidationSourceType sourceType, GLenum target,
     GLint level, GLenum internalformat, GLsizei width, GLsizei height, GLsizei depth, GLint border, GLenum format, GLenum type)
 {
     // We absolutely have to validate the format and type combination.
     // The texImage2D entry points taking HTMLImage, etc. will produce
     // temporary data based on this combination, so it must be legal.
-    if (!validateTexFuncFormatAndType(functionName, functionType, internalformat, format, type, level))
-        return false;
+    if (sourceType == SourceHTMLImageElement || sourceType == SourceHTMLCanvasElement || sourceType == SourceHTMLVideoElement
+        || sourceType == SourceImageData || sourceType == SourceImageBitmap) {
+        if (!validateTexImageSourceFormatAndType(functionName, functionType, internalformat, format, type))
+            return false;
+    } else {
+        if (!validateTexFuncFormatAndType(functionName, functionType, internalformat, format, type, level))
+            return false;
+    }
 
     if (!validateTexFuncDimensions(functionName, functionType, target, level, width, height, depth))
         return false;