Attachment 169460 Details for Bug 98972 – Patch for Landing

[patch] Patch for Landing

98972-6.patch (text/plain), 14.59 KB, created by Max Vujovic on 2012-10-18 13:38:27 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Max Vujovic

Created: 2012-10-18 13:38:27 PDT

Size: 14.59 KB

patch

obsolete

>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index c7a73b1..65ae3b8 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,23 @@
>+2012-10-18  Max Vujovic  <mvujovic@adobe.com>
>+
>+        [CSS Shaders] Validate types of built-in vertex attributes
>+        https://bugs.webkit.org/show_bug.cgi?id=98972
>+
>+        Reviewed by Dean Jackson.
>+
>+        invalid-custom-filter-attribute-types.html uses a set of vertex shaders. Each vertex shader
>+        defines a specific attribute with the wrong type. None of the vertex shaders in the test
>+        should execute. If a vertex shader does execute, its associated fragment shader will render
>+        a green div as red.
>+
>+        * css3/filters/custom/invalid-custom-filter-attribute-types-expected.html: Added.
>+        * css3/filters/custom/invalid-custom-filter-attribute-types.html: Added.
>+        * css3/filters/resources/invalid-type-a-mesh-coord.vs: Added.
>+        * css3/filters/resources/invalid-type-a-position.vs: Added.
>+        * css3/filters/resources/invalid-type-a-tex-coord.vs: Added.
>+        * css3/filters/resources/invalid-type-a-triangle-coord.vs: Added.
>+        * css3/filters/resources/invalid-type-attribute-array.vs: Added.
>+
> 2012-10-16  Emil A Eklund  <eae@chromium.org>
> 
>         Remove special-case flooring of baselinePosition for replaced elements in InlineFlowBox::placeBoxesInBlockDirection
>diff --git a/LayoutTests/css3/filters/custom/invalid-custom-filter-attribute-types-expected.html b/LayoutTests/css3/filters/custom/invalid-custom-filter-attribute-types-expected.html
>new file mode 100644
>index 0000000..6790fde
>--- /dev/null
>+++ b/LayoutTests/css3/filters/custom/invalid-custom-filter-attribute-types-expected.html
>@@ -0,0 +1,25 @@
>+<!doctype html>
>+<html>
>+<head>
>+    <title>Tests that custom filters do not execute if the author defines built-in attributes with the wrong type.</title>
>+    
>+    <style>
>+    div {
>+        background-color: rgb(0, 255, 0);
>+        width: 50px;
>+        height: 50px;
>+        margin: 10px;
>+    }
>+    </style>
>+</head>
>+<body>
>+    <div></div>
>+    <div></div>
>+    <div></div>
>+    <div></div>
>+    <div></div>
>+</body>
>+</html>
>diff --git a/LayoutTests/css3/filters/custom/invalid-custom-filter-attribute-types.html b/LayoutTests/css3/filters/custom/invalid-custom-filter-attribute-types.html
>new file mode 100644
>index 0000000..a6aa054
>--- /dev/null
>+++ b/LayoutTests/css3/filters/custom/invalid-custom-filter-attribute-types.html
>@@ -0,0 +1,52 @@
>+<!doctype html>
>+<html>
>+<head>
>+    <title>Tests that custom filters do not execute if the author defines built-in attributes with the wrong type.</title>
>+    
>+    <script>
>+        if (window.testRunner) {
>+            window.testRunner.overridePreference("WebKitCSSCustomFilterEnabled", "1");
>+            window.testRunner.overridePreference("WebKitWebGLEnabled", "1");
>+            window.testRunner.waitUntilDone();
>+        }
>+        
>+        function runTest()
>+        {
>+            // We need to run the tests after the shaders download.
>+            if (window.testRunner)
>+                window.testRunner.notifyDone();
>+        }
>+    </script>
>+    <style>
>+    div {
>+        /* If the shaders execute, they will turn the element's color from green to red. */
>+        background-color: rgb(0, 255, 0);
>+        width: 50px;
>+        height: 50px;
>+        margin: 10px;
>+    }
>+    .invalid-type-a-mesh-coord {
>+        -webkit-filter: custom(url('../resources/invalid-type-a-mesh-coord.vs') mix(url('../resources/mix-color.fs') normal source-atop), mix_color 1.0 0.0 0.0 1.0);
>+    }
>+    .invalid-type-a-position {
>+        -webkit-filter: custom(url('../resources/invalid-type-a-position.vs') mix(url('../resources/mix-color.fs') normal source-atop), mix_color 1.0 0.0 0.0 1.0);
>+    }
>+    .invalid-type-a-tex-coord {
>+        -webkit-filter: custom(url('../resources/invalid-type-a-tex-coord.vs') mix(url('../resources/mix-color.fs') normal source-atop), mix_color 1.0 0.0 0.0 1.0);
>+    }
>+    .invalid-type-a-triangle-coord {
>+        -webkit-filter: custom(url('../resources/invalid-type-a-triangle-coord.vs') mix(url('../resources/mix-color.fs') normal source-atop), mix_color 1.0 0.0 0.0 1.0);
>+    }
>+    .invalid-type-attribute-array {
>+        -webkit-filter: custom(url('../resources/invalid-type-attribute-array.vs') mix(url('../resources/mix-color.fs') normal source-atop), mix_color 1.0 0.0 0.0 1.0);        
>+    }
>+    </style>
>+</head>
>+<body onload="runTest()">
>+    <div class="invalid-type-a-mesh-coord"></div>
>+    <div class="invalid-type-a-position"></div>
>+    <div class="invalid-type-a-tex-coord"></div>
>+    <div class="invalid-type-a-triangle-coord"></div>
>+    <div class="invalid-type-attribute-array"></div>
>+</body>
>+</html>
>diff --git a/LayoutTests/css3/filters/resources/invalid-type-a-mesh-coord.vs b/LayoutTests/css3/filters/resources/invalid-type-a-mesh-coord.vs
>new file mode 100644
>index 0000000..3ade9c8
>--- /dev/null
>+++ b/LayoutTests/css3/filters/resources/invalid-type-a-mesh-coord.vs
>@@ -0,0 +1,15 @@
>+// If this shader's related test passes, the custom filter does not execute because a_meshCoord's type is invalid.
>+
>+precision mediump float;
>+
>+// a_meshCoord should be a vec2, not a float.
>+attribute float a_meshCoord;
>+
>+attribute vec4 a_position;
>+
>+uniform mat4 u_projectionMatrix;
>+
>+void main()
>+{
>+    gl_Position = u_projectionMatrix * a_position;
>+}
>diff --git a/LayoutTests/css3/filters/resources/invalid-type-a-position.vs b/LayoutTests/css3/filters/resources/invalid-type-a-position.vs
>new file mode 100644
>index 0000000..bda48e8
>--- /dev/null
>+++ b/LayoutTests/css3/filters/resources/invalid-type-a-position.vs
>@@ -0,0 +1,13 @@
>+// If this shader's related test passes, the custom filter does not execute because a_position's type is invalid.
>+
>+precision mediump float;
>+
>+// a_position should be a vec4, not a vec3.
>+attribute vec3 a_position;
>+
>+uniform mat4 u_projectionMatrix;
>+
>+void main()
>+{
>+    gl_Position = u_projectionMatrix * vec4(a_position, 1.0);
>+}
>diff --git a/LayoutTests/css3/filters/resources/invalid-type-a-tex-coord.vs b/LayoutTests/css3/filters/resources/invalid-type-a-tex-coord.vs
>new file mode 100644
>index 0000000..7611331
>--- /dev/null
>+++ b/LayoutTests/css3/filters/resources/invalid-type-a-tex-coord.vs
>@@ -0,0 +1,15 @@
>+// If this shader's related test passes, the custom filter does not execute because a_texCoord's type is invalid.
>+
>+precision mediump float;
>+
>+// a_meshCoord should be a vec2, not an vec4.
>+attribute vec4 a_texCoord;
>+
>+attribute vec4 a_position;
>+
>+uniform mat4 u_projectionMatrix;
>+
>+void main()
>+{
>+    gl_Position = u_projectionMatrix * a_position;
>+}
>diff --git a/LayoutTests/css3/filters/resources/invalid-type-a-triangle-coord.vs b/LayoutTests/css3/filters/resources/invalid-type-a-triangle-coord.vs
>new file mode 100644
>index 0000000..85c3ab5
>--- /dev/null
>+++ b/LayoutTests/css3/filters/resources/invalid-type-a-triangle-coord.vs
>@@ -0,0 +1,15 @@
>+// If this shader's related test passes, the custom filter does not execute because a_triangleCoord's type is invalid.
>+
>+precision mediump float;
>+
>+// a_meshCoord should be a vec3, not a vec2.
>+attribute vec2 a_triangleCoord;
>+
>+attribute vec4 a_position;
>+
>+uniform mat4 u_projectionMatrix;
>+
>+void main()
>+{
>+    gl_Position = u_projectionMatrix * a_position;
>+}
>diff --git a/LayoutTests/css3/filters/resources/invalid-type-attribute-array.vs b/LayoutTests/css3/filters/resources/invalid-type-attribute-array.vs
>new file mode 100644
>index 0000000..37c4221
>--- /dev/null
>+++ b/LayoutTests/css3/filters/resources/invalid-type-attribute-array.vs
>@@ -0,0 +1,13 @@
>+// If this shader's related test passes, the custom filter does not execute because a_position's type is invalid.
>+
>+precision mediump float;
>+
>+// a_position should be a vec4, not a vec4 array.
>+attribute vec4 a_position[1];
>+
>+uniform mat4 u_projectionMatrix;
>+
>+void main()
>+{
>+    gl_Position = u_projectionMatrix * a_position[0];
>+}
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index cc345fa..54e5cab 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,33 @@
>+2012-10-18  Max Vujovic  <mvujovic@adobe.com>
>+
>+        [CSS Shaders] Validate types of built-in vertex attributes
>+        https://bugs.webkit.org/show_bug.cgi?id=98972
>+
>+        Reviewed by Dean Jackson.
>+
>+        Reject custom filters in which the author defined built-in attributes with the wrong type.
>+        For example, the GLSL declaration "attribute float a_position" is incorrect because
>+        a_position should be a vec4, not a float.
>+
>+        Test: css3/filters/custom/invalid-custom-filter-attribute-types.html
>+
>+        * platform/graphics/ANGLEWebKitBridge.h:
>+        (WebCore::ANGLEShaderSymbol::isSampler):
>+            Add const qualifier to isSampler method.
>+        * platform/graphics/filters/CustomFilterValidatedProgram.cpp:
>+        (WebCore):
>+        (WebCore::builtInAttributeNameToTypeMap):
>+            New file static function. Returns a map of the CSS Custom Filters built-in attribute
>+            names and their expected types.
>+        (WebCore::validateSymbols):
>+            New file static function. Loop through all of the symbols. Reject the shader if we find
>+            a built-in attribute defined with the wrong type.
>+        (WebCore::CustomFilterValidatedProgram::CustomFilterValidatedProgram):
>+            Call the new validateSymbols function. If it returns false, exit the constructor early,
>+            which rejects the program.
>+            Move the loop that checks if any uniform is a sampler into the the validateSymbols
>+            function.
>+
> 2012-10-16  Emil A Eklund  <eae@chromium.org>
> 
>         Remove special-case flooring of baselinePosition for replaced elements in InlineFlowBox::placeBoxesInBlockDirection
>diff --git a/Source/WebCore/platform/graphics/ANGLEWebKitBridge.h b/Source/WebCore/platform/graphics/ANGLEWebKitBridge.h
>index 178d3bd..5c5098e 100644
>--- a/Source/WebCore/platform/graphics/ANGLEWebKitBridge.h
>+++ b/Source/WebCore/platform/graphics/ANGLEWebKitBridge.h
>@@ -54,7 +54,7 @@ struct ANGLEShaderSymbol {
>     ShDataType dataType;
>     int size;
> 
>-    bool isSampler()
>+    bool isSampler() const
>     {
>         return symbolType == SHADER_SYMBOL_TYPE_UNIFORM
>             && (dataType == SH_SAMPLER_2D
>diff --git a/Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp b/Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp
>index 85ddda5..26e8b7e 100644
>--- a/Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp
>+++ b/Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp
>@@ -37,12 +37,70 @@
> #include "CustomFilterGlobalContext.h"
> #include "CustomFilterProgramInfo.h"
> #include "NotImplemented.h"
>+#include <wtf/HashMap.h>
> #include <wtf/text/StringBuilder.h>
>+#include <wtf/text/StringHash.h>
> 
> namespace WebCore {
> 
> #define SHADER(Src) (#Src) 
> 
>+// FIXME: Reuse this type when we validate the types of built-in uniforms.
>+// https://bugs.webkit.org/show_bug.cgi?id=98974
>+typedef HashMap<String, ShDataType> SymbolNameToTypeMap;
>+
>+static SymbolNameToTypeMap* builtInAttributeNameToTypeMap()
>+{
>+    static SymbolNameToTypeMap* nameToTypeMap = 0;
>+    if (!nameToTypeMap) {
>+        nameToTypeMap = new SymbolNameToTypeMap;        
>+        nameToTypeMap->set("a_meshCoord", SH_FLOAT_VEC2);
>+        nameToTypeMap->set("a_position", SH_FLOAT_VEC4);
>+        nameToTypeMap->set("a_texCoord", SH_FLOAT_VEC2);
>+        nameToTypeMap->set("a_triangleCoord", SH_FLOAT_VEC3);
>+    }
>+    return nameToTypeMap;
>+}
>+
>+static bool validateSymbols(const Vector<ANGLEShaderSymbol>& symbols)
>+{
>+    for (size_t i = 0; i < symbols.size(); ++i) {
>+        const ANGLEShaderSymbol& symbol = symbols[i];
>+        switch (symbol.symbolType) {
>+        case SHADER_SYMBOL_TYPE_ATTRIBUTE: {
>+            SymbolNameToTypeMap* attributeNameToTypeMap = builtInAttributeNameToTypeMap();
>+            SymbolNameToTypeMap::iterator builtInAttribute = attributeNameToTypeMap->find(symbol.name);
>+            if (builtInAttribute != attributeNameToTypeMap->end() && symbol.dataType != builtInAttribute->value) {
>+                // The author defined one of the built-in attributes with the wrong type.
>+                return false;
>+            }
>+
>+            // FIXME: Return false when the attribute is not one of the built-in attributes.
>+            // https://bugs.webkit.org/show_bug.cgi?id=98973
>+            break;
>+        }
>+        case SHADER_SYMBOL_TYPE_UNIFORM:
>+            if (symbol.isSampler()) {
>+                // FIXME: For now, we restrict shaders with any sampler defined.
>+                // When we implement texture parameters, we will allow shaders whose samplers are bound to valid textures.
>+                // We must not allow OpenGL to give unbound samplers a default value of 0 because that references the element texture,
>+                // which should be inaccessible to the author's shader code.
>+                // https://bugs.webkit.org/show_bug.cgi?id=96230
>+                return false;
>+            }
>+
>+            // FIXME: Validate the types of built-in uniforms.
>+            // https://bugs.webkit.org/show_bug.cgi?id=98974
>+            break;
>+        default:
>+            ASSERT_NOT_REACHED();
>+            break;
>+        }
>+    }
>+
>+    return true;
>+}
>+
> String CustomFilterValidatedProgram::defaultVertexShaderString()
> {
>     DEFINE_STATIC_LOCAL(String, vertexShaderString, (ASCIILiteral(SHADER(
>@@ -95,16 +153,10 @@ CustomFilterValidatedProgram::CustomFilterValidatedProgram(CustomFilterGlobalCon
>         return;
>     }
> 
>-    // Validate the author's samplers.
>-    for (Vector<ANGLEShaderSymbol>::iterator it = symbols.begin(); it != symbols.end(); ++it) {
>-        if (it->isSampler()) {
>-            // FIXME: For now, we restrict shaders with any sampler defined.
>-            // When we implement texture parameters, we will allow shaders whose samplers are bound to valid textures.
>-            // We must not allow OpenGL to give unbound samplers a default value of 0 because that references the DOM element texture,
>-            // which should be inaccessible to the author's shader code.
>-            // https://bugs.webkit.org/show_bug.cgi?id=96230
>-            return;
>-        }
>+    if (!validateSymbols(symbols)) {
>+        // FIXME: Report validation errors.
>+        // https://bugs.webkit.org/show_bug.cgi?id=74416
>+        return;
>     }
> 
>     // We need to add texture access, blending, and compositing code to shaders that are referenced from the CSS mix function.

Actions: View | Formatted Diff | Diff

Attachments on bug 98972: 168447 | 168464 | 169037 | 169246 | 169286 | 169460