573541 - More enums validation and fixes

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

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

Attachment: Lots of enum fixes

This patch fixes more enum-related issues.

* Fix the last problematic pnames in GetParameter
* validate the enums in various functions in which we still weren't doing that.

Comment 1

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

Attachment: Lots of enum fixes

sorry, the previous patch was mistakenly removing a function, i forgot to do a "hg qref".

Comment 2

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

Comment on attachment 452846 [details] [diff] [review]
Lots of enum fixes

This looks good, though I wonder if we should just define these in the header directly so that we can be pretty certain that they get inlined.

Also, one style comment:


>+    if(!ValidateTextureTargetEnum(target))
>+        return ErrorInvalidEnum("GenerateMipmap: invalid target");

Well ok, two style comments -- missing a space, just do a search for "if(" :-)


>+NS_IMETHODIMP
>+WebGLContext::StencilOp(WebGLenum sfail, WebGLenum dpfail, WebGLenum dppass)
>+{
>+    if (!ValidateStencilOpEnum(sfail)
>+     || !ValidateStencilOpEnum(dpfail)
>+     || !ValidateStencilOpEnum(dppass))
>+        return ErrorInvalidEnum("StencilOp: invalid action enum");


This is quite weird indentation -- just 4-space indent everything like normal.  I have no preference on || before or after the previous line, either's fine, but with a multiline if condition, the statement should always have { } around it.

Comment 3

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

Comment on attachment 452846 [details] [diff] [review]
Lots of enum fixes

argh, looks like I forgot to r+ this.. see above for comments, sorry!

Comment 4

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

Attachment: Lots of enum fixes, updated

No worries, it's just that I've been lazy. Also, I'm looking at adding a pre-"hg qrefresh" hook checking for "if(" ;-)

Comment 5

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

http://hg.mozilla.org/mozilla-central/rev/ef93c10b7d97