Bug 77238 – Add Plumbing to get graphics error messages to JS Console

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Gregg Tavares]

Add Plumming to get graphics error messages to JS Console

[Gregg Tavares]

Created attachment 124356 [details]
Patch

[Gregg Tavares]

Hope this is good.

[WebKit Review Bot]

Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

[Gregg Tavares]

Created attachment 124363 [details]
Patch

[Darin Fisher (:fishd, Google)]

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().

[Kenneth Russell]

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?

[Darin Fisher (:fishd, Google)]

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.

[Kenneth Russell]

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.

[Gregg Tavares]

Created attachment 124417 [details]
Patch

[Gregg Tavares]

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.

[Kenneth Russell]

Comment on attachment 124417 [details]
Patch

Looks great. r=me

[WebKit Review Bot]

Comment on attachment 124417 [details]
Patch

Clearing flags on attachment: 124417

Committed r106320: <http://trac.webkit.org/changeset/106320>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

I can’t help thinking that “plumming” is referring to some sort of drive-by fruiting.