ImageBufferCreationContext is used in wrong ImageBuffer::create facto…

…ry function

https://bugs.webkit.org/show_bug.cgi?id=261259
rdar://115098672

Reviewed by Matt Woodrow.

ImageBufferCreationContext is ad hoc amalgamation of all parameters
accepted by any ImageBufferBackend, populated for the purpose of
constructing a specific backend.

ImageBuffer::create<Backend>(..., const ImageBufferCreationContext&)
is the concrete function constructing a specific ImageBuffer type with
a specific backend.

ImageBuffer::create(...) is a polymorphic factory, which invokes the
GraphicsClient, a polymorphic interface. It doesn't request a specific
backend, rather the polymorphism selects the backend based on policy.
This function shouldn't accept ImageBufferCreationContext, there's
nothing that can be populated by the caller, except fields incorrectly
added to ImageBufferCreationContext.

Remove ImageBufferCreationContext from ImageBuffer::create(...).
It cannot be populated from the call sites, as the call sites don't know
what type of buffers will be created -- that is the part that is abstracted.

Remove GraphicsClient from ImageBufferCreationContext. GraphicsClient
is the polymorphic factory, which is pointless in context of
ImageBufferCreationContext. ImageBufferCreationContext is used when
the concrete type is known at the call site.

Move ImageBufferCreationContext::avoidIOSurfaceSizeCheckInWebProcessForTesting
as ImageBufferOptions::AvoidBackendSizeCheckForTesting. The property
is a testing property, configuring the behavior of
ImageBuffer::create(...).

* Source/WebCore/html/CanvasBase.cpp:
(WebCore::CanvasBase::allocateImageBuffer const):
* Source/WebCore/html/HTMLVideoElement.cpp:
(WebCore::HTMLVideoElement::createBufferForPainting const):
* Source/WebCore/html/ImageBitmap.cpp:
(WebCore::ImageBitmap::createImageBuffer):
* Source/WebCore/page/FrameSnapshotting.cpp:
(WebCore::snapshotFrameRectWithClip):
* Source/WebCore/platform/graphics/ImageBuffer.cpp:
(WebCore::ImageBuffer::create):
* Source/WebCore/platform/graphics/ImageBuffer.h:
(WebCore::ImageBuffer::create):
(WebCore::ImageBufferCreationContext::ImageBufferCreationContext): Deleted.
* Source/WebCore/platform/graphics/cg/ImageBufferIOSurfaceBackend.cpp:
(WebCore::ImageBufferIOSurfaceBackend::create):
* Source/WebCore/platform/graphics/cg/ImageBufferIOSurfaceBackend.h:
* Source/WebCore/rendering/style/StyleFilterImage.cpp:
(WebCore::StyleFilterImage::image const):
* Source/WebCore/svg/graphics/SVGImage.cpp:
(WebCore::SVGImage::nativeImage):
* Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp:
(WebKit::RemoteRenderingBackend::moveToImageBuffer):
(WebKit::RemoteRenderingBackend::createImageBuffer):
* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStoreCollection.mm:
(WebKit::RemoteLayerBackingStoreCollection::allocateBufferForBackingStore):
* Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferShareableMappedIOSurfaceBackend.cpp:
(WebKit::ImageBufferShareableMappedIOSurfaceBackend::create):

Canonical link: https://commits.webkit.org/267792@main

Source/WebCore/html/CanvasBase.cpp

@@ -348,16 +348,14 @@ RefPtr<ImageBuffer> CanvasBase::allocateImageBuffer(bool avoidBackendSizeCheckFo
     OptionSet<ImageBufferOptions> bufferOptions;
     if (shouldAccelerate(area))
         bufferOptions.add(ImageBufferOptions::Accelerated);
-
+    if (avoidBackendSizeCheckForTesting)
+        bufferOptions.add(ImageBufferOptions::AvoidBackendSizeCheckForTesting);
     auto [colorSpace, pixelFormat] = [&] {
         if (renderingContext())
             return std::pair { renderingContext()->colorSpace(), renderingContext()->pixelFormat() };
         return std::pair { DestinationColorSpace::SRGB(), PixelFormat::BGRA8 };
     }();
-    ImageBufferCreationContext context = { };
-    context.graphicsClient = graphicsClient();
-    context.avoidIOSurfaceSizeCheckInWebProcessForTesting = avoidBackendSizeCheckForTesting;
-    return ImageBuffer::create(size(), RenderingPurpose::Canvas, 1, colorSpace, pixelFormat, bufferOptions, context);
+    return ImageBuffer::create(size(), RenderingPurpose::Canvas, 1, colorSpace, pixelFormat, bufferOptions, graphicsClient());
 }
 
 bool CanvasBase::shouldInjectNoiseBeforeReadback() const

Source/WebCore/html/HTMLVideoElement.cpp

@@ -294,7 +294,7 @@ RefPtr<ImageBuffer> HTMLVideoElement::createBufferForPainting(const FloatSize& s
     if (document().settings().displayListDrawingEnabled())
         bufferOptions.add(ImageBufferOptions::UseDisplayList);
 
-    return ImageBuffer::create(size, RenderingPurpose::MediaPainting, 1, colorSpace, pixelFormat, bufferOptions, { hostWindow });
+    return ImageBuffer::create(size, RenderingPurpose::MediaPainting, 1, colorSpace, pixelFormat, bufferOptions, hostWindow);
 }
 
 void HTMLVideoElement::paintCurrentFrameInContext(GraphicsContext& context, const FloatRect& destRect)

Source/WebCore/html/ImageBitmap.cpp

@@ -106,7 +106,7 @@ RefPtr<ImageBuffer> ImageBitmap::createImageBuffer(ScriptExecutionContext& scrip
     } else if (scriptExecutionContext.isWorkerGlobalScope())
         client = downcast<WorkerGlobalScope>(scriptExecutionContext).workerClient();
 
-    return ImageBuffer::create(size, RenderingPurpose::Canvas, resolutionScale, *imageBufferColorSpace, PixelFormat::BGRA8, bufferOptions, { client });
+    return ImageBuffer::create(size, RenderingPurpose::Canvas, resolutionScale, *imageBufferColorSpace, PixelFormat::BGRA8, bufferOptions, client);
 }
 
 void ImageBitmap::createCompletionHandler(ScriptExecutionContext& scriptExecutionContext, ImageBitmap::Source&& source, ImageBitmapOptions&& options, ImageBitmapCompletionHandler&& completionHandler)

Source/WebCore/page/FrameSnapshotting.cpp

@@ -120,7 +120,7 @@ RefPtr<ImageBuffer> snapshotFrameRectWithClip(LocalFrame& frame, const IntRect&
     auto purpose = options.flags.contains(SnapshotFlags::Shareable) ? RenderingPurpose::ShareableSnapshot : RenderingPurpose::Snapshot;
     auto hostWindow = (document->view() && document->view()->root()) ? document->view()->root()->hostWindow() : nullptr;
 
-    auto buffer = ImageBuffer::create(imageRect.size(), purpose, scaleFactor, options.colorSpace, options.pixelFormat, { }, { hostWindow });
+    auto buffer = ImageBuffer::create(imageRect.size(), purpose, scaleFactor, options.colorSpace, options.pixelFormat, { }, hostWindow);
     if (!buffer)
         return nullptr;
 

Source/WebCore/platform/graphics/ImageBuffer.cpp

@@ -52,9 +52,14 @@ namespace WebCore {
 static const float MaxClampedLength = 4096;
 static const float MaxClampedArea = MaxClampedLength * MaxClampedLength;
 
-RefPtr<ImageBuffer> ImageBuffer::create(const FloatSize& size, RenderingPurpose purpose, float resolutionScale, const DestinationColorSpace& colorSpace, PixelFormat pixelFormat, OptionSet<ImageBufferOptions> options, const ImageBufferCreationContext& creationContext)
+RefPtr<ImageBuffer> ImageBuffer::create(const FloatSize& size, RenderingPurpose purpose, float resolutionScale, const DestinationColorSpace& colorSpace, PixelFormat pixelFormat, OptionSet<ImageBufferOptions> options, GraphicsClient* graphicsClient)
 {
     RefPtr<ImageBuffer> imageBuffer;
+    ImageBufferCreationContext creationContext;
+#if HAVE(IOSURFACE)
+    if (graphicsClient)
+        creationContext.displayID = graphicsClient->displayID();
+#endif
 
     // Give UseDisplayList a higher precedence since it is a debug option.
     if (options.contains(ImageBufferOptions::UseDisplayList)) {
@@ -65,9 +70,10 @@ RefPtr<ImageBuffer> ImageBuffer::create(const FloatSize& size, RenderingPurpose
             imageBuffer = DisplayList::ImageBuffer::create<UnacceleratedImageBufferBackend>(size, resolutionScale, colorSpace, pixelFormat, purpose, creationContext);
     }
 
-    if (creationContext.graphicsClient && !imageBuffer) {
+    if (graphicsClient && !imageBuffer) {
         auto renderingMode = options.contains(ImageBufferOptions::Accelerated) ? RenderingMode::Accelerated : RenderingMode::Unaccelerated;
-        imageBuffer = creationContext.graphicsClient->createImageBuffer(size, renderingMode, purpose, resolutionScale, colorSpace, pixelFormat, creationContext.avoidIOSurfaceSizeCheckInWebProcessForTesting);
+        bool avoidBackendSizeCheckForTesting = options.contains(ImageBufferOptions::AvoidBackendSizeCheckForTesting);
+        imageBuffer = graphicsClient->createImageBuffer(size, renderingMode, purpose, resolutionScale, colorSpace, pixelFormat, avoidBackendSizeCheckForTesting);
     }
 
     if (imageBuffer)
@@ -89,7 +95,7 @@ template<typename BackendType, typename ImageBufferType, typename... Arguments>
 RefPtr<ImageBufferType> ImageBuffer::create(const FloatSize& size, const GraphicsContext& context, RenderingPurpose purpose, Arguments&&... arguments)
 {
     auto parameters = ImageBufferBackend::Parameters { size, 1, context.colorSpace(), PixelFormat::BGRA8, purpose };
-    auto backend = BackendType::create(parameters, { nullptr });
+    auto backend = BackendType::create(parameters, { });
     if (!backend)
         return nullptr;
     auto backendInfo = populateBackendInfo<BackendType>(parameters);

Source/WebCore/platform/graphics/ImageBuffer.h

@@ -29,6 +29,7 @@
 
 #include "ImageBufferAllocator.h"
 #include "ImageBufferBackend.h"
+#include "PlatformScreen.h"
 #include "ProcessIdentity.h"
 #include "RenderingMode.h"
 #include "RenderingResourceIdentifier.h"
@@ -51,51 +52,30 @@ class ScriptExecutionContext;
 
 enum class ImageBufferOptions : uint8_t {
     Accelerated     = 1 << 0,
-    UseDisplayList  = 1 << 1
+    UseDisplayList  = 1 << 1,
+    AvoidBackendSizeCheckForTesting = 1 << 2,
 };
 
 class SerializedImageBuffer;
 
 struct ImageBufferCreationContext {
-    // clang 13.1.6 throws errors if we use default initializers here.
-    GraphicsClient* graphicsClient;
 #if HAVE(IOSURFACE)
-    IOSurfacePool* surfacePool;
+    IOSurfacePool* surfacePool { nullptr };
+    PlatformDisplayID displayID { 0 };
 #endif
-    bool avoidIOSurfaceSizeCheckInWebProcessForTesting = false;
-
 #if ENABLE(CG_DISPLAY_LIST_BACKED_IMAGE_BUFFER)
     enum class UseCGDisplayListImageCache : bool { No, Yes };
-    UseCGDisplayListImageCache useCGDisplayListImageCache;
+    UseCGDisplayListImageCache useCGDisplayListImageCache { UseCGDisplayListImageCache::No };
 #endif
     WebCore::ProcessIdentity resourceOwner;
 
-    ImageBufferCreationContext(GraphicsClient* client = nullptr
-#if HAVE(IOSURFACE)
-        , IOSurfacePool* pool = nullptr
-#endif
-        , bool avoidCheck = false
-#if ENABLE(CG_DISPLAY_LIST_BACKED_IMAGE_BUFFER)
-        , UseCGDisplayListImageCache useCGDisplayListImageCache = UseCGDisplayListImageCache::No
-#endif
-        , WebCore::ProcessIdentity resourceOwner = { }
-    )
-        : graphicsClient(client)
-#if HAVE(IOSURFACE)
-        , surfacePool(pool)
-#endif
-        , avoidIOSurfaceSizeCheckInWebProcessForTesting(avoidCheck)
-#if ENABLE(CG_DISPLAY_LIST_BACKED_IMAGE_BUFFER)
-        , useCGDisplayListImageCache(useCGDisplayListImageCache)
-#endif
-        , resourceOwner(WTFMove(resourceOwner))
-    { }
+    ImageBufferCreationContext() = default; // To guarantee order in presence of ifdefs, use individual .property to initialize them.
 };
 
 class ImageBuffer : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<ImageBuffer> {
 public:
 
-    WEBCORE_EXPORT static RefPtr<ImageBuffer> create(const FloatSize&, RenderingPurpose, float resolutionScale, const DestinationColorSpace&, PixelFormat, OptionSet<ImageBufferOptions> = { }, const ImageBufferCreationContext& = { });
+    WEBCORE_EXPORT static RefPtr<ImageBuffer> create(const FloatSize&, RenderingPurpose, float resolutionScale, const DestinationColorSpace&, PixelFormat, OptionSet<ImageBufferOptions> = { }, GraphicsClient* graphicsClient = nullptr);
 
     template<typename BackendType, typename ImageBufferType = ImageBuffer, typename... Arguments>
     static RefPtr<ImageBufferType> create(const FloatSize& size, float resolutionScale, const DestinationColorSpace& colorSpace, PixelFormat pixelFormat, RenderingPurpose purpose, const ImageBufferCreationContext& creationContext, Arguments&&... arguments)

Source/WebCore/platform/graphics/cairo/ImageBufferCairoImageSurfaceBackend.cpp

@@ -100,7 +100,7 @@ std::unique_ptr<ImageBufferCairoImageSurfaceBackend> ImageBufferCairoImageSurfac
 
 std::unique_ptr<ImageBufferCairoImageSurfaceBackend> ImageBufferCairoImageSurfaceBackend::create(const Parameters& parameters, const GraphicsContext&)
 {
-    return ImageBufferCairoImageSurfaceBackend::create(parameters, nullptr);
+    return ImageBufferCairoImageSurfaceBackend::create(parameters, ImageBufferCreationContext { });
 }
 
 ImageBufferCairoImageSurfaceBackend::ImageBufferCairoImageSurfaceBackend(const Parameters& parameters, RefPtr<cairo_surface_t>&& surface)

Source/WebCore/platform/graphics/cg/ImageBufferIOSurfaceBackend.cpp

@@ -84,14 +84,13 @@ std::unique_ptr<ImageBufferIOSurfaceBackend> ImageBufferIOSurfaceBackend::create
     if (!surface)
         return nullptr;
 
-    auto displayID = creationContext.graphicsClient ? creationContext.graphicsClient->displayID() : 0;
-    RetainPtr<CGContextRef> cgContext = surface->createPlatformContext(displayID);
+    RetainPtr<CGContextRef> cgContext = surface->createPlatformContext(creationContext.displayID);
     if (!cgContext)
         return nullptr;
 
     CGContextClearRect(cgContext.get(), FloatRect(FloatPoint::zero(), backendSize));
 
-    return makeUnique<ImageBufferIOSurfaceBackend>(parameters, WTFMove(surface), WTFMove(cgContext), displayID, creationContext.surfacePool);
+    return std::unique_ptr<ImageBufferIOSurfaceBackend> { new ImageBufferIOSurfaceBackend { parameters, WTFMove(surface), WTFMove(cgContext), creationContext.displayID, creationContext.surfacePool } };
 }
 
 ImageBufferIOSurfaceBackend::ImageBufferIOSurfaceBackend(const Parameters& parameters, std::unique_ptr<IOSurface> surface, RetainPtr<CGContextRef> platformContext, PlatformDisplayID displayID, IOSurfacePool* ioSurfacePool)

Source/WebCore/platform/graphics/cg/ImageBufferIOSurfaceBackend.h

@@ -46,7 +46,6 @@ class WEBCORE_EXPORT ImageBufferIOSurfaceBackend : public ImageBufferCGBackend {
 
     static std::unique_ptr<ImageBufferIOSurfaceBackend> create(const Parameters&, const ImageBufferCreationContext&);
 
-    ImageBufferIOSurfaceBackend(const Parameters&, std::unique_ptr<IOSurface>, RetainPtr<CGContextRef> platformContext, PlatformDisplayID, IOSurfacePool*);
     ~ImageBufferIOSurfaceBackend();
 
     static constexpr RenderingMode renderingMode = RenderingMode::Accelerated;
@@ -56,6 +55,7 @@ class WEBCORE_EXPORT ImageBufferIOSurfaceBackend : public ImageBufferCGBackend {
     void flushContext() override;
 
 protected:
+    ImageBufferIOSurfaceBackend(const Parameters&, std::unique_ptr<IOSurface>, RetainPtr<CGContextRef> platformContext, PlatformDisplayID, IOSurfacePool*);
     CGContextRef ensurePlatformContext();
     // Returns true if flush happened.
     bool flushContextDraws();

Source/WebCore/rendering/style/StyleFilterImage.cpp

@@ -133,7 +133,7 @@ RefPtr<Image> StyleFilterImage::image(const RenderElement* renderer, const Float
 
     cssFilter->setFilterRegion(sourceImageRect);
 
-    auto sourceImage = ImageBuffer::create(size, RenderingPurpose::DOM, 1, DestinationColorSpace::SRGB(), PixelFormat::BGRA8, bufferOptionsForRendingMode(cssFilter->renderingMode()), { renderer->hostWindow() });
+    auto sourceImage = ImageBuffer::create(size, RenderingPurpose::DOM, 1, DestinationColorSpace::SRGB(), PixelFormat::BGRA8, bufferOptionsForRendingMode(cssFilter->renderingMode()), renderer->hostWindow());
     if (!sourceImage)
         return &Image::nullImage();
 

Source/WebCore/svg/graphics/SVGImage.cpp

@@ -228,7 +228,7 @@ RefPtr<NativeImage> SVGImage::nativeImage(const DestinationColorSpace& colorSpac
     if (auto contentRenderer = embeddedContentBox())
         hostWindow = contentRenderer->hostWindow();
 
-    auto imageBuffer = ImageBuffer::create(size(), RenderingPurpose::DOM, 1, colorSpace, PixelFormat::BGRA8, bufferOptions, { hostWindow });
+    auto imageBuffer = ImageBuffer::create(size(), RenderingPurpose::DOM, 1, colorSpace, PixelFormat::BGRA8, bufferOptions, hostWindow);
     if (!imageBuffer)
         return nullptr;
 

Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp

@@ -201,7 +201,7 @@ void RemoteRenderingBackend::moveToImageBuffer(RenderingResourceIdentifier image
 
     ASSERT(imageBufferIdentifier == imageBuffer->renderingResourceIdentifier());
 
-    ImageBufferCreationContext creationContext { nullptr };
+    ImageBufferCreationContext creationContext;
 #if HAVE(IOSURFACE)
     creationContext.surfacePool = &ioSurfacePool();
 #endif
@@ -214,7 +214,7 @@ void RemoteRenderingBackend::createImageBuffer(const FloatSize& logicalSize, Ren
 {
     assertIsCurrent(workQueue());
     RefPtr<ImageBuffer> imageBuffer;
-    ImageBufferCreationContext creationContext { nullptr };
+    ImageBufferCreationContext creationContext;
 #if HAVE(IOSURFACE)
     creationContext.surfacePool = &ioSurfacePool();
 #endif

Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStoreCollection.mm

@@ -255,8 +255,11 @@
 RefPtr<WebCore::ImageBuffer> RemoteLayerBackingStoreCollection::allocateBufferForBackingStore(const RemoteLayerBackingStore& backingStore)
 {
     switch (backingStore.type()) {
-    case RemoteLayerBackingStore::Type::IOSurface:
-        return WebCore::ImageBuffer::create<AcceleratedImageBufferShareableMappedBackend>(backingStore.size(), backingStore.scale(), backingStore.colorSpace(), backingStore.pixelFormat(), WebCore::RenderingPurpose::LayerBacking, { nullptr, &WebCore::IOSurfacePool::sharedPool() });
+    case RemoteLayerBackingStore::Type::IOSurface: {
+        ImageBufferCreationContext creationContext;
+        creationContext.surfacePool = &WebCore::IOSurfacePool::sharedPool();
+        return WebCore::ImageBuffer::create<AcceleratedImageBufferShareableMappedBackend>(backingStore.size(), backingStore.scale(), backingStore.colorSpace(), backingStore.pixelFormat(), WebCore::RenderingPurpose::LayerBacking, creationContext);
+    }
     case RemoteLayerBackingStore::Type::Bitmap:
         return WebCore::ImageBuffer::create<UnacceleratedImageBufferShareableBackend>(backingStore.size(), backingStore.scale(), backingStore.colorSpace(), backingStore.pixelFormat(), WebCore::RenderingPurpose::LayerBacking, { });
     }

Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferShareableMappedIOSurfaceBackend.cpp

@@ -59,7 +59,7 @@ std::unique_ptr<ImageBufferShareableMappedIOSurfaceBackend> ImageBufferShareable
 
     CGContextClearRect(cgContext.get(), FloatRect(FloatPoint::zero(), backendSize));
 
-    return makeUnique<ImageBufferShareableMappedIOSurfaceBackend>(parameters, WTFMove(surface), WTFMove(cgContext), 0, creationContext.surfacePool);
+    return std::unique_ptr<ImageBufferShareableMappedIOSurfaceBackend> { new ImageBufferShareableMappedIOSurfaceBackend { parameters, WTFMove(surface), WTFMove(cgContext), 0, creationContext.surfacePool } };
 }
 
 std::unique_ptr<ImageBufferShareableMappedIOSurfaceBackend> ImageBufferShareableMappedIOSurfaceBackend::create(const Parameters& parameters, ImageBufferBackendHandle handle)
@@ -77,7 +77,7 @@ std::unique_ptr<ImageBufferShareableMappedIOSurfaceBackend> ImageBufferShareable
         return nullptr;
 
     ASSERT(surface->colorSpace() == parameters.colorSpace);
-    return makeUnique<ImageBufferShareableMappedIOSurfaceBackend>(parameters, WTFMove(surface), WTFMove(cgContext), 0, nullptr);
+    return std::unique_ptr<ImageBufferShareableMappedIOSurfaceBackend> { new ImageBufferShareableMappedIOSurfaceBackend { parameters, WTFMove(surface), WTFMove(cgContext), 0, nullptr } };
 }
 
 ImageBufferBackendHandle ImageBufferShareableMappedIOSurfaceBackend::createBackendHandle(SharedMemory::Protection) const

Source/WebKit/WebProcess/WebPage/wc/DrawingAreaWC.cpp

@@ -412,7 +412,7 @@ RefPtr<ImageBuffer> DrawingAreaWC::createImageBuffer(FloatSize size)
 {
     if (WebProcess::singleton().shouldUseRemoteRenderingFor(RenderingPurpose::DOM))
         return Ref { m_webPage.get() }->ensureRemoteRenderingBackendProxy().createImageBuffer(size, RenderingMode::Unaccelerated, RenderingPurpose::DOM, 1, DestinationColorSpace::SRGB(), PixelFormat::BGRA8);
-    return ImageBuffer::create<UnacceleratedImageBufferShareableBackend>(size, 1, DestinationColorSpace::SRGB(), PixelFormat::BGRA8, RenderingPurpose::DOM, nullptr);
+    return ImageBuffer::create<UnacceleratedImageBufferShareableBackend>(size, 1, DestinationColorSpace::SRGB(), PixelFormat::BGRA8, RenderingPurpose::DOM, { });
 }
 
 void DrawingAreaWC::displayDidRefresh()