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.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@286160 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index a9a4597..7c62fcd 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+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):
+
2021-11-24 David Kilzer <ddkilzer@apple.com>
Compiler should be able to check localized format strings for consistency
diff --git a/Source/WebCore/PAL/ChangeLog b/Source/WebCore/PAL/ChangeLog
index e72dbfd..5d6d677 100644
--- a/Source/WebCore/PAL/ChangeLog
+++ b/Source/WebCore/PAL/ChangeLog
@@ -1,3 +1,16 @@
+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:
+
2021-11-22 Myles C. Maxfield <mmaxfield@apple.com>
[WebGPU] Use OptionSet where it makes sense to
diff --git a/Source/WebCore/PAL/pal/spi/cocoa/IOSurfaceSPI.h b/Source/WebCore/PAL/pal/spi/cocoa/IOSurfaceSPI.h
index 4d17cdc..c1b4b43 100644
--- a/Source/WebCore/PAL/pal/spi/cocoa/IOSurfaceSPI.h
+++ b/Source/WebCore/PAL/pal/spi/cocoa/IOSurfaceSPI.h
@@ -71,6 +71,7 @@
size_t IOSurfaceGetPropertyMaximum(CFStringRef property);
size_t IOSurfaceGetWidth(IOSurfaceRef buffer);
OSType IOSurfaceGetPixelFormat(IOSurfaceRef buffer);
+void IOSurfaceIncrementUseCount(IOSurfaceRef buffer);
Boolean IOSurfaceIsInUse(IOSurfaceRef buffer);
IOReturn IOSurfaceLock(IOSurfaceRef buffer, uint32_t options, uint32_t *seed);
IOSurfaceRef IOSurfaceLookupFromMachPort(mach_port_t);
diff --git a/Source/WebCore/platform/graphics/cocoa/GraphicsContextGLIOSurfaceSwapChain.cpp b/Source/WebCore/platform/graphics/cocoa/GraphicsContextGLIOSurfaceSwapChain.cpp
index 8d4c388..4b214fa 100644
--- a/Source/WebCore/platform/graphics/cocoa/GraphicsContextGLIOSurfaceSwapChain.cpp
+++ b/Source/WebCore/platform/graphics/cocoa/GraphicsContextGLIOSurfaceSwapChain.cpp
@@ -47,9 +47,8 @@
if (m_spareBuffer.surface) {
if (m_spareBuffer.surface->isInUse())
m_spareBuffer.surface.reset();
- return WTFMove(m_spareBuffer);
}
- return { };
+ return std::exchange(m_spareBuffer, { });
}
void* GraphicsContextGLIOSurfaceSwapChain::detachClient()
@@ -60,6 +59,8 @@
void GraphicsContextGLIOSurfaceSwapChain::present(Buffer&& buffer)
{
+ ASSERT(!m_spareBuffer.surface);
+ ASSERT(!m_spareBuffer.handle);
m_spareBuffer = std::exchange(m_displayBuffer, WTFMove(buffer));
if (m_displayBufferInUse) {
m_displayBufferInUse = false;
diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index a58ffc1..5f50631 100644
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,29 @@
+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.
+
2021-11-23 Lauro Moura <lmoura@igalia.com>
[GTK] GdkRGBA expects colors to be in the 0.0 to 1.0 range
diff --git a/Tools/TestWebKitAPI/Configurations/TestWebKitAPI.xcconfig b/Tools/TestWebKitAPI/Configurations/TestWebKitAPI.xcconfig
index fc88769..7c92ab3 100644
--- a/Tools/TestWebKitAPI/Configurations/TestWebKitAPI.xcconfig
+++ b/Tools/TestWebKitAPI/Configurations/TestWebKitAPI.xcconfig
@@ -93,7 +93,7 @@
OTHER_CPLUSPLUSFLAGS = $(inherited) -isystem $(SDKROOT)/System/Library/Frameworks/System.framework/PrivateHeaders;
-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));
+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));
OTHER_LDFLAGS_PLATFORM_ = -framework Cocoa -framework Carbon;
// FIXME: This should not be built on iOS. Instead we should create and use a TestWebKitAPI application.
diff --git a/Tools/TestWebKitAPI/Sources.txt b/Tools/TestWebKitAPI/Sources.txt
index aa375f1..926d56e88 100644
--- a/Tools/TestWebKitAPI/Sources.txt
+++ b/Tools/TestWebKitAPI/Sources.txt
@@ -25,4 +25,5 @@
JavaScriptTest.cpp
PlatformUtilities.cpp
TestsController.cpp
+TestUtilities.cpp
diff --git a/Tools/TestWebKitAPI/TestUtilities.cpp b/Tools/TestWebKitAPI/TestUtilities.cpp
new file mode 100644
index 0000000..69f2f87
--- /dev/null
+++ b/Tools/TestWebKitAPI/TestUtilities.cpp
@@ -0,0 +1,44 @@
+/*
+ * 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();
+}
+
+}
diff --git a/Tools/TestWebKitAPI/TestUtilities.h b/Tools/TestWebKitAPI/TestUtilities.h
new file mode 100644
index 0000000..496a62b
--- /dev/null
+++ b/Tools/TestWebKitAPI/TestUtilities.h
@@ -0,0 +1,49 @@
+/*
+ * 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();
+}
+
+}
diff --git a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
index a3ecf30..3bd1257 100644
--- a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
+++ b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
@@ -2372,6 +2372,8 @@
7B7D09692519F8F90017A078 /* WebGLNoCrashOnOtherThreadAccess.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = WebGLNoCrashOnOtherThreadAccess.mm; sourceTree = "<group>"; };
7BA3936B271EDFCA0015911C /* TestGraphicsContextGLOpenGLCocoa.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = TestGraphicsContextGLOpenGLCocoa.mm; sourceTree = "<group>"; };
7BA3936D271EEC530015911C /* WebCoreUtilities.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WebCoreUtilities.h; sourceTree = "<group>"; };
+ 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>"; };
7C1AF7931E8DCBAB002645B9 /* PrepareForMoveToWindow.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = PrepareForMoveToWindow.mm; sourceTree = "<group>"; };
7C3965051CDD74F90094DBB8 /* ColorTests.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ColorTests.cpp; sourceTree = "<group>"; };
7C3DB8E21D12129B00AE8CC3 /* CommandBackForward.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CommandBackForward.mm; sourceTree = "<group>"; };
@@ -3181,6 +3183,8 @@
BCB9E7FA112359A300A137E0 /* Test.h */,
BC131AA8117131FC00B69727 /* TestsController.cpp */,
BCB9E7C711234E3A00A137E0 /* TestsController.h */,
+ 7BB754B0274E39A100D00EC1 /* TestUtilities.cpp */,
+ 7BB754AF274E39A100D00EC1 /* TestUtilities.h */,
7C83E0361D0A5F7000FEBCF3 /* Utilities.h */,
7BA3936D271EEC530015911C /* WebCoreUtilities.h */,
7CBD5A2222DE42A6004A9E32 /* WTFStringUtilities.cpp */,
diff --git a/Tools/TestWebKitAPI/Tests/WebCore/ImageBufferTests.cpp b/Tools/TestWebKitAPI/Tests/WebCore/ImageBufferTests.cpp
index 6737300..a1e5507 100644
--- a/Tools/TestWebKitAPI/Tests/WebCore/ImageBufferTests.cpp
+++ b/Tools/TestWebKitAPI/Tests/WebCore/ImageBufferTests.cpp
@@ -25,6 +25,7 @@
#include "config.h"
+#include "TestUtilities.h"
#include <WebCore/Color.h>
#include <WebCore/ImageBuffer.h>
#include <WebCore/PlatformImageBuffer.h>
@@ -34,17 +35,6 @@
namespace TestWebKitAPI {
using namespace WebCore;
-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();
-}
-
static ::testing::AssertionResult imageBufferPixelIs(Color expected, ImageBuffer& imageBuffer, int x, int y)
{
PixelBufferFormat format { AlphaPremultiplication::Unpremultiplied, PixelFormat::RGBA8, DestinationColorSpace::SRGB() };
@@ -52,7 +42,7 @@
auto& data = frontPixelBuffer->data();
auto got = Color { SRGBA<uint8_t> { data.item(0), data.item(1), data.item(2), data.item(3) } };
if (got != expected)
- return ::testing::AssertionFailure() << "color is not expected."; // FIXME: implement Color <<.
+ return ::testing::AssertionFailure() << "color is not expected. Got: " << got << ", expected: " << expected << ".";
return ::testing::AssertionSuccess();
}
diff --git a/Tools/TestWebKitAPI/Tests/WebCore/cocoa/TestGraphicsContextGLOpenGLCocoa.mm b/Tools/TestWebKitAPI/Tests/WebCore/cocoa/TestGraphicsContextGLOpenGLCocoa.mm
index 917d2e8..b933d51 100644
--- a/Tools/TestWebKitAPI/Tests/WebCore/cocoa/TestGraphicsContextGLOpenGLCocoa.mm
+++ b/Tools/TestWebKitAPI/Tests/WebCore/cocoa/TestGraphicsContextGLOpenGLCocoa.mm
@@ -27,27 +27,92 @@
#import "Test.h"
#if PLATFORM(COCOA) && ENABLE(WEBGL)
+#import "TestUtilities.h"
#import "WebCoreUtilities.h"
#import <Metal/Metal.h>
+#import <WebCore/Color.h>
#import <WebCore/GraphicsContextGLOpenGL.h>
+#import <optional>
+#import <wtf/HashSet.h>
+#import <wtf/MemoryFootprint.h>
namespace TestWebKitAPI {
-using namespace WebCore;
namespace {
-class TestedGraphicsContextGLOpenGL : public GraphicsContextGLOpenGL {
+
+class TestedGraphicsContextGLOpenGL : public WebCore::GraphicsContextGLOpenGL {
public:
- static RefPtr<TestedGraphicsContextGLOpenGL> create(GraphicsContextGLAttributes attributes)
+ static RefPtr<TestedGraphicsContextGLOpenGL> create(WebCore::GraphicsContextGLAttributes attributes)
{
auto context = adoptRef(*new TestedGraphicsContextGLOpenGL(WTFMove(attributes)));
return context;
}
+ WebCore::IOSurface* displayBuffer()
+ {
+ return m_swapChain.displayBuffer().surface.get();
+ }
+ void markDisplayBufferInUse()
+ {
+ m_swapChain.markDisplayBufferInUse();
+ }
private:
- TestedGraphicsContextGLOpenGL(GraphicsContextGLAttributes attributes)
- : GraphicsContextGLOpenGL(WTFMove(attributes))
+ TestedGraphicsContextGLOpenGL(WebCore::GraphicsContextGLAttributes attributes)
+ : WebCore::GraphicsContextGLOpenGL(WTFMove(attributes))
{
}
};
+
+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;
+};
+
+}
+
+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();
}
static bool hasMultipleGPUs()
@@ -68,29 +133,121 @@
// Tests for a bug where high-performance context would use low-power GPU if low-power or default
// context was created first. Test is applicable only for Metal, since GPU selection for OpenGL is
// very different.
-TEST(GraphicsContextGLOpenGLCocoaTest, MAYBE_MultipleGPUsDifferentPowerPreferenceMetal)
+TEST_F(GraphicsContextGLOpenGLCocoaTest, MAYBE_MultipleGPUsDifferentPowerPreferenceMetal)
{
if (!hasMultipleGPUs())
return;
- ScopedSetAuxiliaryProcessTypeForTesting scopedProcessType { AuxiliaryProcessType::GPU };
- GraphicsContextGLAttributes attributes;
+ WebCore::GraphicsContextGLAttributes attributes;
attributes.useMetal = true;
- EXPECT_EQ(attributes.powerPreference, GraphicsContextGLPowerPreference::Default);
+ EXPECT_EQ(attributes.powerPreference, WebCore::GraphicsContextGLPowerPreference::Default);
auto defaultContext = TestedGraphicsContextGLOpenGL::create(attributes);
- EXPECT_NE(defaultContext, nullptr);
+ ASSERT_NE(defaultContext, nullptr);
- attributes.powerPreference = GraphicsContextGLPowerPreference::LowPower;
+ attributes.powerPreference = WebCore::GraphicsContextGLPowerPreference::LowPower;
auto lowPowerContext = TestedGraphicsContextGLOpenGL::create(attributes);
- EXPECT_NE(lowPowerContext, nullptr);
+ ASSERT_NE(lowPowerContext, nullptr);
- attributes.powerPreference = GraphicsContextGLPowerPreference::HighPerformance;
+ attributes.powerPreference = WebCore::GraphicsContextGLPowerPreference::HighPerformance;
auto highPerformanceContext = TestedGraphicsContextGLOpenGL::create(attributes);
- EXPECT_NE(highPerformanceContext, nullptr);
+ ASSERT_NE(highPerformanceContext, nullptr);
- EXPECT_NE(lowPowerContext->getString(GraphicsContextGL::RENDERER), highPerformanceContext->getString(GraphicsContextGL::RENDERER));
- EXPECT_EQ(defaultContext->getString(GraphicsContextGL::RENDERER), lowPowerContext->getString(GraphicsContextGL::RENDERER));
+ EXPECT_NE(lowPowerContext->getString(WebCore::GraphicsContextGL::RENDERER), highPerformanceContext->getString(WebCore::GraphicsContextGL::RENDERER));
+ EXPECT_EQ(defaultContext->getString(WebCore::GraphicsContextGL::RENDERER), lowPowerContext->getString(WebCore::GraphicsContextGL::RENDERER));
+}
+
+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;
+ }
+}
+
+// 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));
}
}
+
#endif
