Bug 179070

<rdar://problem/35278276>

I'm not sure how useful this is. Listing supported extensions is as simple as calling `getSupportedExtensions()`. document.createElement("canvas").getContext("webgl").getSupportedExtensions() I think it would be more useful to list what extensions are enabled per context.

Created attachment 325702 [details] Patch Too tired to add comments to ChangeLog :(

Created attachment 325703 [details] [Image] After Patch is applied Not the prettiest, I know, but I'm not really sure what UI to use here. DataGrid doesn't make a lot of sense as every item shown will be true/on, and that seems like a waste. I figured that this is better than nothing ¯\_(ツ)_/¯

(In reply to Devin Rousso from comment #2) > I'm not sure how useful this is. Listing supported extensions is as simple > as calling `getSupportedExtensions()`. > > > document.createElement("canvas").getContext("webgl").getSupportedExtensions() > > I think it would be more useful to list what extensions are enabled per > context. Yes, this sounds more useful.

Comment on attachment 325702 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325702&action=review r=me, looks awesome! You may want to adjust the bug title and changelog to reflect what the patch actually does, though. > Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1084 > +#define ENABLE_EXTENSION(type, variable, nameLiteral, ternaryCondition) \ This is a great cleanup, but the macro name is nonsensical as it conditionally enables an extension, but this reads as if we are enabling all extensions. Maybe ENABLE_EXTENSION_IF_REQUESTED(...). I also think ternaryCondition is a bit obtuse, perhaps it reads better as "canEnable": variable = (canEnable) ? std::make_unique<type>(*this) : nullptr; > Source/WebCore/inspector/InspectorCanvasAgent.cpp:103 > + if (is<WebGLRenderingContextBase>(context)) { Does this handle WebGL2 as well? > LayoutTests/inspector/canvas/extensions-expected.txt:1 > +Test that CanvasManager tracks canvas memory costs and is notified of changes. Nope, lol. Please add subtly different boilerplate text. > LayoutTests/inspector/canvas/extensions.html:48 > + <p>Test that CanvasManager tracks canvas memory costs and is notified of changes.</p> Ditto.

Thinking a bit more, you could easily capture a stack trace for these getExtension calls and put them in the UI. I think this would be reasonable in the "live" case (Web Inspector is open when the calls happen). There's no point trying to save a call stack if Inspector isn't open.

Comment on attachment 325702 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325702&action=review >> Source/WebCore/inspector/InspectorCanvasAgent.cpp:103 >> + if (is<WebGLRenderingContextBase>(context)) { > > Does this handle WebGL2 as well? Yes this works for both. `getSupportedExtensions` is virtual and implemented by each subclass, and `extensionIsEnabled` is defined on WebGLRenderingContextBase. >> LayoutTests/inspector/canvas/extensions-expected.txt:1 >> +Test that CanvasManager tracks canvas memory costs and is notified of changes. > > Nope, lol. Please add subtly different boilerplate text. I did mention this when I uploaded the patch :P > (In reply to Devin Rousso from comment #3) >> Created attachment 325702 [details] >> Patch >> >> Too tired to add comments to ChangeLog :(

(In reply to Brian Burg from comment #7) > Thinking a bit more, you could easily capture a stack trace for these > getExtension calls and put them in the UI. I think this would be reasonable > in the "live" case (Web Inspector is open when the calls happen). There's no > point trying to save a call stack if Inspector isn't open. I don't think that a backtrace for `getExtension` is really all that useful, since AFAIK an enabled extension doesn't really do anything by itself. Furthermore, the user could just as easily put a breakpoint on any `getExtension` line and see it that way. I'm happy to add it, but I am skeptical of how useful it would be (and how to present it nicely in the UI). Regardless, I think it should be a separate change. <https://webkit.org/b/179205>

Neat! What does it look like when there are no extensions?

Created attachment 325785 [details] Patch

Comment on attachment 325785 [details] Patch Rejecting attachment 325785 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 325785, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: LayoutTests :040000 040000 fa6a38837d8e81970cb71cacb32c83aa9dbe0e07 fbe2d210b97d5f1faa5461bbb7037ec9426da33a M Source Current branch master is up to date. ERROR: Not all changes have been committed into SVN, however the committed ones (if any) seem to be successfully integrated into the working tree. Please see the above messages for details. Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Updating OpenSource Current branch master is up to date. Full output: http://webkit-queues.webkit.org/results/5082908

Created attachment 325808 [details] Patch

Comment on attachment 325808 [details] Patch Clearing flags on attachment: 325808 Committed r224370: <https://trac.webkit.org/changeset/224370>

All reviewed patches have been landed. Closing bug.