<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[286160] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/286160">286160</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2021-11-25 01:17:45 -0800 (Thu, 25 Nov 2021)</dd>
</dl>

<h3>Log Message</h3>
<pre>ANGLE Metal: The memory backing IOSurfaces of former client buffer pbuffers is leaked
https://bugs.webkit.org/show_bug.cgi?id=233328
<rdar://problem/85563187>

Patch by Kimmo Kinnunen <kkinnunen@apple.com> on 2021-11-25
Reviewed by Antti Koivisto.

Source/WebCore:

Fix a bug where recycling GraphicsContextGLOpenGL display buffers would
leak the spare buffer pbuffer handle. This would happen if the
display buffer was marked as "in use", so that the IOSurface reference
would be dropped immediately in order to not use it as next drawing buffer.
However, the IOSurface is still bound in ANGLE and as such, the pbuffer handle
must be returned during `GraphicsContextGLIOSurfaceSwapChain::recycleBuffer``
call.

Adds API tests for testing the leak.

* platform/graphics/cocoa/GraphicsContextGLIOSurfaceSwapChain.cpp:
(WebCore::GraphicsContextGLIOSurfaceSwapChain::recycleBuffer):
(WebCore::GraphicsContextGLIOSurfaceSwapChain::present):

Source/WebCore/PAL:

Add prototype for IOSurfaceIncrementUseCount.
Currently used for a test, in simulating CA behavior.

* pal/spi/cocoa/IOSurfaceSPI.h:

Tools:

Add tests testing Cocoa GraphicsContextGLOpenGL drawing buffer
recycling behavior.

* TestWebKitAPI/Configurations/TestWebKitAPI.xcconfig:
* TestWebKitAPI/Sources.txt:
* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebCore/ImageBufferTests.cpp:
(TestWebKitAPI::imageBufferPixelIs):
(TestWebKitAPI::memoryFootprintChangedBy): Deleted.
* TestWebKitAPI/Tests/WebCore/cocoa/TestGraphicsContextGLOpenGLCocoa.mm:
(TestWebKitAPI::createDefaultTestContext):
(TestWebKitAPI::changeContextContents):
(TestWebKitAPI::TEST):
* TestWebKitAPI/TestUtilities.h: Added.
* TestWebKitAPI/TestUtilities.cpp: Added.
Add few useful functions to TestUtilities.h so that different
tests can use them.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorePALChangeLog">trunk/Source/WebCore/PAL/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorePALpalspicocoaIOSurfaceSPIh">trunk/Source/WebCore/PAL/pal/spi/cocoa/IOSurfaceSPI.h</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicscocoaGraphicsContextGLIOSurfaceSwapChaincpp">trunk/Source/WebCore/platform/graphics/cocoa/GraphicsContextGLIOSurfaceSwapChain.cpp</a></li>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsTestWebKitAPIConfigurationsTestWebKitAPIxcconfig">trunk/Tools/TestWebKitAPI/Configurations/TestWebKitAPI.xcconfig</a></li>
<li><a href="#trunkToolsTestWebKitAPISourcestxt">trunk/Tools/TestWebKitAPI/Sources.txt</a></li>
<li><a href="#trunkToolsTestWebKitAPITestWebKitAPIxcodeprojprojectpbxproj">trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj</a></li>
<li><a href="#trunkToolsTestWebKitAPITestsWebCoreImageBufferTestscpp">trunk/Tools/TestWebKitAPI/Tests/WebCore/ImageBufferTests.cpp</a></li>
<li><a href="#trunkToolsTestWebKitAPITestsWebCorecocoaTestGraphicsContextGLOpenGLCocoamm">trunk/Tools/TestWebKitAPI/Tests/WebCore/cocoa/TestGraphicsContextGLOpenGLCocoa.mm</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkToolsTestWebKitAPITestUtilitiescpp">trunk/Tools/TestWebKitAPI/TestUtilities.cpp</a></li>
<li><a href="#trunkToolsTestWebKitAPITestUtilitiesh">trunk/Tools/TestWebKitAPI/TestUtilities.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (286159 => 286160)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog   2021-11-25 06:59:55 UTC (rev 286159)
+++ trunk/Source/WebCore/ChangeLog      2021-11-25 09:17:45 UTC (rev 286160)
</span><span class="lines">@@ -1,3 +1,25 @@
</span><ins>+2021-11-25  Kimmo Kinnunen  <kkinnunen@apple.com>
+
+        ANGLE Metal: The memory backing IOSurfaces of former client buffer pbuffers is leaked
+        https://bugs.webkit.org/show_bug.cgi?id=233328
+        <rdar://problem/85563187>
+
+        Reviewed by Antti Koivisto.
+
+        Fix a bug where recycling GraphicsContextGLOpenGL display buffers would
+        leak the spare buffer pbuffer handle. This would happen if the
+        display buffer was marked as "in use", so that the IOSurface reference
+        would be dropped immediately in order to not use it as next drawing buffer.
+        However, the IOSurface is still bound in ANGLE and as such, the pbuffer handle
+        must be returned during `GraphicsContextGLIOSurfaceSwapChain::recycleBuffer``
+        call.
+
+        Adds API tests for testing the leak.
+
+        * platform/graphics/cocoa/GraphicsContextGLIOSurfaceSwapChain.cpp:
+        (WebCore::GraphicsContextGLIOSurfaceSwapChain::recycleBuffer):
+        (WebCore::GraphicsContextGLIOSurfaceSwapChain::present):
+
</ins><span class="cx"> 2021-11-24  David Kilzer  <ddkilzer@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Compiler should be able to check localized format strings for consistency
</span></span></pre></div>
<a id="trunkSourceWebCorePALChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/PAL/ChangeLog (286159 => 286160)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/PAL/ChangeLog       2021-11-25 06:59:55 UTC (rev 286159)
+++ trunk/Source/WebCore/PAL/ChangeLog  2021-11-25 09:17:45 UTC (rev 286160)
</span><span class="lines">@@ -1,3 +1,16 @@
</span><ins>+2021-11-25  Kimmo Kinnunen  <kkinnunen@apple.com>
+
+        ANGLE Metal: The memory backing IOSurfaces of former client buffer pbuffers is leaked
+        https://bugs.webkit.org/show_bug.cgi?id=233328
+        <rdar://problem/85563187>
+
+        Reviewed by Antti Koivisto.
+
+        Add prototype for IOSurfaceIncrementUseCount.
+        Currently used for a test, in simulating CA behavior.
+
+        * pal/spi/cocoa/IOSurfaceSPI.h:
+
</ins><span class="cx"> 2021-11-22  Myles C. Maxfield  <mmaxfield@apple.com>
</span><span class="cx"> 
</span><span class="cx">         [WebGPU] Use OptionSet where it makes sense to
</span></span></pre></div>
<a id="trunkSourceWebCorePALpalspicocoaIOSurfaceSPIh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/PAL/pal/spi/cocoa/IOSurfaceSPI.h (286159 => 286160)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/PAL/pal/spi/cocoa/IOSurfaceSPI.h    2021-11-25 06:59:55 UTC (rev 286159)
+++ trunk/Source/WebCore/PAL/pal/spi/cocoa/IOSurfaceSPI.h       2021-11-25 09:17:45 UTC (rev 286160)
</span><span class="lines">@@ -71,6 +71,7 @@
</span><span class="cx"> size_t IOSurfaceGetPropertyMaximum(CFStringRef property);
</span><span class="cx"> size_t IOSurfaceGetWidth(IOSurfaceRef buffer);
</span><span class="cx"> OSType IOSurfaceGetPixelFormat(IOSurfaceRef buffer);
</span><ins>+void IOSurfaceIncrementUseCount(IOSurfaceRef buffer);
</ins><span class="cx"> Boolean IOSurfaceIsInUse(IOSurfaceRef buffer);
</span><span class="cx"> IOReturn IOSurfaceLock(IOSurfaceRef buffer, uint32_t options, uint32_t *seed);
</span><span class="cx"> IOSurfaceRef IOSurfaceLookupFromMachPort(mach_port_t);
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicscocoaGraphicsContextGLIOSurfaceSwapChaincpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/cocoa/GraphicsContextGLIOSurfaceSwapChain.cpp (286159 => 286160)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/cocoa/GraphicsContextGLIOSurfaceSwapChain.cpp     2021-11-25 06:59:55 UTC (rev 286159)
+++ trunk/Source/WebCore/platform/graphics/cocoa/GraphicsContextGLIOSurfaceSwapChain.cpp        2021-11-25 09:17:45 UTC (rev 286160)
</span><span class="lines">@@ -47,9 +47,8 @@
</span><span class="cx">     if (m_spareBuffer.surface) {
</span><span class="cx">         if (m_spareBuffer.surface->isInUse())
</span><span class="cx">             m_spareBuffer.surface.reset();
</span><del>-        return WTFMove(m_spareBuffer);
</del><span class="cx">     }
</span><del>-    return { };
</del><ins>+    return std::exchange(m_spareBuffer, { });
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void* GraphicsContextGLIOSurfaceSwapChain::detachClient()
</span><span class="lines">@@ -60,6 +59,8 @@
</span><span class="cx"> 
</span><span class="cx"> void GraphicsContextGLIOSurfaceSwapChain::present(Buffer&& buffer)
</span><span class="cx"> {
</span><ins>+    ASSERT(!m_spareBuffer.surface);
+    ASSERT(!m_spareBuffer.handle);
</ins><span class="cx">     m_spareBuffer = std::exchange(m_displayBuffer, WTFMove(buffer));
</span><span class="cx">     if (m_displayBufferInUse) {
</span><span class="cx">         m_displayBufferInUse = false;
</span></span></pre></div>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (286159 => 286160)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog    2021-11-25 06:59:55 UTC (rev 286159)
+++ trunk/Tools/ChangeLog       2021-11-25 09:17:45 UTC (rev 286160)
</span><span class="lines">@@ -1,3 +1,29 @@
</span><ins>+2021-11-25  Kimmo Kinnunen  <kkinnunen@apple.com>
+
+        ANGLE Metal: The memory backing IOSurfaces of former client buffer pbuffers is leaked
+        https://bugs.webkit.org/show_bug.cgi?id=233328
+        <rdar://problem/85563187>
+
+        Reviewed by Antti Koivisto.
+
+        Add tests testing Cocoa GraphicsContextGLOpenGL drawing buffer
+        recycling behavior.
+
+        * TestWebKitAPI/Configurations/TestWebKitAPI.xcconfig:
+        * TestWebKitAPI/Sources.txt:
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+        * TestWebKitAPI/Tests/WebCore/ImageBufferTests.cpp:
+        (TestWebKitAPI::imageBufferPixelIs):
+        (TestWebKitAPI::memoryFootprintChangedBy): Deleted.
+        * TestWebKitAPI/Tests/WebCore/cocoa/TestGraphicsContextGLOpenGLCocoa.mm:
+        (TestWebKitAPI::createDefaultTestContext):
+        (TestWebKitAPI::changeContextContents):
+        (TestWebKitAPI::TEST):
+        * TestWebKitAPI/TestUtilities.h: Added.
+        * TestWebKitAPI/TestUtilities.cpp: Added.
+        Add few useful functions to TestUtilities.h so that different
+        tests can use them.
+
</ins><span class="cx"> 2021-11-23  Lauro Moura  <lmoura@igalia.com>
</span><span class="cx"> 
</span><span class="cx">         [GTK] GdkRGBA expects colors to be in the 0.0 to 1.0 range
</span></span></pre></div>
<a id="trunkToolsTestWebKitAPIConfigurationsTestWebKitAPIxcconfig"></a>
<div class="modfile"><h4>Modified: trunk/Tools/TestWebKitAPI/Configurations/TestWebKitAPI.xcconfig (286159 => 286160)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/TestWebKitAPI/Configurations/TestWebKitAPI.xcconfig  2021-11-25 06:59:55 UTC (rev 286159)
+++ trunk/Tools/TestWebKitAPI/Configurations/TestWebKitAPI.xcconfig     2021-11-25 09:17:45 UTC (rev 286160)
</span><span class="lines">@@ -93,7 +93,7 @@
</span><span class="cx"> 
</span><span class="cx"> OTHER_CPLUSPLUSFLAGS = $(inherited) -isystem $(SDKROOT)/System/Library/Frameworks/System.framework/PrivateHeaders;
</span><span class="cx"> 
</span><del>-OTHER_LDFLAGS = $(inherited) -lgtest -force_load $(BUILT_PRODUCTS_DIR)/libTestWebKitAPI.a -framework JavaScriptCore -framework WebKit -lWebCoreTestSupport -framework Metal $(WK_APPSERVERSUPPORT_LDFLAGS) $(WK_AUTHKIT_LDFLAGS) -framework Network $(WK_HID_LDFLAGS) $(WK_OPENGL_LDFLAGS) $(WK_PDFKIT_LDFLAGS) $(WK_SYSTEM_LDFLAGS) $(WK_UIKITMACHELPER_LDFLAGS) $(WK_VISIONKITCORE_LDFLAGS) $(OTHER_LDFLAGS_PLATFORM_$(WK_COCOA_TOUCH));
</del><ins>+OTHER_LDFLAGS = $(inherited) -lgtest -force_load $(BUILT_PRODUCTS_DIR)/libTestWebKitAPI.a -framework JavaScriptCore -framework WebKit -lWebCoreTestSupport -framework Metal -framework IOSurface $(WK_APPSERVERSUPPORT_LDFLAGS) $(WK_AUTHKIT_LDFLAGS) -framework Network $(WK_HID_LDFLAGS) $(WK_OPENGL_LDFLAGS) $(WK_PDFKIT_LDFLAGS) $(WK_SYSTEM_LDFLAGS) $(WK_UIKITMACHELPER_LDFLAGS) $(WK_VISIONKITCORE_LDFLAGS) $(OTHER_LDFLAGS_PLATFORM_$(WK_COCOA_TOUCH));
</ins><span class="cx"> OTHER_LDFLAGS_PLATFORM_ = -framework Cocoa -framework Carbon;
</span><span class="cx"> 
</span><span class="cx"> // FIXME: This should not be built on iOS. Instead we should create and use a TestWebKitAPI application.
</span></span></pre></div>
<a id="trunkToolsTestWebKitAPISourcestxt"></a>
<div class="modfile"><h4>Modified: trunk/Tools/TestWebKitAPI/Sources.txt (286159 => 286160)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/TestWebKitAPI/Sources.txt    2021-11-25 06:59:55 UTC (rev 286159)
+++ trunk/Tools/TestWebKitAPI/Sources.txt       2021-11-25 09:17:45 UTC (rev 286160)
</span><span class="lines">@@ -25,4 +25,5 @@
</span><span class="cx"> JavaScriptTest.cpp
</span><span class="cx"> PlatformUtilities.cpp
</span><span class="cx"> TestsController.cpp
</span><ins>+TestUtilities.cpp
</ins><span class="cx"> 
</span></span></pre></div>
<a id="trunkToolsTestWebKitAPITestUtilitiescpp"></a>
<div class="addfile"><h4>Added: trunk/Tools/TestWebKitAPI/TestUtilities.cpp (0 => 286160)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/TestWebKitAPI/TestUtilities.cpp                              (rev 0)
+++ trunk/Tools/TestWebKitAPI/TestUtilities.cpp 2021-11-25 09:17:45 UTC (rev 286160)
</span><span class="lines">@@ -0,0 +1,44 @@
</span><ins>+/*
+ * Copyright (C) 2021 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "TestUtilities.h"
+
+#import <wtf/MemoryFootprint.h>
+
+namespace TestWebKitAPI {
+
+::testing::AssertionResult memoryFootprintChangedBy(size_t& lastFootprint, double expectedChange, double error)
+{
+    WTF::releaseFastMallocFreeMemory();
+    size_t newFootprint = memoryFootprint();
+    size_t oldFootprint = std::exchange(lastFootprint, newFootprint);
+    double change = static_cast<double>(newFootprint) - oldFootprint;
+    if (change - expectedChange > error)
+        return ::testing::AssertionFailure() << "Footprint changed by " << change << ". Expected at most " << expectedChange << "+-" << error;
+    return ::testing::AssertionSuccess();
+}
+
+}
</ins></span></pre></div>
<a id="trunkToolsTestWebKitAPITestUtilitiesh"></a>
<div class="addfile"><h4>Added: trunk/Tools/TestWebKitAPI/TestUtilities.h (0 => 286160)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/TestWebKitAPI/TestUtilities.h                                (rev 0)
+++ trunk/Tools/TestWebKitAPI/TestUtilities.h   2021-11-25 09:17:45 UTC (rev 286160)
</span><span class="lines">@@ -0,0 +1,49 @@
</span><ins>+/*
+ * Copyright (C) 2021 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#pragma once
+
+#include "Test.h"
+#include "WTFStringUtilities.h"
+#include <WebCore/Color.h>
+#include <wtf/text/TextStream.h>
+
+namespace TestWebKitAPI {
+
+// Caller should initialize lastFootprint with memoryFootprint() for the initial call.
+::testing::AssertionResult memoryFootprintChangedBy(size_t& lastFootprint, double expectedChange, double error);
+
+}
+
+namespace WebCore {
+
+inline std::ostream& operator<<(std::ostream& os, const WebCore::Color& value)
+{
+    TextStream s { TextStream::LineMode::SingleLine };
+    s << value;
+    return os << s.release();
+}
+
+}
</ins></span></pre></div>
<a id="trunkToolsTestWebKitAPITestWebKitAPIxcodeprojprojectpbxproj"></a>
<div class="modfile"><h4>Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (286159 => 286160)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj        2021-11-25 06:59:55 UTC (rev 286159)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj   2021-11-25 09:17:45 UTC (rev 286160)
</span><span class="lines">@@ -2372,6 +2372,8 @@
</span><span class="cx">          7B7D09692519F8F90017A078 /* WebGLNoCrashOnOtherThreadAccess.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = WebGLNoCrashOnOtherThreadAccess.mm; sourceTree = "<group>"; };
</span><span class="cx">          7BA3936B271EDFCA0015911C /* TestGraphicsContextGLOpenGLCocoa.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = TestGraphicsContextGLOpenGLCocoa.mm; sourceTree = "<group>"; };
</span><span class="cx">          7BA3936D271EEC530015911C /* WebCoreUtilities.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WebCoreUtilities.h; sourceTree = "<group>"; };
</span><ins>+               7BB754AF274E39A100D00EC1 /* TestUtilities.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = TestUtilities.h; sourceTree = "<group>"; };
+               7BB754B0274E39A100D00EC1 /* TestUtilities.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = TestUtilities.cpp; sourceTree = "<group>"; };
</ins><span class="cx">           7C1AF7931E8DCBAB002645B9 /* PrepareForMoveToWindow.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = PrepareForMoveToWindow.mm; sourceTree = "<group>"; };
</span><span class="cx">          7C3965051CDD74F90094DBB8 /* ColorTests.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ColorTests.cpp; sourceTree = "<group>"; };
</span><span class="cx">          7C3DB8E21D12129B00AE8CC3 /* CommandBackForward.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CommandBackForward.mm; sourceTree = "<group>"; };
</span><span class="lines">@@ -3181,6 +3183,8 @@
</span><span class="cx">                          BCB9E7FA112359A300A137E0 /* Test.h */,
</span><span class="cx">                          BC131AA8117131FC00B69727 /* TestsController.cpp */,
</span><span class="cx">                          BCB9E7C711234E3A00A137E0 /* TestsController.h */,
</span><ins>+                               7BB754B0274E39A100D00EC1 /* TestUtilities.cpp */,
+                               7BB754AF274E39A100D00EC1 /* TestUtilities.h */,
</ins><span class="cx">                           7C83E0361D0A5F7000FEBCF3 /* Utilities.h */,
</span><span class="cx">                          7BA3936D271EEC530015911C /* WebCoreUtilities.h */,
</span><span class="cx">                          7CBD5A2222DE42A6004A9E32 /* WTFStringUtilities.cpp */,
</span></span></pre></div>
<a id="trunkToolsTestWebKitAPITestsWebCoreImageBufferTestscpp"></a>
<div class="modfile"><h4>Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/ImageBufferTests.cpp (286159 => 286160)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/TestWebKitAPI/Tests/WebCore/ImageBufferTests.cpp     2021-11-25 06:59:55 UTC (rev 286159)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/ImageBufferTests.cpp        2021-11-25 09:17:45 UTC (rev 286160)
</span><span class="lines">@@ -25,6 +25,7 @@
</span><span class="cx"> 
</span><span class="cx"> #include "config.h"
</span><span class="cx"> 
</span><ins>+#include "TestUtilities.h"
</ins><span class="cx"> #include <WebCore/Color.h>
</span><span class="cx"> #include <WebCore/ImageBuffer.h>
</span><span class="cx"> #include <WebCore/PlatformImageBuffer.h>
</span><span class="lines">@@ -34,17 +35,6 @@
</span><span class="cx"> namespace TestWebKitAPI {
</span><span class="cx"> using namespace WebCore;
</span><span class="cx"> 
</span><del>-static ::testing::AssertionResult memoryFootprintChangedBy(size_t& lastFootprint, double expectedChange, double error)
-{
-    WTF::releaseFastMallocFreeMemory();
-    size_t newFootprint = memoryFootprint();
-    size_t oldFootprint = std::exchange(lastFootprint, newFootprint);
-    double change = static_cast<double>(newFootprint) - oldFootprint;
-    if (change - expectedChange > error)
-        return ::testing::AssertionFailure() << "Footprint changed by " << change << ". Expected at most " << expectedChange << "+-" << error;
-    return ::testing::AssertionSuccess();
-}
-
</del><span class="cx"> static ::testing::AssertionResult imageBufferPixelIs(Color expected, ImageBuffer& imageBuffer, int x, int y)
</span><span class="cx"> {
</span><span class="cx">     PixelBufferFormat format { AlphaPremultiplication::Unpremultiplied, PixelFormat::RGBA8, DestinationColorSpace::SRGB() };
</span><span class="lines">@@ -52,7 +42,7 @@
</span><span class="cx">     auto& data = frontPixelBuffer->data();
</span><span class="cx">     auto got = Color { SRGBA<uint8_t> { data.item(0), data.item(1), data.item(2), data.item(3) } };
</span><span class="cx">     if (got != expected)
</span><del>-        return ::testing::AssertionFailure() << "color is not expected."; // FIXME: implement Color <<.
</del><ins>+        return ::testing::AssertionFailure() << "color is not expected. Got: " << got << ", expected: " << expected << ".";
</ins><span class="cx">     return ::testing::AssertionSuccess();
</span><span class="cx"> }
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkToolsTestWebKitAPITestsWebCorecocoaTestGraphicsContextGLOpenGLCocoamm"></a>
<div class="modfile"><h4>Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/cocoa/TestGraphicsContextGLOpenGLCocoa.mm (286159 => 286160)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/TestWebKitAPI/Tests/WebCore/cocoa/TestGraphicsContextGLOpenGLCocoa.mm        2021-11-25 06:59:55 UTC (rev 286159)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/cocoa/TestGraphicsContextGLOpenGLCocoa.mm   2021-11-25 09:17:45 UTC (rev 286160)
</span><span class="lines">@@ -27,29 +27,94 @@
</span><span class="cx"> #import "Test.h"
</span><span class="cx"> 
</span><span class="cx"> #if PLATFORM(COCOA) && ENABLE(WEBGL)
</span><ins>+#import "TestUtilities.h"
</ins><span class="cx"> #import "WebCoreUtilities.h"
</span><span class="cx"> #import <Metal/Metal.h>
</span><ins>+#import <WebCore/Color.h>
</ins><span class="cx"> #import <WebCore/GraphicsContextGLOpenGL.h>
</span><ins>+#import <optional>
+#import <wtf/HashSet.h>
+#import <wtf/MemoryFootprint.h>
</ins><span class="cx"> 
</span><span class="cx"> namespace TestWebKitAPI {
</span><del>-using namespace WebCore;
</del><span class="cx"> 
</span><span class="cx"> namespace {
</span><del>-class TestedGraphicsContextGLOpenGL : public GraphicsContextGLOpenGL {
</del><ins>+
+class TestedGraphicsContextGLOpenGL : public WebCore::GraphicsContextGLOpenGL {
</ins><span class="cx"> public:
</span><del>-    static RefPtr<TestedGraphicsContextGLOpenGL> create(GraphicsContextGLAttributes attributes)
</del><ins>+    static RefPtr<TestedGraphicsContextGLOpenGL> create(WebCore::GraphicsContextGLAttributes attributes)
</ins><span class="cx">     {
</span><span class="cx">         auto context = adoptRef(*new TestedGraphicsContextGLOpenGL(WTFMove(attributes)));
</span><span class="cx">         return context;
</span><span class="cx">     }
</span><ins>+    WebCore::IOSurface* displayBuffer()
+    {
+        return m_swapChain.displayBuffer().surface.get();
+    }
+    void markDisplayBufferInUse()
+    {
+        m_swapChain.markDisplayBufferInUse();
+    }
</ins><span class="cx"> private:
</span><del>-    TestedGraphicsContextGLOpenGL(GraphicsContextGLAttributes attributes)
-        : GraphicsContextGLOpenGL(WTFMove(attributes))
</del><ins>+    TestedGraphicsContextGLOpenGL(WebCore::GraphicsContextGLAttributes attributes)
+        : WebCore::GraphicsContextGLOpenGL(WTFMove(attributes))
</ins><span class="cx">     {
</span><span class="cx">     }
</span><span class="cx"> };
</span><ins>+
+class GraphicsContextGLOpenGLCocoaTest : public ::testing::Test {
+public:
+    void SetUp() override // NOLINT
+    {
+        m_scopedProcessType = ScopedSetAuxiliaryProcessTypeForTesting { WebCore::AuxiliaryProcessType::GPU };
+    }
+    void TearDown() override // NOLINT
+    {
+        m_scopedProcessType = std::nullopt;
+    }
+private:
+    std::optional<ScopedSetAuxiliaryProcessTypeForTesting> m_scopedProcessType;
+};
+
</ins><span class="cx"> }
</span><span class="cx"> 
</span><ins>+static const int expectedDisplayBufferPoolSize = 3;
+
+static RefPtr<TestedGraphicsContextGLOpenGL> createDefaultTestContext(WebCore::IntSize contextSize)
+{
+    WebCore::GraphicsContextGLAttributes attributes;
+    attributes.useMetal = true;
+    attributes.antialias = false;
+    attributes.depth = false;
+    attributes.stencil = false;
+    attributes.alpha = true;
+    attributes.preserveDrawingBuffer = false;
+    auto context = TestedGraphicsContextGLOpenGL::create(attributes);
+    if (!context)
+        return nullptr;
+    context->reshape(contextSize.width(), contextSize.height());
+    return context;
+}
+
+static ::testing::AssertionResult changeContextContents(TestedGraphicsContextGLOpenGL& context, int iteration)
+{
+    context.markContextChanged();
+    WebCore::Color expected { iteration % 2 ? WebCore::Color::green : WebCore::Color::yellow };
+    auto [colorSpace, components] = expected.colorSpaceAndComponents();
+    UNUSED_VARIABLE(colorSpace);
+    context.clearColor(components[0], components[1], components[2], components[3]);
+    context.clear(WebCore::GraphicsContextGL::COLOR_BUFFER_BIT);
+    uint8_t gotValues[4] = { };
+    auto sampleAt = context.getInternalFramebufferSize();
+    sampleAt.contract(2, 3);
+    sampleAt.clampNegativeToZero();
+    context.readnPixels(sampleAt.width(), sampleAt.height(), 1, 1, WebCore::GraphicsContextGL::RGBA, WebCore::GraphicsContextGL::UNSIGNED_BYTE, gotValues);
+    WebCore::Color got { WebCore::SRGBA<uint8_t> { gotValues[0], gotValues[1], gotValues[2], gotValues[3] } };
+    if (got != expected)
+        return ::testing::AssertionFailure() << "Failed to verify draw to context. Got: " << got << ", expected: " << expected << ".";
+    return ::testing::AssertionSuccess();
+}
+
</ins><span class="cx"> static bool hasMultipleGPUs()
</span><span class="cx"> {
</span><span class="cx"> #if (PLATFORM(MAC) || PLATFORM(MACCATALYST))
</span><span class="lines">@@ -68,29 +133,121 @@
</span><span class="cx"> // Tests for a bug where high-performance context would use low-power GPU if low-power or default
</span><span class="cx"> // context was created first. Test is applicable only for Metal, since GPU selection for OpenGL is
</span><span class="cx"> // very different.
</span><del>-TEST(GraphicsContextGLOpenGLCocoaTest, MAYBE_MultipleGPUsDifferentPowerPreferenceMetal)
</del><ins>+TEST_F(GraphicsContextGLOpenGLCocoaTest, MAYBE_MultipleGPUsDifferentPowerPreferenceMetal)
</ins><span class="cx"> {
</span><span class="cx">     if (!hasMultipleGPUs())
</span><span class="cx">         return;
</span><del>-    ScopedSetAuxiliaryProcessTypeForTesting scopedProcessType { AuxiliaryProcessType::GPU };
</del><span class="cx"> 
</span><del>-    GraphicsContextGLAttributes attributes;
</del><ins>+    WebCore::GraphicsContextGLAttributes attributes;
</ins><span class="cx">     attributes.useMetal = true;
</span><del>-    EXPECT_EQ(attributes.powerPreference, GraphicsContextGLPowerPreference::Default);
</del><ins>+    EXPECT_EQ(attributes.powerPreference, WebCore::GraphicsContextGLPowerPreference::Default);
</ins><span class="cx">     auto defaultContext = TestedGraphicsContextGLOpenGL::create(attributes);
</span><del>-    EXPECT_NE(defaultContext, nullptr);
</del><ins>+    ASSERT_NE(defaultContext, nullptr);
</ins><span class="cx"> 
</span><del>-    attributes.powerPreference = GraphicsContextGLPowerPreference::LowPower;
</del><ins>+    attributes.powerPreference = WebCore::GraphicsContextGLPowerPreference::LowPower;
</ins><span class="cx">     auto lowPowerContext = TestedGraphicsContextGLOpenGL::create(attributes);
</span><del>-    EXPECT_NE(lowPowerContext, nullptr);
</del><ins>+    ASSERT_NE(lowPowerContext, nullptr);
</ins><span class="cx"> 
</span><del>-    attributes.powerPreference = GraphicsContextGLPowerPreference::HighPerformance;
</del><ins>+    attributes.powerPreference = WebCore::GraphicsContextGLPowerPreference::HighPerformance;
</ins><span class="cx">     auto highPerformanceContext = TestedGraphicsContextGLOpenGL::create(attributes);
</span><del>-    EXPECT_NE(highPerformanceContext, nullptr);
</del><ins>+    ASSERT_NE(highPerformanceContext, nullptr);
</ins><span class="cx"> 
</span><del>-    EXPECT_NE(lowPowerContext->getString(GraphicsContextGL::RENDERER), highPerformanceContext->getString(GraphicsContextGL::RENDERER));
-    EXPECT_EQ(defaultContext->getString(GraphicsContextGL::RENDERER), lowPowerContext->getString(GraphicsContextGL::RENDERER));
</del><ins>+    EXPECT_NE(lowPowerContext->getString(WebCore::GraphicsContextGL::RENDERER), highPerformanceContext->getString(WebCore::GraphicsContextGL::RENDERER));
+    EXPECT_EQ(defaultContext->getString(WebCore::GraphicsContextGL::RENDERER), lowPowerContext->getString(WebCore::GraphicsContextGL::RENDERER));
</ins><span class="cx"> }
</span><span class="cx"> 
</span><ins>+TEST_F(GraphicsContextGLOpenGLCocoaTest, DisplayBuffersAreRecycled)
+{
+    auto context = createDefaultTestContext({ 20, 20 });
+    ASSERT_NE(context, nullptr);
+    RetainPtr<IOSurfaceRef> expectedDisplayBuffers[expectedDisplayBufferPoolSize];
+    for (int i = 0; i < 50; ++i) {
+        EXPECT_TRUE(changeContextContents(*context, i));
+        context->prepareForDisplay();
+        auto* surface = context->displayBuffer();
+        ASSERT_NE(surface, nullptr);
+        int slot = i % expectedDisplayBufferPoolSize;
+        if (!expectedDisplayBuffers[slot])
+            expectedDisplayBuffers[slot] = surface->surface();
+        EXPECT_EQ(expectedDisplayBuffers[slot].get(), surface->surface()) << "for i:" << i << " slot: " << slot;
+    }
+    for (int i = 0; i < expectedDisplayBufferPoolSize - 1; ++i) {
+        for (int j = i + 1; j < expectedDisplayBufferPoolSize; ++j)
+            EXPECT_NE(expectedDisplayBuffers[i].get(), expectedDisplayBuffers[j].get()) << "for i: " << i << " j:" << j;
+    }
</ins><span class="cx"> }
</span><ins>+
+// Test that drawing buffers are not recycled if `GraphicsContextGLOpenGL::markDisplayBufferInUse()`
+// is called.
+TEST_F(GraphicsContextGLOpenGLCocoaTest, DisplayBuffersAreNotRecycledWhenMarkedInUse)
+{
+    auto context = createDefaultTestContext({ 20, 20 });
+    ASSERT_NE(context, nullptr);
+    HashSet<RetainPtr<IOSurfaceRef>> seenSurfaceRefs;
+    for (int i = 0; i < 50; ++i) {
+        EXPECT_TRUE(changeContextContents(*context, i));
+        context->prepareForDisplay();
+        WebCore::IOSurface* surface = context->displayBuffer();
+        ASSERT_NE(surface, nullptr);
+        IOSurfaceRef surfaceRef = surface->surface();
+        EXPECT_NE(surfaceRef, nullptr);
+        EXPECT_FALSE(seenSurfaceRefs.contains(surfaceRef));
+        seenSurfaceRefs.add(surfaceRef);
+
+        context->markDisplayBufferInUse();
+    }
+    ASSERT_EQ(seenSurfaceRefs.size(), 50u);
+}
+
+// Test that drawing buffers are not recycled if the use count of the underlying IOSurface
+// changes. Use count is modified for example by CoreAnimation when the IOSurface is attached
+// to the contents.
+TEST_F(GraphicsContextGLOpenGLCocoaTest, DisplayBuffersAreNotRecycledWhedInUse)
+{
+    auto context = createDefaultTestContext({ 20, 20 });
+    ASSERT_NE(context, nullptr);
+    HashSet<RetainPtr<IOSurfaceRef>> seenSurfaceRefs;
+    for (int i = 0; i < 50; ++i) {
+        EXPECT_TRUE(changeContextContents(*context, i));
+        context->prepareForDisplay();
+        WebCore::IOSurface* surface = context->displayBuffer();
+        ASSERT_NE(surface, nullptr);
+        IOSurfaceRef surfaceRef = surface->surface();
+        EXPECT_NE(surfaceRef, nullptr);
+        EXPECT_FALSE(seenSurfaceRefs.contains(surfaceRef));
+        seenSurfaceRefs.add(surfaceRef);
+
+        IOSurfaceIncrementUseCount(surfaceRef);
+    }
+    ASSERT_EQ(seenSurfaceRefs.size(), 50u);
+}
+
+// Test that drawing to GraphicsContextGL and marking the display buffer in use does not leak big
+// amounts of memory for each displayed buffer.
+TEST_F(GraphicsContextGLOpenGLCocoaTest, UnrecycledDisplayBuffersNoLeaks)
+{
+    // The test detects the leak by observing memory footprint. However, some of the freed IOSurface
+    // memory (130mb) stays resident, presumably by intention of IOKit. The test would originally leak
+    // 2.7gb so the intended bug would be detected with 150mb error range.
+    size_t footprintError = 150 * 1024 * 1024;
+    size_t footprintChange = 0;
+
+    auto context = createDefaultTestContext({ 2048, 2048 });
+    ASSERT_NE(context, nullptr);
+
+    WTF::releaseFastMallocFreeMemory();
+    auto lastFootprint = memoryFootprint();
+
+    for (int i = 0; i < 50; ++i) {
+        EXPECT_TRUE(changeContextContents(*context, i));
+        context->prepareForDisplay();
+        EXPECT_NE(context->displayBuffer(), nullptr);
+        context->markDisplayBufferInUse();
+    }
+
+    EXPECT_TRUE(memoryFootprintChangedBy(lastFootprint, footprintChange, footprintError));
+}
+
+}
+
</ins><span class="cx"> #endif
</span></span></pre>
</div>
</div>

</body>
</html>