Changeset 277255 in webkit

Timestamp:

May 9, 2021, 6:48:09 PM (4 years ago)

Author:

weinig@apple.com

Message:

Add back protection of the pixel buffer in ImageBufferCGBackend::toCFData removed in r277237
​https://bugs.webkit.org/show_bug.cgi?id=225574

Reviewed by Darin Adler.

In r277237, I accidentally removed a RefPtr<Uint8ClampedArray> protectedPixelArray
in ImageBufferCGBackend::toCFData that was needed to avoided crashing in some cases
when running fast/canvas/canvas-toDataURL-jpeg-crash.html.

Since it wasn't super clear what it was doing, this switches to using the more idiomatic
method of keeping the data alive in a CGDataProviderRef by passing the leaked image data
as the context and derefing in the callback lambda.

Just to be consistent, I went to other callers of CGDataProviderCreateWithData and
updated them to be idiomatically consistent.

• platform/graphics/cg/GraphicsContextGLCG.cpp:

(WebCore::GraphicsContextGLOpenGL::paintToCanvas):
(WebCore::releaseImageData): Deleted.

• platform/graphics/cg/ImageBufferCGBackend.cpp:

(WebCore::ImageBufferCGBackend::toCFData const):

• platform/graphics/cg/ImageBufferCGBitmapBackend.cpp:

(WebCore::ImageBufferCGBitmapBackend::create):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/cg/GraphicsContextGLCG.cpp (modified) (2 diffs)
• platform/graphics/cg/ImageBufferCGBackend.cpp (modified) (1 diff)
• platform/graphics/cg/ImageBufferCGBitmapBackend.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-05-09  Sam Weinig  <weinig@apple.com>
+
+        Add back protection of the pixel buffer in ImageBufferCGBackend::toCFData removed in r277237
+        https://bugs.webkit.org/show_bug.cgi?id=225574
+
+        Reviewed by Darin Adler.
+
+        In r277237, I accidentally removed a `RefPtr<Uint8ClampedArray> protectedPixelArray`
+        in ImageBufferCGBackend::toCFData that was needed to avoided crashing in some cases
+        when running fast/canvas/canvas-toDataURL-jpeg-crash.html.
+
+        Since it wasn't super clear what it was doing, this switches to using the more idiomatic
+        method of keeping the data alive in a CGDataProviderRef by passing the leaked image data
+        as the context and derefing in the callback lambda.
+
+        Just to be consistent, I went to other callers of CGDataProviderCreateWithData and
+        updated them to be idiomatically consistent.
+
+        * platform/graphics/cg/GraphicsContextGLCG.cpp:
+        (WebCore::GraphicsContextGLOpenGL::paintToCanvas):
+        (WebCore::releaseImageData): Deleted.
+        * platform/graphics/cg/ImageBufferCGBackend.cpp:
+        (WebCore::ImageBufferCGBackend::toCFData const):
+        * platform/graphics/cg/ImageBufferCGBitmapBackend.cpp:
+        (WebCore::ImageBufferCGBitmapBackend::create):
+
 2021-05-09  Lauro Moura  <lmoura@igalia.com>
 

trunk/Source/WebCore/platform/graphics/cg/GraphicsContextGLCG.cpp

@@ -507 +507 @@
 }
 
-static void releaseImageData(void* imageData, const void*, size_t)
-{
-    reinterpret_cast<ImageData*>(imageData)->deref();
-}
-
 void GraphicsContextGLOpenGL::paintToCanvas(const GraphicsContextGLAttributes& sourceContextAttributes, Ref<ImageData>&& imageData, const IntSize& canvasSize, GraphicsContext& context)
 {
@@ -529 +524 @@
     auto imageSize = imageData->size();
     int rowBytes = imageSize.width() * 4;
-        size_t dataSize = rowBytes * imageSize.height();
+    size_t dataSize = rowBytes * imageSize.height();
     uint8_t* imagePixels = imageData->data().data();
-        verifyImageBufferIsBigEnough(imagePixels, dataSize);
-    RetainPtr<CGDataProviderRef> dataProvider = adoptCF(CGDataProviderCreateWithData(&imageData.leakRef(), imagePixels, dataSize, releaseImageData));
-
-    auto image = NativeImage::create(adoptCF(CGImageCreate(imageSize.width(), imageSize.height(), 8, 32, rowBytes, sRGBColorSpaceRef(), bitmapInfo,
-        dataProvider.get(), 0, false, kCGRenderingIntentDefault)));
+    verifyImageBufferIsBigEnough(imagePixels, dataSize);
+    auto dataProvider = adoptCF(CGDataProviderCreateWithData(&imageData.leakRef(), imagePixels, dataSize, [] (void* context, const void*, size_t) {
+        reinterpret_cast<ImageData*>(context)->deref();
+    }));
+
+    auto image = NativeImage::create(adoptCF(CGImageCreate(imageSize.width(), imageSize.height(), 8, 32, rowBytes, sRGBColorSpaceRef(), bitmapInfo, dataProvider.get(), 0, false, kCGRenderingIntentDefault)));
 
     // CSS styling may cause the canvas's content to be resized on

trunk/Source/WebCore/platform/graphics/cg/ImageBufferCGBackend.cpp

@@ -192 +192 @@
             return nullptr;
 
-        auto protectedPixelArray = makeRef(imageData->data());
-        size_t dataSize = protectedPixelArray->byteLength();
-        IntSize pixelArrayDimensions = imageData->size();
-
-        verifyImageBufferIsBigEnough(protectedPixelArray->data(), dataSize);
-        auto dataProvider = adoptCF(CGDataProviderCreateWithData(nullptr, protectedPixelArray->data(), dataSize, nullptr));
+        auto& pixelArray = imageData->data();
+        auto dataSize = pixelArray.byteLength();
+        auto pixelArrayDimensions = imageData->size();
+
+        verifyImageBufferIsBigEnough(pixelArray.data(), dataSize);
+
+        auto dataProvider = adoptCF(CGDataProviderCreateWithData(imageData.leakRef(), pixelArray.data(), dataSize, [] (void* context, const void*, size_t) {
+            reinterpret_cast<ImageData*>(context)->deref();
+        }));
+        
         if (!dataProvider)
             return nullptr;

trunk/Source/WebCore/platform/graphics/cg/ImageBufferCGBitmapBackend.cpp

@@ -88 +88 @@
     auto context = makeUnique<GraphicsContext>(cgContext.get());
 
-    const auto releaseImageData = [] (void*, const void* data, size_t) {
+    auto dataProvider = adoptCF(CGDataProviderCreateWithData(nullptr, data, numBytes, [] (void*, const void* data, size_t) {
         fastFree(const_cast<void*>(data));
-    };
-
-    auto dataProvider = adoptCF(CGDataProviderCreateWithData(0, data, numBytes, releaseImageData));
+    }));
 
     return std::unique_ptr<ImageBufferCGBitmapBackend>(new ImageBufferCGBitmapBackend(parameters, data, WTFMove(dataProvider), WTFMove(context)));