Add Plumming to get graphics error messages to JS Console
Created attachment 124356 [details] Patch
Hope this is good.
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Created attachment 124363 [details] Patch
Comment on attachment 124363 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=124363&action=review > Source/WebKit/chromium/public/platform/WebGraphicsContext3D.h:121 > + class WebGraphicsErrorMessageCallback { WebKit already has a way to write to the console. Why do you need this? See DOMWindow::console().
Comment on attachment 124363 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=124363&action=review >> Source/WebKit/chromium/public/platform/WebGraphicsContext3D.h:121 >> + class WebGraphicsErrorMessageCallback { > > WebKit already has a way to write to the console. Why do you need this? > See DOMWindow::console(). The intent is to allow deeper layers of Chromium's graphics stack to call back up and print detailed error messages to the console. Is there way to do this entirely on the Chromium side? How could we get from the WebGraphicsContext3DCommandBufferImpl to a WebFrame?
Comment on attachment 124363 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=124363&action=review >>> Source/WebKit/chromium/public/platform/WebGraphicsContext3D.h:121 >>> + class WebGraphicsErrorMessageCallback { >> >> WebKit already has a way to write to the console. Why do you need this? >> See DOMWindow::console(). > > The intent is to allow deeper layers of Chromium's graphics stack to call back up and print detailed error messages to the console. > > Is there way to do this entirely on the Chromium side? How could we get from the WebGraphicsContext3DCommandBufferImpl to a WebFrame? Oh, sorry, I had the interfaces inverted in my head. This makes sense now. Yes, plumbing through here makes more sense than trying to grovel around in the renderer for the associated WebFrame.
Comment on attachment 124363 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=124363&action=review Looks fine to me. Could you please add a little more detail to the bug report about the goal of this patch? Thanks. > Source/WebCore/ChangeLog:8 > + No new tests. (OOPS!) You'll need to remove this line in order for the commit queue to process the patch.
Created attachment 124417 [details] Patch
This patch is to allow the graphics system, which may be running in another process, the get errors back up to the WebGLRenderingContext so it can send those errors to the JS console. Since the errors are generated asynchronously we need way to get them back into the class that called them. There is one issue in that we can't currently associate which line of JavaScript issued the offending command but at least this way the developer will see the errors. The implementation does return a 32bit id for each error so if we wanted to, in the future, we could add a function, GraphicsContext3D::setDebugId(int id) or similar after which all commands that generate errors would return that id. We could then associate those ids with some javascript data to lookup the correct lines. I suspect doing that would be somewhat slow so it would probably be best disabled by default and only enabled through some kind of developer facing option. In the meantime, at least with these errors plummed through devs can see their program is doing something wrong. Most errors also have a function name associated with them which probably makes them pretty easy to find even without the line number.
Comment on attachment 124417 [details] Patch Looks great. r=me
Comment on attachment 124417 [details] Patch Clearing flags on attachment: 124417 Committed r106320: <http://trac.webkit.org/changeset/106320>
All reviewed patches have been landed. Closing bug.
I can’t help thinking that “plumming” is referring to some sort of drive-by fruiting.