As part of converting imgIContainer::GetFrame to return a Moz2D SourceSurface instead of a Thebes gfxASurface, we should convert the Windows widget consumers of imgIContainer::GetFrame to act on a Moz2D SourceSurface instead of a Thebes gfxASurface.

Comment on attachment 8386067 [details] [diff] [review] patch Review of attachment 8386067 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/windows/nsImageClipboard.cpp @@ +129,5 @@ > + thebesSurface->GetAsReadableARGB32ImageSurface(); > + NS_ENSURE_TRUE(thebesImageSurface, NS_ERROR_FAILURE); > + > + RefPtr<DataSourceSurface> dataSurface = > + thebesImageSurface->CopyToB8G8R8A8DataSourceSurface(); Should use Factory::CreateWrappingDataSurface here instead of copying. ::: widget/windows/nsWindowGfx.cpp @@ +688,5 @@ > + } else { > + dataSurface = surface->GetDataSurface(); > + dataSurface->Map(DataSourceSurface::MapType::READ, &map); > + } > + NS_ENSURE_TRUE(dataSurface && map.mData, NS_ERROR_FAILURE); map.mData is probably uninitialized if Map() failed. We should check that return value instead. @@ +693,5 @@ > + MOZ_ASSERT(dataSurface->GetFormat() == SurfaceFormat::B8G8R8A8); > + > + uint8_t* data = nullptr; > + nsAutoArrayPtr<uint8_t> autoDeleteArray; > + if (map.mStride == 4 * sizeof(uint8_t) * iconSize.width) { gfx/2d/Tools.h has BytesPerPixel(SurfaceFormat) for this.

(In reply to Jonathan Watt [:jwatt] from comment #4) > https://hg.mozilla.org/integration/mozilla-inbound/rev/22e34e33e9ee hi sorry had to backout this change for win7/8 debug crashes like https://tbpl.mozilla.org/php/getParsedLog.php?id=35715170&tree=Mozilla-Inbound

This is because I added a "missing" Unmap() call and we now hit the MOZ_ASSERT(mIsMapped) in DataSourceSurfaceD2D::Unmap(). It seems DataSourceSurfaceD2D::Map doesn't actually set mIsMapped, so I don't see how this can work anywhere else in the codebase unless all those calls are either (a) not on a DataSourceSurfaceD2D DataSourceSurface, or (b) the Unmap() calls are missing. I've noticed that the latter is the case in some places (Unmap calls are missing). In other words I seem to have stumbled upon an existing DataSourceSurfaceD2D::Map bug, and some existing Unmap-missing bugs.

Relanded, now that bug 980272 is fixed. https://hg.mozilla.org/integration/mozilla-inbound/rev/4efacf2f947b