310 bytes, text/html
10.29 KB, text/plain
804 bytes, patch
|Details | Diff | Splinter Review|
Created attachment 639813 [details] testcase (requires extension & pref) 1. Install https://www.squarefree.com/extensions/domFuzzLite3.xpi 2. Set user_pref("dom.experimental_bindings", false); 3. Load the testcase. Result: Crash [@ mozilla::WebGLContext::ErrorInvalidValue ] Related to bug 764254 and/or bug 743753?
This doesn't seem related to bug 764254. It is true that this wouldn't have happened before bug 743753 landed, but that is not the underlying problem. The problem seems that ErrorInvalidValue doesn't seem to behave gracefully under memory pressure. It should, as there is no reason to it to want to allocate much memory.
"memory-pressure" is a notification that triggers GC, CC, and flushes caches. In this case, it's unrelated to actually being low on memory.
Ah, that memory-pressure. OK, so memory-pressure causes the WebGL context to get 'lost' which means that all GL resources are freed. So I take back what I said in comment 2, this could well be the same issue as bug 764254, a general issue about WebGL contexts in lost state not behaving correctly.
OK, and the stack trace you attached shows what's happening, the GL context pointer is reset to NULL and we're dereferencing that when we attempt to update the GL error state. Bug 764254 is a similar but distinct bug, where we are also dereferencing this null gl context pointer.
Created attachment 639934 [details] [diff] [review] check for lost context in this overload of BufferData Indeed, this overload of BufferData wasn't properly checking for context loss like other overloads and other webgl functions do.
On Windows: bp-0c1bd681-bc4d-4f64-8054-972b92120707.
Hmm, does sending a memory-pressure notification break all existing WebGL contexts? That seems excessive compared to what memory-pressure does in the rest of Gecko. (Indeed, pressing the "Minimize memory usage" button in about:memory causes the visualization on http://cubicvr.org/CubicVR.js/bd3/BeatDetektor3HD.html to go away.)
Yes and no. memory-pressure causes all WebGLContext to be "lost" immediately. However, WebGL applications then receive a webglcontextlost event that they are supposed to handle to request getting restored, in which case they receive a webglcontextrestored event which they are supposed to handle to restore their WebGL state. Here is an example of a WebGL page that handles this correctly: https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/demos/google/san-angeles/index.html In practice, of course many WebGL pages do not handle this at all. This is a problem; having the "Minimize memory usage" button in about:memory causing WebGL context loss was considered an acceptable way of raising developer attention about this issue and providing an easy way to test any application. This can also be tested programatically using the WEBGL_lose_context extension. Notice that context loss preexists WebGL. The OpenGL ES API that WebGL is based on, and that WebGL implementations build on, already have context loss. GPU resources are inherently non-permanent in many architectures (e.g. wrt device sleep).
I see. Thanks for explaining.
Oops, forgot to land. https://tbpl.mozilla.org/?tree=Try&rev=237a172fc1c4
syntax error. https://tbpl.mozilla.org/?tree=Try&rev=a2a80830d77c
Comment on attachment 639934 [details] [diff] [review] check for lost context in this overload of BufferData [Approval Request Comment] Bug caused by (feature/regressing bug #): since the switch of WebGLContext to new bindings, firefox 15 User impact if declined: occasional, non-security-sensitive webgl crash Testing completed (on m-c, etc.): m-i Risk to taking this patch (and alternatives if risky): no risk String or UUID changes made by this patch: none
Comment on attachment 639934 [details] [diff] [review] check for lost context in this overload of BufferData [Triage Comment] It's hard to argue with a no-risk crash fix for Aurora/Beta this early in the cycle. Approved.
http://hg.mozilla.org/releases/mozilla-aurora/rev/da26c9c61b2e http://hg.mozilla.org/releases/mozilla-beta/rev/a6fba0be4e2d Sorry for the delayed landing.
Verified using the steps to reproduce from the Description that Firefox 15 beta 4 does not crash when loading the attached test case. Verified on Windows XP, Ubuntu 12.04 and Mac OS X 10.6: Mozilla/5.0 (Windows NT 5.1; rv:15.0) Gecko/20100101 Firefox/15.0 Mozilla/5.0 (X11; Linux i686; rv:15.0) Gecko/20100101 Firefox/15.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:15.0) Gecko/20100101 Firefox/15.0 Verified also the Socorro reports, and I didn't fond any crashes after this fix was pushed.