Bug 173569 – Web Inspector: Support getting the content of WebGL/WebGL2 contexts

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

For WebGL/WebGL2/WebGPU canvases that do not have the "preserveDrawingBuffer" attribute set to true, it is not possible to get the content of the canvas via toDataURL.  In order to display a snapshot of the canvas, we need another way to get its content.

[Devin Rousso]

Created attachment 313465 [details]
Patch

[Build Bot]

Comment on attachment 313465 [details]
Patch

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

New failing tests:
inspector/canvas/requestContent.html

[Build Bot]

Created attachment 313474 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 313465 [details]
Patch

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

New failing tests:
inspector/canvas/requestContent.html

[Build Bot]

Created attachment 313479 [details]
Archive of layout-test-results from ews112 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 313465 [details]
Patch

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

New failing tests:
inspector/canvas/requestContent.html

[Build Bot]

Created attachment 313481 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Devin Rousso]

Created attachment 314513 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/33112420>

[Joseph Pecoraro]

Comment on attachment 314513 [details]
Patch

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

r- just because of the bunch of comments I'd like to see an updated patch. The approach seems good to me.

> LayoutTests/inspector/canvas/requestContent-webgl2.html:27
> +            CanvasAgent.requestContent(canvas.identifier, (error, content) => {

You should be able to write this like:

    CanvasAgent.requestContent(canvas.identifier)
        .then(({content}) => { InspectorTest.log(content); })
        .then(resolve, reject);

Which looks a little cleaner.

> Source/WebCore/html/canvas/WebGLRenderingContextBase.h:213
> +    void preventBufferClearForInspector(bool value) { m_preventBufferClearForInspector = value; }

This should be a setter `setPreventBufferClearForInspector(bool)`. And you might as well include a getter even if it is not used.

> Source/WebCore/html/canvas/WebGLRenderingContextBase.h:841
> +    bool m_preventBufferClearForInspector { false };

The rest of the member variables are in the protected area. For example:

    572    bool m_synthesizedErrorsToConsole { true };
    573    int m_numGLErrorsToConsoleAllowed;

I'd expect this to go near those.

> Source/WebCore/inspector/InspectorCanvasAgent.cpp:151
> +            return;

This would return without clearing the PreventBufferClearForInspector state you just added. I'd expect code more like:

    WebGLRenderingContextBase* glContext = downcast<WebGLRenderingContextBase>(context);
    glContext->setPreventBufferClearForInspector(true);
    ExceptionOr<String> result = canvasEntry->element->toDataURL(ASCIILiteral("image/png"));
    glContext->setPreventBufferClearForInspector(false);
    if (result.hasException()) {
        errorString = result.releaseException().releaseMessage();
        return;
    }

[Devin Rousso]

Created attachment 314617 [details]
Patch

[Build Bot]

Comment on attachment 314617 [details]
Patch

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

New failing tests:
inspector/canvas/requestContent-2d.html

[Build Bot]

Created attachment 314624 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 314617 [details]
Patch

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

New failing tests:
inspector/canvas/requestContent-2d.html

[Build Bot]

Created attachment 314627 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 314617 [details]
Patch

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

New failing tests:
inspector/canvas/requestContent-2d.html

[Build Bot]

Created attachment 314633 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Devin Rousso]

Created attachment 314645 [details]
Patch

[Devin Rousso]

Created attachment 314647 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 314647 [details]
Patch

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

r=me

> Source/WebCore/html/canvas/WebGLRenderingContextBase.h:213
> +    bool getPreventBufferClearForInspector() const { return m_preventBufferClearForInspector; }

Getter should not have a `get` prefix.

> Source/WebCore/inspector/InspectorCanvasAgent.cpp:160
> +        gl->setPreventBufferClearForInspector(true);
> +
> +        ExceptionOr<String> result = canvasEntry->element->toDataURL(ASCIILiteral("image/png"));
> +
> +        gl->setPreventBufferClearForInspector(false);

Can you group these right next to the toDataURL line? That is the significant part to me.

[Devin Rousso]

Created attachment 314732 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 314732 [details]
Patch

Clearing flags on attachment: 314732

Committed r219208: <http://trac.webkit.org/changeset/219208>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.