896254 - WebGL2RenderingContext WebIDL interface should be disabled on release channels

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Per our prefixing policy, we should not expose window.WebGL2RenderingContext on the release channels.

Comment 1

[Guillaume Abadie]

Attachment: patch revision 1

Comment 2

[Guillaume Abadie]

https://tbpl.mozilla.org/?tree=Try&rev=78766ae0ca1f

Comment 3

[Guillaume Abadie]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fc9bfbd308d1

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/fc9bfbd308d1

Comment 5

[Guillaume Abadie]

The patch has been done by the wrong way.

Comment 6

[(no longer active)]

So the correct way to do this is to include the .webidl files in all builds, and hide all of WebGL2 interfaces behind a pref, like this:

[Pref="foo.bar.webgl2]
interface WebGL2FooBar {
  // ...
};

Then, these interfaces will only be exposed if this pref is set, which won't be the case for our users anywhere except where they explicitly opt in to test this feature.  You can see an example of this in Gamepad.webidl, among others.

Comment 7

[Guillaume Abadie]

Attachment: new way to solve the problem's patch

Here is the new way to solve this bug.

Comment 8

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 782891 [details] [diff] [review]
new way to solve the problem's patch

Review of attachment 782891 [details] [diff] [review]:
-----------------------------------------------------------------

This is apparently the correct way to do this, it seems.

Comment 9

[Guillaume Abadie]

To be sure, let's wait the Ehsan's R+.

Comment 10

[Guillaume Abadie]

This patch is green like an apple :
https://tbpl.mozilla.org/?tree=Try&rev=37eeac151ef8
Keep waiting for Ehsan's R+.

Comment 11

[Guillaume Abadie]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2fcf08bde979

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2fcf08bde979

Comment 13

[u279076]

Guillaume, what should QA look for to verify this is fixed?

Comment 14

[Guillaume Abadie]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #13)
> Guillaume, what should QA look for to verify this is fixed?

http://dxr.mozilla.org/mozilla-central/source/dom/webidl/WebGL2RenderingContext.webidl#l9
http://dxr.mozilla.org/mozilla-central/source/dom/webidl/WebGL2RenderingContext.webidl#l13

So the behavior is HTMLCanvasElement::getContext('experimental-webgl2') should never ever succeed with webgl.enable-prototype-webgl2 to False or not set thanks by:

http://dxr.mozilla.org/mozilla-central/source/content/canvas/src/WebGL2Context.cpp#l37
http://dxr.mozilla.org/mozilla-central/source/content/html/content/src/HTMLCanvasElement.cpp#l722

Comment 15

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

This is even stronger than that. The expected behavior is that the WebGL2RenderingContext interface should not even exist.

The test is the following: go to the Browser Console (Ctrl+Shift+J) or any place where you can evaluate a JS expression, and enter this:

  0 instanceof WebGL2RenderingContext

This should say:

  ReferenceError: WebGL2RenderingContext is not defined

meaning that WebGL2RenderingContext is not even known as an interface. That's the expected behavior on Stable/Beta channels. If instead this just says

  false

then that means that WebGL2RenderingContext is known as an interface, which is the expected behavior on Nightly and Aurora.

Comment 16

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

Note that the test in comment 15 is independent of the machine you test this on (doesn't depend on drivers, GPU, etc) while the test in comment 14 is very machine-dependent.

Comment 17

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

This should have an automated test.

Comment 18

[Manuela Muntean [Away]]

Verified as fixed with Firefox 25 beta 2 (build ID: 20130923194050), using the STR from comment 15, on: Ubuntu 12.10 32-bit, Win 7 64-bit and Mac OS X 10.8.4. 

On the other hand, with latest Aurora and Nightly, I also get the " ReferenceError: WebGL2RenderingContext is not defined " message, on all 3 platforms. Is this expected?

Comment 19

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Doesn't look like the pref is enabled by default anywhere, so that seems expected.

Comment 20

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Guillaume, can you please write that automated test?

Comment 21

[Kelsey Gilbert [:jgilbert]]

(In reply to Manuela Muntean [:Manuela] [QA] from comment #18)
> Verified as fixed with Firefox 25 beta 2 (build ID: 20130923194050), using
> the STR from comment 15, on: Ubuntu 12.10 32-bit, Win 7 64-bit and Mac OS X
> 10.8.4. 
> 
> On the other hand, with latest Aurora and Nightly, I also get the "
> ReferenceError: WebGL2RenderingContext is not defined " message, on all 3
> platforms. Is this expected?

Yes.

(In reply to :Ms2ger from comment #20)
> Guillaume, can you please write that automated test?

I can do this real quick.

Comment 22

[Kelsey Gilbert [:jgilbert]]

Attachment: no-webgl2-expose

Comment 23

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

Comment on attachment 8348326 [details] [diff] [review]
no-webgl2-expose

Review of attachment 8348326 [details] [diff] [review]:
-----------------------------------------------------------------

That is fine to land on beta/release...  but can't you make a single test that would automatically detect the channel and require WebGL2RenderingContext to be exposed or not exposed accordingly? I mean, the present test looks like it will have to be landed over and over again on each beta train.

Comment 24

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

Oh no, i see --- you can land this everywhere since even on Nightly, webgl2 is not exposed by default. ok, fine then :-)