569943 - Need to explicitly check Enable/Disable arguments

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

We need to explicitly check for valid arguments to Enable/Disable, instead of just passing them down to GL.

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Check Enable/Disable/IsEnabled enums

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 449705 [details] [diff] [review]
Check Enable/Disable/IsEnabled enums

Wow, are those really the only enable caps that are valid?  Thought there would be a lot more, crazy.

r+, with the following style changes:

>+NS_IMETHODIMP WebGLContext::Enable(WebGLuint cap)
>+{
>+    if (ValidateCapabilityEnum(cap)) {
>+        MakeContextCurrent();
>+        gl->fEnable(cap);
>+        return NS_OK;
>+    } else {
>+        return ErrorInvalidEnum("Enable: invalid capability enum");
>+    }
>+}

Treat if clauses as "exceptional" cases, and have the normal flow be the "good" path for readability, i.e.:

{
    if (!ValidateCapabilityEnum(cap))
        return ErrorInvalidEnum(...);

    MakeContextCurrent();
    fEnable(...);
    return NS_OK;
}

>+NS_IMETHODIMP WebGLContext::Disable(WebGLuint cap)
>+{
>+    if (ValidateCapabilityEnum(cap)) {
>+        MakeContextCurrent();
>+        gl->fDisable(cap);
>+        return NS_OK;
>+    } else {
>+        return ErrorInvalidEnum("Disable: invalid capability enum");
>+    }
>+}

Same.

> NS_IMETHODIMP
>-WebGLContext::IsEnabled(WebGLenum k, WebGLboolean *retval)
>+WebGLContext::IsEnabled(WebGLenum cap, WebGLboolean *retval)
> {
>-    MakeContextCurrent();
>-    *retval = gl->fIsEnabled(k);
>-    return NS_OK;
>+    if(ValidateCapabilityEnum(cap)) {
>+        MakeContextCurrent();
>+        *retval = gl->fIsEnabled(cap);
>+        return NS_OK;
>+    } else {
>+        *retval = 0; // as per the OpenGL ES spec
>+        return ErrorInvalidEnum("IsEnabled: invalid capability enum");
>+    }

Same.

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Check Enable/Disable/IsEnabled enums, v2

Here you go!

See here the list of enums:
http://www.khronos.org/opengles/sdk/2.0/docs/man/glIsEnabled.xml

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/mozilla-central/rev/048ddcc077d8