Changeset 280601 in webkit

Timestamp:

Aug 3, 2021, 10:34:48 AM (5 years ago)

Author:

commit-queue@webkit.org

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 <​kkinnunen@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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/webgl/webgl-oom-paint-document-no-crash-expected.html (added)
• LayoutTests/webgl/webgl-oom-paint-document-no-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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 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  <antti@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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 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  <antti@apple.com>
 

trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp

@@ -1219 +1219 @@
             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;
@@ -1237 +1239 @@
 
     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);
+    }
 }