Title: [287673] trunk
Revision
287673
Author
mmaxfi...@apple.com
Date
2022-01-05 20:40:39 -0800 (Wed, 05 Jan 2022)

Log Message

[GPU Process] Small ImageBuffers cause the web process to crash
https://bugs.webkit.org/show_bug.cgi?id=232470
<rdar://problem/84626560>

Reviewed by Tim Horton.

Source/WebKit:

The problem is when the (floating point) size < 1x1, but the size*resolution is >= 1x1.
In this situation, calculateSafeBackendSize() is correctly determining that this
isn't a zero-sized ImageBuffer, but when we go to actually pass the size to the GPU
process, we call this:

IntSize logicalSize() const override { return IntSize(m_parameters.logicalSize); }

So, the logical size gets truncated down to 0, and then the GPU process fails to allocate
the ImageBuffer, and then the web process blocks on the GPU process indefinitely, and then
eventually times out and then crashes. I'm going to deal with that last step (the crash
itself) in a secondary patch - if the web process doesn't hear from the GPU process, it
shouldn't crash.

This patch simply exposes a floatLogicalSize() function on ImageBuffer, so we can get
the full-fidelity logical size to pass that to the GPU process.

This patch is just enough to stop WebKit from crashing. I'm going to continue looking into this bug in
https://bugs.webkit.org/show_bug.cgi?id=225377.

Test: compositing/device-pixel-image-buffer-hidpi.html

* WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:
(WebKit::RemoteRenderingBackendProxy::createRemoteImageBuffer):

LayoutTests:

* compositing/device-pixel-image-buffer-hidpi-expected.html: Added.
* compositing/device-pixel-image-buffer-hidpi.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (287672 => 287673)


--- trunk/LayoutTests/ChangeLog	2022-01-06 03:54:05 UTC (rev 287672)
+++ trunk/LayoutTests/ChangeLog	2022-01-06 04:40:39 UTC (rev 287673)
@@ -1,3 +1,14 @@
+2021-10-30  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        [GPU Process] Small ImageBuffers cause the web process to crash
+        https://bugs.webkit.org/show_bug.cgi?id=232470
+        <rdar://problem/84626560>
+
+        Reviewed by Tim Horton.
+
+        * compositing/device-pixel-image-buffer-hidpi-expected.html: Added.
+        * compositing/device-pixel-image-buffer-hidpi.html: Added.
+
 2022-01-05  Brandon Stewart  <brandonstew...@apple.com>
 
         border radii may have missing values

Added: trunk/LayoutTests/compositing/device-pixel-image-buffer-hidpi-expected.html (0 => 287673)


--- trunk/LayoutTests/compositing/device-pixel-image-buffer-hidpi-expected.html	                        (rev 0)
+++ trunk/LayoutTests/compositing/device-pixel-image-buffer-hidpi-expected.html	2022-01-06 04:40:39 UTC (rev 287673)
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta name="viewport" content="width=device-width, initial-scale=1">
+</head>
+<body>
+This test passes if there is no crash. The crash might happen on a subsequent test.
+<div style="font-size: 20px; width: 50px; height: 0.5px; background: green; overflow: hidden;">Hello</div>
+</body>
+</html>

Added: trunk/LayoutTests/compositing/device-pixel-image-buffer-hidpi.html (0 => 287673)


--- trunk/LayoutTests/compositing/device-pixel-image-buffer-hidpi.html	                        (rev 0)
+++ trunk/LayoutTests/compositing/device-pixel-image-buffer-hidpi.html	2022-01-06 04:40:39 UTC (rev 287673)
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta name="viewport" content="width=device-width, initial-scale=1">
+</head>
+<body>
+This test passes if there is no crash. The crash might happen on a subsequent test.
+<div style="transform: translateZ(20px); font-size: 20px; width: 50px; height: 0.5px; background: green; overflow: hidden;">Hello</div>
+</body>
+</html>

Modified: trunk/Source/WebKit/ChangeLog (287672 => 287673)


--- trunk/Source/WebKit/ChangeLog	2022-01-06 03:54:05 UTC (rev 287672)
+++ trunk/Source/WebKit/ChangeLog	2022-01-06 04:40:39 UTC (rev 287673)
@@ -1,3 +1,35 @@
+2021-10-30  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        [GPU Process] Small ImageBuffers cause the web process to crash
+        https://bugs.webkit.org/show_bug.cgi?id=232470
+        <rdar://problem/84626560>
+
+        Reviewed by Tim Horton.
+
+        The problem is when the (floating point) size < 1x1, but the size*resolution is >= 1x1.
+        In this situation, calculateSafeBackendSize() is correctly determining that this
+        isn't a zero-sized ImageBuffer, but when we go to actually pass the size to the GPU
+        process, we call this:
+
+        IntSize logicalSize() const override { return IntSize(m_parameters.logicalSize); }
+
+        So, the logical size gets truncated down to 0, and then the GPU process fails to allocate
+        the ImageBuffer, and then the web process blocks on the GPU process indefinitely, and then
+        eventually times out and then crashes. I'm going to deal with that last step (the crash
+        itself) in a secondary patch - if the web process doesn't hear from the GPU process, it
+        shouldn't crash.
+
+        This patch simply exposes a floatLogicalSize() function on ImageBuffer, so we can get
+        the full-fidelity logical size to pass that to the GPU process.
+
+        This patch is just enough to stop WebKit from crashing. I'm going to continue looking into this bug in
+        https://bugs.webkit.org/show_bug.cgi?id=225377.
+
+        Test: compositing/device-pixel-image-buffer-hidpi.html
+
+        * WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:
+        (WebKit::RemoteRenderingBackendProxy::createRemoteImageBuffer):
+
 2022-01-05  Per Arne Vollan  <pvol...@apple.com>
 
         REGRESSION(287556): Domain name not shown in Activity Monitor

Modified: trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp (287672 => 287673)


--- trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp	2022-01-06 03:54:05 UTC (rev 287672)
+++ trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp	2022-01-06 04:40:39 UTC (rev 287673)
@@ -127,7 +127,13 @@
 
 void RemoteRenderingBackendProxy::createRemoteImageBuffer(ImageBuffer& imageBuffer)
 {
-    sendToStream(Messages::RemoteRenderingBackend::CreateImageBuffer(imageBuffer.truncatedLogicalSize(), imageBuffer.renderingMode(), imageBuffer.resolutionScale(), imageBuffer.colorSpace(), imageBuffer.pixelFormat(), imageBuffer.renderingResourceIdentifier()));
+    auto logicalSize = imageBuffer.logicalSize();
+    if (logicalSize.width() > 1 || logicalSize.height() > 1) {
+        // FIXME: https://bugs.webkit.org/show_bug.cgi?id=225377 If we unconditionally use imageBuffer.logicalSize() here instead of imageBuffer.truncatedLogicalSize(),
+        // there may be a memory regression. See https://trac.webkit.org/changeset/287358/webkit
+        logicalSize = imageBuffer.truncatedLogicalSize();
+    }
+    sendToStream(Messages::RemoteRenderingBackend::CreateImageBuffer(logicalSize, imageBuffer.renderingMode(), imageBuffer.resolutionScale(), imageBuffer.colorSpace(), imageBuffer.pixelFormat(), imageBuffer.renderingResourceIdentifier()));
 }
 
 RefPtr<ImageBuffer> RemoteRenderingBackendProxy::createImageBuffer(const FloatSize& size, RenderingMode renderingMode, float resolutionScale, const DestinationColorSpace& colorSpace, PixelFormat pixelFormat)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to