Skip to content

Commit

Permalink
Decoding an SVG off the main thread causes a crash
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=206055

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Rebaseline OffscreenCanvas SVG tests.

* web-platform-tests/html/canvas/offscreen/drawing-images-to-the-canvas/2d.drawImage.zerosource.image.worker-expected.txt:

Source/WebCore:

Use BitmapImage::create instead of Image::create in ImageBitmap. This
bypasses use of SVGImage and PDFDocumentImage, which are not safe to
use off the main thread.

No new tests, rebaselined existing tests.

* html/ImageBitmap.cpp:
(WebCore::ImageBitmap::createFromBuffer):
  Use BitmapImage instead of Image.

* platform/graphics/Image.cpp:
(WebCore::Image::create):
  Add main-thread assert on Image creation.

LayoutTests:

Enable OffscreenCanvas SVG tests.

* platform/glib/TestExpectations:


Canonical link: https://commits.webkit.org/237241@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@276895 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Chris Lord committed May 3, 2021
1 parent 56b7318 commit 8ba49be
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 10 deletions.
11 changes: 11 additions & 0 deletions LayoutTests/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
2021-05-03 Chris Lord <clord@igalia.com>

Decoding an SVG off the main thread causes a crash
https://bugs.webkit.org/show_bug.cgi?id=206055

Reviewed by Darin Adler.

Enable OffscreenCanvas SVG tests.

* platform/glib/TestExpectations:

2021-05-03 Diego Pino Garcia <dpino@igalia.com>

[GLIB][GTK] Unreviewed test gardening. Updated expectations with latest failures from GTK release bot.
Expand Down
11 changes: 11 additions & 0 deletions LayoutTests/imported/w3c/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
2021-05-03 Chris Lord <clord@igalia.com>

Decoding an SVG off the main thread causes a crash
https://bugs.webkit.org/show_bug.cgi?id=206055

Reviewed by Darin Adler.

Rebaseline OffscreenCanvas SVG tests.

* web-platform-tests/html/canvas/offscreen/drawing-images-to-the-canvas/2d.drawImage.zerosource.image.worker-expected.txt:

2021-05-02 Sam Weinig <weinig@apple.com>

Consider removing imported/w3c/canvas and canvas/philip (Part 2)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@

PASS drawImage with zero-sized source rectangle from image throws INDEX_SIZE_ERR
PASS drawImage with zero-sized source rectangle from image draws nothing without exception

2 changes: 0 additions & 2 deletions LayoutTests/platform/glib/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -710,8 +710,6 @@ webkit.org/b/224111 imported/w3c/web-platform-tests/mathml/presentation-markup/o
webkit.org/b/203146 fast/canvas/offscreen-enabled.html [ Pass ]
webkit.org/b/203146 http/wpt/offscreen-canvas [ Pass ]
webkit.org/b/203146 imported/w3c/web-platform-tests/html/canvas/offscreen [ Pass ]
webkit.org/b/206055 imported/w3c/web-platform-tests/html/canvas/offscreen/drawing-images-to-the-canvas/2d.drawImage.svg.worker.html [ Skip ]
webkit.org/b/206055 imported/w3c/web-platform-tests/html/canvas/offscreen/drawing-images-to-the-canvas/2d.drawImage.zerosource.image.worker.html [ Skip ]

# Console log lines may appear in a different order so we silence them.
imported/w3c/web-platform-tests/html/canvas/offscreen/convert-to-blob/offscreencanvas.convert.to.blob.w.html [ DumpJSConsoleLogInStdErr ]
Expand Down
21 changes: 21 additions & 0 deletions Source/WebCore/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,24 @@
2021-05-03 Chris Lord <clord@igalia.com>

Decoding an SVG off the main thread causes a crash
https://bugs.webkit.org/show_bug.cgi?id=206055

Reviewed by Darin Adler.

Use BitmapImage::create instead of Image::create in ImageBitmap. This
bypasses use of SVGImage and PDFDocumentImage, which are not safe to
use off the main thread.

No new tests, rebaselined existing tests.

* html/ImageBitmap.cpp:
(WebCore::ImageBitmap::createFromBuffer):
Use BitmapImage instead of Image.

* platform/graphics/Image.cpp:
(WebCore::Image::create):
Add main-thread assert on Image creation.

2021-05-03 Rob Buis <rbuis@igalia.com>

getPropertyValue for url path doesn't return the "#" character
Expand Down
9 changes: 2 additions & 7 deletions Source/WebCore/html/ImageBitmap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -736,12 +736,7 @@ void ImageBitmap::createFromBuffer(ScriptExecutionContext& scriptExecutionContex

auto sharedBuffer = SharedBuffer::create(static_cast<const char*>(arrayBuffer->data()), arrayBuffer->byteLength());
auto observer = ImageBitmapImageObserver::create(mimeType, expectedContentLength, sourceURL);
auto image = Image::create(observer.get());
if (!image) {
promise.reject(InvalidStateError, "The type of the argument to createImageBitmap is not supported");
return;
}

auto image = BitmapImage::create(observer.ptr());
auto result = image->setData(sharedBuffer.copyRef(), true);
if (result != EncodedDataStatus::Complete) {
promise.reject(InvalidStateError, "Cannot decode the data in the argument to createImageBitmap");
Expand All @@ -762,7 +757,7 @@ void ImageBitmap::createFromBuffer(ScriptExecutionContext& scriptExecutionContex
}

FloatRect destRect(FloatPoint(), outputSize);
bitmapData->context().drawImage(*image, destRect, sourceRectangle.releaseReturnValue(), { interpolationQualityForResizeQuality(options.resizeQuality), imageOrientationForOrientation(options.imageOrientation) });
bitmapData->context().drawImage(image, destRect, sourceRectangle.releaseReturnValue(), { interpolationQualityForResizeQuality(options.resizeQuality), imageOrientationForOrientation(options.imageOrientation) });

OptionSet<SerializationState> serializationState = SerializationState::OriginClean;
if (alphaPremultiplicationForPremultiplyAlpha(options.premultiplyAlpha) == AlphaPremultiplication::Premultiplied)
Expand Down
4 changes: 4 additions & 0 deletions Source/WebCore/platform/graphics/Image.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ Image& Image::nullImage()

RefPtr<Image> Image::create(ImageObserver& observer)
{
// SVGImage and PDFDocumentImage are not safe to use off the main thread.
// Workers can use BitmapImage directly.
ASSERT(isMainThread());

auto mimeType = observer.mimeType();
if (mimeType == "image/svg+xml")
return SVGImage::create(observer);
Expand Down

0 comments on commit 8ba49be

Please sign in to comment.