Attachment 434827 Details for Bug 228737 – Patch

[patch] Patch

bug-228737-20210803163229.patch (text/plain), 7.05 KB, created by Kimmo Kinnunen on 2021-08-03 06:32:31 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Kimmo Kinnunen

Created: 2021-08-03 06:32:31 PDT

Size: 7.05 KB

patch

obsolete

>Subversion Revision: 280527
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 329e767486ee0343cd9db656bd4c0f41ef8008ea..adf3e6354e14f5daea55a41bfa9de871b12776bd 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2021-08-03  Kimmo Kinnunen  <kkinnunen@apple.com>
>+
>+        Crash while reading WebGL drawing buffer if canvas image buffer allocation fails
>+        https://bugs.webkit.org/show_bug.cgi?id=228737
>+        <rdar://81150042>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The crash would happen for example when running out of memory during snapshot
>+        or printing. Snapshots and printing forces the WebGL canvas to be "painted
>+        to document", which would then trigger the crash.
>+
>+        Other code-paths that invoke CanvasBase::makeRenderingResultsAvailable,
>+        e.g. toDataURL and drawImage will check for the buffer before, and
>+        as such are not testable in the sense that adding the test would trigger
>+        the bug.
>+
>+        Test: webgl/webgl-oom-paint-document-no-crash.html
>+
>+        * html/canvas/WebGLRenderingContextBase.cpp:
>+        (WebCore::WebGLRenderingContextBase::paintRenderingResultsToCanvas):
>+        Check for the nullptr from CanvasBase::buffer(). This might happen
>+        when the ImageBuffer was not allocated due to memory constraints.
>+
> 2021-08-02  Martin Robinson  <mrobinson@webkit.org>
> 
>         [css-scroll-snap] Consider all snap areas at a given snap offset when snapping
>diff --git a/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp b/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp
>index 0616031732755a0dd73f8d9418b0f58467d1183a..a46b66d2540e61ae43a33eae52e38f0688b8a8e2 100644
>--- a/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp
>+++ b/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp
>@@ -1218,11 +1218,13 @@ void WebGLRenderingContextBase::paintRenderingResultsToCanvas()
>             auto& base = canvasBase();
>             base.clearCopiedImage();
>             m_markedCanvasDirty = false;
>-            // FIXME: Remote ImageBuffers do not flush the buffers that are drawn to a buffer.
>-            // Avoid leaking the WebGL content in the cases where a WebGL canvas element is drawn to a Context2D
>-            // canvas element repeatedly.
>-            base.buffer()->flushDrawingContext();
>-            m_context->paintCompositedResultsToCanvas(*base.buffer());
>+            if (auto buffer = base.buffer()) {
>+                // FIXME: Remote ImageBuffers do not flush the buffers that are drawn to a buffer.
>+                // Avoid leaking the WebGL content in the cases where a WebGL canvas element is drawn to a Context2D
>+                // canvas element repeatedly.
>+                buffer->flushDrawingContext();
>+                m_context->paintCompositedResultsToCanvas(*buffer);
>+            }
>         }
>         return;
>     }
>@@ -1236,11 +1238,13 @@ void WebGLRenderingContextBase::paintRenderingResultsToCanvas()
>     base.clearCopiedImage();
> 
>     m_markedCanvasDirty = false;
>-    // FIXME: Remote ImageBuffers do not flush the buffers that are drawn to a buffer.
>-    // Avoid leaking the WebGL content in the cases where a WebGL canvas element is drawn to a Context2D
>-    // canvas element repeatedly.
>-    base.buffer()->flushDrawingContext();
>-    m_context->paintRenderingResultsToCanvas(*base.buffer());
>+    if (auto buffer = base.buffer()) {
>+        // FIXME: Remote ImageBuffers do not flush the buffers that are drawn to a buffer.
>+        // Avoid leaking the WebGL content in the cases where a WebGL canvas element is drawn to a Context2D
>+        // canvas element repeatedly.
>+        buffer->flushDrawingContext();
>+        m_context->paintRenderingResultsToCanvas(*buffer);
>+    }
> }
> 
> std::optional<PixelBuffer> WebGLRenderingContextBase::paintRenderingResultsToPixelBuffer()
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index d3c65993528dbe931bf96e46d6e2d6e0a6219358..1a6b670c7fca18a6f36f953d51823b26449473f8 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,22 @@
>+2021-08-03  Kimmo Kinnunen  <kkinnunen@apple.com>
>+
>+        Crash while reading WebGL drawing buffer if canvas image buffer allocation fails
>+        https://bugs.webkit.org/show_bug.cgi?id=228737
>+        <rdar://81150042>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a test for failure to paint the WebGL canvas to document.
>+        Trigger the mode by using printing.
>+
>+        Other code-paths that invoke CanvasBase::makeRenderingResultsAvailable,
>+        e.g. toDataURL and drawImage will check for the buffer before, and
>+        as such are not testable in the sense that adding the test would trigger
>+        the bug.
>+
>+        * webgl/webgl-oom-paint-document-no-crash-expected.html: Added.
>+        * webgl/webgl-oom-paint-document-no-crash.html: Added.
>+
> 2021-08-01  Fujii Hironori  <Hironori.Fujii@sony.com>
> 
>         [WinCairo] Unreviewed test gardening
>diff --git a/LayoutTests/webgl/webgl-oom-paint-document-no-crash-expected.html b/LayoutTests/webgl/webgl-oom-paint-document-no-crash-expected.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..7a48260f0a8053f3750e3a4cc0edfd61d6329218
>--- /dev/null
>+++ b/LayoutTests/webgl/webgl-oom-paint-document-no-crash-expected.html
>@@ -0,0 +1,16 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<meta charset="utf-8">
>+<title>WebGL canvas "image buffer" OOM during document paint test reference image</title>
>+</head>
>+<body>
>+<div style="position: absolute; width: 300px; height: 150px; border:55px solid green;"></div>
>+<script>
>+"use strict";
>+// Printing adjusts the size of the result.
>+if (window.testRunner)
>+    testRunner.setPrinting();
>+</script>
>+</body>
>+</html>
>diff --git a/LayoutTests/webgl/webgl-oom-paint-document-no-crash.html b/LayoutTests/webgl/webgl-oom-paint-document-no-crash.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..0d51eec5e4ad32bed41835b6ccc20c9dac8f99a4
>--- /dev/null
>+++ b/LayoutTests/webgl/webgl-oom-paint-document-no-crash.html
>@@ -0,0 +1,31 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<meta charset="utf-8">
>+<title>WebGL canvas "image buffer" OOM during document paint test</title>
>+</head>
>+<body>
>+
>+<div style="position: absolute; width: 300px; height: 150px; border:55px solid green;"></div>
>+<canvas style="position: absolute;" id="gl"></canvas>
>+</div>
>+<script>
>+"use strict";
>+var canvasGL = document.getElementById('gl');
>+let gl = canvasGL.getContext('webgl');
>+gl.clearColor(0, 1, 0, 1);
>+gl.clear(gl.COLOR_BUFFER_BIT);
>+if (window.internals)
>+    window.internals.setMaxCanvasPixelMemory(7);
>+if (window.testRunner)
>+    testRunner.setPrinting();
>+</script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 228737: 434827