Move WebGLErrorsToConsole page setting to window.internals.settings

• Component: WebGL
• Status: RESOLVED FIXED

[Kenneth Russell]

Per Bug 107035 the page setting WebGLErrorsToConsole should be moved to window.internals.settings. It shouldn't be hard to make all the WebGL tests set this to false, preventing spew by the test harness, while making it default to true (in browsers).

[Tony Chang]

Since you used Settings.in to define the property, you get this for free!

We automatically generate setters on window.internals.settings for bool preferences defined in Settings.in. window.internals.settings.setWebGLEnabled should already exist.

[arno.]

Created attachment 183587 [details]
patch proposal

[Build Bot]

Comment on attachment 183587 [details]
patch proposal

Attachment 183587 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/15966212

New failing tests:
inspector/profiler/webgl/webgl-profiler-api-changes.html
inspector/profiler/webgl/webgl-profiler-get-error.html

[arno.]

Created attachment 183833 [details]
updated patch

[Build Bot]

Comment on attachment 183833 [details]
updated patch

Attachment 183833 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/16037385

New failing tests:
compositing/z-order/negative-z-index.html
http/tests/security/window-events-clear-domain.html

[WebKit Review Bot]

Comment on attachment 183833 [details]
updated patch

Attachment 183833 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15991037

New failing tests:
inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html
http/tests/security/window-events-clear-domain.html
http/tests/security/webgl-remote-read-remote-image-blocked-no-crossorigin.html

[Kenneth Russell]

Comment on attachment 183833 [details]
updated patch

Arno, have you gone through the layout test failures reported by the EWS? A couple look like they could be related to this patch.

[arno.]

Created attachment 184266 [details]
updated patch

I think this patch covers all tests.
But the problem is: forceLostContext(SyntheticLostContext) is called every time a document is unloaded. So when working with webgl, the console receives a lot of unuseful WebGL: CONTEXT_LOST_WEBGL: loseContext: context lost messages.
What about sending lost context messages to console only if mode is RealLostContext ?

[Kenneth Russell]

(In reply to comment #8)
> What about sending lost context messages to console only if mode is RealLostContext ?

No objections, but I think you will find this difficult to implement. The CONTEXT_LOST_WEBGL error needs to be synthesized both for real and synthetic lost contexts, and by the time it's queried, the fact that the original lost context was synthetic is lost.

[arno.]

Created attachment 184537 [details]
updated patch

updated patch: do not display context lost errors on console if it is a synthetic lost. Also, instead of setting setWebGLErrorsToConsoleEnabled to false for all webgl tests, only do that for tests which trigger some errors. So, it will be easier to check if a test starts triggering an unexpected error

[Kenneth Russell]

Comment on attachment 184537 [details]
updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184537&action=review

Thanks for persisting. This patch looks good. Did you want it reviewed and cq'd? Please mark r? and cq? in Details of patch if so.

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:1885
> +            && (!state.bufferBinding || !state.bufferBinding->object())) {

Spurious change?

[arno.]

Created attachment 184626 [details]
updated patch (remove spurious change)

[Kenneth Russell]

Comment on attachment 184626 [details]
updated patch (remove spurious change)

Looks good. r=me

[WebKit Review Bot]

Comment on attachment 184626 [details]
updated patch (remove spurious change)

Clearing flags on attachment: 184626

Committed r140851: <http://trac.webkit.org/changeset/140851>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.