Title: [280601] trunk
Revision
280601
Author
commit-qu...@webkit.org
Date
2021-08-03 10:34:48 -0700 (Tue, 03 Aug 2021)

Log Message

Crash while reading WebGL drawing buffer if canvas image buffer allocation fails
https://bugs.webkit.org/show_bug.cgi?id=228737
<rdar://81150042>

Patch by Kimmo Kinnunen <kkinnu...@apple.com> on 2021-08-03
Reviewed by Brent Fulgham.

Source/WebCore:

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.

LayoutTests:

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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (280600 => 280601)


--- trunk/LayoutTests/ChangeLog	2021-08-03 17:05:15 UTC (rev 280600)
+++ trunk/LayoutTests/ChangeLog	2021-08-03 17:34:48 UTC (rev 280601)
@@ -1,3 +1,22 @@
+2021-08-03  Kimmo Kinnunen  <kkinnu...@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 Brent Fulgham.
+
+        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-03  Antti Koivisto  <an...@apple.com>
 
         REGRESSION(r279050): Crash under CSSImageValue::createDeprecatedCSSOMWrapper with cursor images

Added: trunk/LayoutTests/webgl/webgl-oom-paint-document-no-crash-expected.html (0 => 280601)


--- trunk/LayoutTests/webgl/webgl-oom-paint-document-no-crash-expected.html	                        (rev 0)
+++ trunk/LayoutTests/webgl/webgl-oom-paint-document-no-crash-expected.html	2021-08-03 17:34:48 UTC (rev 280601)
@@ -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>

Added: trunk/LayoutTests/webgl/webgl-oom-paint-document-no-crash.html (0 => 280601)


--- trunk/LayoutTests/webgl/webgl-oom-paint-document-no-crash.html	                        (rev 0)
+++ trunk/LayoutTests/webgl/webgl-oom-paint-document-no-crash.html	2021-08-03 17:34:48 UTC (rev 280601)
@@ -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>
+<!--
+At the time of writing printing or snapshotting would access the "image buffer" of the WebGL canvas.
+This would leading to crash if the allocation was denied.
+To ensure expected behavior, e.g. to observe the failure, the canvas should not be visible.
+To assert this, use div with border as indicator. Background cannot be used as printing
+disables the backgrounds and -webkit-print-color-adjust: exact is not respected in
+WTR printing mode.
+-->
+<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>

Modified: trunk/Source/WebCore/ChangeLog (280600 => 280601)


--- trunk/Source/WebCore/ChangeLog	2021-08-03 17:05:15 UTC (rev 280600)
+++ trunk/Source/WebCore/ChangeLog	2021-08-03 17:34:48 UTC (rev 280601)
@@ -1,3 +1,27 @@
+2021-08-03  Kimmo Kinnunen  <kkinnu...@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 Brent Fulgham.
+
+        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-03  Antti Koivisto  <an...@apple.com>
 
         REGRESSION(r279050): Crash under CSSImageValue::createDeprecatedCSSOMWrapper with cursor images

Modified: trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp (280600 => 280601)


--- trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp	2021-08-03 17:05:15 UTC (rev 280600)
+++ trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp	2021-08-03 17:34:48 UTC (rev 280601)
@@ -1218,11 +1218,13 @@
             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 @@
     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()
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to