Clean up WebGL parameter functions

[dzbarsky]

Do we have tests for WebGL?
Trying to get http://learningwebgl.com/lessons/lesson01/index.html running but we're still missing some things.

[jdm]

We have some tests (there's webgl/ under WPT and webg/ under the mozilla/ directory), but most are disabled due to #7931.

[jdm]

@ecoal95 Want to review this one? :)

[emilio]

@jdm Yep! :)

[emilio]

Looks good to me, @bzbarsky ping me when gleam is updated and I'll r+ this :)

-S-awaiting-review +S-blocked-on-external

---

Review status: 0 of 6 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

components/canvas/webgl_paint_task.rs, line 343 [r1] (raw file):
Nice! These validations were done in script to avoid an ipc-channel roundtrip, but I think this way is better, since we only have the logic in one place, and we'd only get the speed-up in invalid cases anyways, so...

---

components/script/dom/webglrenderingcontext.rs, line 640 [r1] (raw file):
I've re-read the spec here, and even though there's no behaviour specified, both gecko and chromium generate an InvalidValue error when the program is null.

I think it would be nice to mimic this behaviour where null values are not expected. They're a few functions though, so this can be left as a followup if you want. I've filled #8753

---

Comments from the review on Reviewable.io

[bzbarsky]

@ecoal95 You have the wrong Zbarsky, I think.

[emilio]

@bzbarsky thanks! Sorry, I meant @dzbarsky.

[dzbarsky]

Review status: 0 of 6 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

components/canvas/webgl_paint_task.rs, line 343 [r1] (raw file):
Yep, that's what I figured.

---

components/script/dom/webglrenderingcontext.rs, line 640 [r1] (raw file):
If we do that we should also get the spec changed.

---

Comments from the review on Reviewable.io

[emilio]

Review status: 0 of 6 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

components/script/dom/webglrenderingcontext.rs, line 640 [r1] (raw file):
I don't think so, I think it conforms to the spec in the sense that WebGL's behaviour, unless specified otherwise, must much the behaviour of GLES 2.

In that sense, the reference for most of this functions (eg glGetProgramiv) have a line similar to the following:

GL_INVALID_OPERATION is generated if program does not refer to a program object.

I therefore think it's legit and spec-compliant (probably even required?) to acquire this behaviour.

---

Comments from the review on Reviewable.io

[emilio]

Review status: 0 of 6 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

components/script/dom/webglrenderingcontext.rs, line 640 [r1] (raw file):
s/must much/must match/ (sorry)

---

Comments from the review on Reviewable.io

[dzbarsky]

#8761 instead