corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1739454 - Don't overwrite the ImageBitmap's surface in PrepareForDrawTarget. r=emilio

Browse Source 
ImageBitmap::PrepareForDrawTarget overwrites its surface's contents when premultiplying the
data. This code pattern seems to stem all the way back to bug 1239752. It doesn't seem like
we have much control over where this surface comes from, and we have no strong guarantees
that this surface isn't shared by multiple consumers. As such, overwriting the data could
result in another consumer getting premultiplied data in the surface that it didn't expect.
We should just always be cloning the surface first before premultiplying unless we can
otherwise prove the surface isn't shared.

* * *
Bug 1739454 - Remove reftest-wait in reftest. r?emilio

Differential Revision: https://phabricator.services.mozilla.com/D131583
This commit is contained in:
Lee Salzman
2021-11-19 18:46:46 +00:00
parent af93252e61
commit a4a8855aa9
3 changed files with 39 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
dom/canvas/ImageBitmap.cpp
View File

@@ -635,44 +635,31 @@ already_AddRefed<SourceSurface> ImageBitmap::PrepareForDrawTarget(
mSurface->GetFormat() == SurfaceFormat::B8G8R8A8 || mSurface->GetFormat() == SurfaceFormat::B8G8R8A8 ||
mSurface->GetFormat() == SurfaceFormat::A8R8G8B8); mSurface->GetFormat() == SurfaceFormat::A8R8G8B8);
RefPtr<DataSourceSurface> dstSurface = mSurface->GetDataSurface(); RefPtr<DataSourceSurface> srcSurface = mSurface->GetDataSurface();
MOZ_ASSERT(dstSurface); RefPtr<DataSourceSurface> dstSurface = Factory::CreateDataSourceSurface(
srcSurface->GetSize(), srcSurface->GetFormat());
if (NS_WARN_IF(!srcSurface) || NS_WARN_IF(!dstSurface)) {
return nullptr;
}
RefPtr<DataSourceSurface> srcSurface; DataSourceSurface::ScopedMap srcMap(srcSurface, DataSourceSurface::READ);
DataSourceSurface::MappedSurface srcMap; if (!srcMap.IsMapped()) {
DataSourceSurface::MappedSurface dstMap;
if (dstSurface->Map(DataSourceSurface::MapType::READ_WRITE, &dstMap)) {
srcMap = dstMap;
} else {
srcSurface = dstSurface;
if (!srcSurface->Map(DataSourceSurface::READ, &srcMap)) {
gfxCriticalError() gfxCriticalError()
<< "Failed to map source surface for premultiplying alpha."; << "Failed to map source surface for premultiplying alpha.";
return nullptr; return nullptr;
} }
dstSurface = Factory::CreateDataSourceSurface(srcSurface->GetSize(), DataSourceSurface::ScopedMap dstMap(dstSurface, DataSourceSurface::WRITE);
srcSurface->GetFormat()); if (!dstMap.IsMapped()) {
if (!dstSurface ||
!dstSurface->Map(DataSourceSurface::MapType::WRITE, &dstMap)) {
gfxCriticalError() gfxCriticalError()
<< "Failed to map destination surface for premultiplying alpha."; << "Failed to map destination surface for premultiplying alpha.";
srcSurface->Unmap();
return nullptr; return nullptr;
} }
}
PremultiplyData(srcMap.mData, srcMap.mStride, mSurface->GetFormat(), PremultiplyData(srcMap.GetData(), srcMap.GetStride(), mSurface->GetFormat(),
dstMap.mData, dstMap.mStride, mSurface->GetFormat(), dstMap.GetData(), dstMap.GetStride(), mSurface->GetFormat(),
dstSurface->GetSize()); dstSurface->GetSize());
dstSurface->Unmap();
if (srcSurface) {
srcSurface->Unmap();
}
mAlphaType = gfxAlphaType::Premult; mAlphaType = gfxAlphaType::Premult;
mSurface = dstSurface; mSurface = dstSurface;
} }
@@ -1761,15 +1748,10 @@ CreateImageBitmapFromBlob::OnImageReady(imgIContainer* aImgContainer,
RefPtr<SourceSurface> croppedSurface = surface; RefPtr<SourceSurface> croppedSurface = surface;
RefPtr<DataSourceSurface> dataSurface = surface->GetDataSurface(); RefPtr<DataSourceSurface> dataSurface = surface->GetDataSurface();
#ifdef DEBUG
// the returned dataSurface image memory is write protected in debug mode
// force a copy into unprotected memory as a side effect of // force a copy into unprotected memory as a side effect of
// CropAndCopyDataSourceSurface // CropAndCopyDataSourceSurface
bool copyRequired = mCropRect.isSome() || bool copyRequired = mCropRect.isSome() ||
mOptions.mImageOrientation == ImageOrientation::FlipY; mOptions.mImageOrientation == ImageOrientation::FlipY;
#else
bool copyRequired = mCropRect.isSome();
#endif
if (copyRequired) { if (copyRequired) {
// The blob is just decoded into a RasterImage and not optimized yet, so the // The blob is just decoded into a RasterImage and not optimized yet, so the

17
dom/canvas/crashtests/1739454-1.html Normal file
View File

@@ -0,0 +1,17 @@
<!DOCTYPE html>
<html class="reftest-wait">
<script>
document.addEventListener('DOMContentLoaded', async () => {
cnv = document.createElement('canvas')
ctx = cnv.getContext('2d')
try {
a = await fetch('');
b = await a.arrayBuffer();
c = await self.createImageBitmap(new Blob([b]), {'premultiplyAlpha':'none'})
ctx.createPattern(c, 'repeat-x')
} finally {
document.documentElement.className = "";
}
})
</script>
</html>

1
dom/canvas/crashtests/crashtests.list
View File

@@ -57,3 +57,4 @@ load 1549853.html
load 1551745.html load 1551745.html
load 1569648.html load 1569648.html
skip-if(!winWidget||!isDebugBuild) pref(layers.gpu-process.crash-also-crashes-browser,true) load 1654477.html skip-if(!winWidget||!isDebugBuild) pref(layers.gpu-process.crash-also-crashes-browser,true) load 1654477.html
load 1739454-1.html