From 556768640554f80bfad184beb8441758ebe4143f Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Mon, 12 Feb 2018 07:59:58 -0500 Subject: [PATCH] Bug 1432375 - Part 2. Images decoded into an SourceSurfaceSharedData should be shared immediately. r=nical --- gfx/layers/SourceSurfaceSharedData.cpp | 36 +++++++++++++++++++++----- gfx/layers/SourceSurfaceSharedData.h | 11 ++++++-- image/RasterImage.cpp | 7 +---- image/VectorImage.cpp | 11 +------- image/imgFrame.cpp | 16 ------------ image/imgFrame.h | 2 -- 6 files changed, 41 insertions(+), 42 deletions(-) diff --git a/gfx/layers/SourceSurfaceSharedData.cpp b/gfx/layers/SourceSurfaceSharedData.cpp index 63d9f7440d57..fa2ee03f1f66 100644 --- a/gfx/layers/SourceSurfaceSharedData.cpp +++ b/gfx/layers/SourceSurfaceSharedData.cpp @@ -8,6 +8,17 @@ #include "mozilla/Likely.h" #include "mozilla/Types.h" // for decltype +#include "mozilla/layers/SharedSurfacesChild.h" + +#ifdef DEBUG +/** + * If defined, this makes SourceSurfaceSharedData::Finalize memory protect the + * underlying shared buffer in the producing process (the content or UI + * process). Given flushing the page table is expensive, and its utility is + * predominantly diagnostic (in case of overrun), turn it off by default. + */ +#define SHARED_SURFACE_PROTECT_FINALIZED +#endif namespace mozilla { namespace gfx { @@ -52,7 +63,8 @@ SourceSurfaceSharedDataWrapper::Init(SourceSurfaceSharedData* aSurface) bool SourceSurfaceSharedData::Init(const IntSize &aSize, int32_t aStride, - SurfaceFormat aFormat) + SurfaceFormat aFormat, + bool aShare /* = true */) { mSize = aSize; mStride = aStride; @@ -66,6 +78,10 @@ SourceSurfaceSharedData::Init(const IntSize &aSize, return false; } + if (aShare) { + layers::SharedSurfacesChild::Share(this); + } + return true; } @@ -126,12 +142,11 @@ SourceSurfaceSharedData::CloseHandleInternal() if (mClosed) { MOZ_ASSERT(mHandleCount == 0); - MOZ_ASSERT(mFinalized); MOZ_ASSERT(mShared); return; } - if (mFinalized && mShared) { + if (mShared) { mBuf->CloseHandle(); mClosed = true; } @@ -143,7 +158,14 @@ SourceSurfaceSharedData::ReallocHandle() MutexAutoLock lock(mMutex); MOZ_ASSERT(mHandleCount > 0); MOZ_ASSERT(mClosed); - MOZ_ASSERT(mFinalized); + + if (NS_WARN_IF(!mFinalized)) { + // We haven't finished populating the surface data yet, which means we are + // out of luck, as we have no means of synchronizing with the producer to + // write new data to a new buffer. This should be fairly rare, caused by a + // crash in the GPU process, while we were decoding an image. + return false; + } size_t len = GetAlignedDataLength(); RefPtr buf = new SharedMemoryBasic(); @@ -154,7 +176,9 @@ SourceSurfaceSharedData::ReallocHandle() size_t copyLen = GetDataLength(); memcpy(buf->memory(), mBuf->memory(), copyLen); +#ifdef SHARED_SURFACE_PROTECT_FINALIZED buf->Protect(static_cast(buf->memory()), len, RightsRead); +#endif if (mMapCount > 0 && !mOldBuf) { mOldBuf = Move(mBuf); @@ -169,14 +193,14 @@ void SourceSurfaceSharedData::Finalize() { MutexAutoLock lock(mMutex); - MOZ_ASSERT(!mClosed); MOZ_ASSERT(!mFinalized); +#ifdef SHARED_SURFACE_PROTECT_FINALIZED size_t len = GetAlignedDataLength(); mBuf->Protect(static_cast(mBuf->memory()), len, RightsRead); +#endif mFinalized = true; - CloseHandleInternal(); } } // namespace gfx diff --git a/gfx/layers/SourceSurfaceSharedData.h b/gfx/layers/SourceSurfaceSharedData.h index 30cfdd6eb3c3..6d0c6a0786a0 100644 --- a/gfx/layers/SourceSurfaceSharedData.h +++ b/gfx/layers/SourceSurfaceSharedData.h @@ -136,9 +136,16 @@ public: { } + /** + * Initialize the surface by creating a shared memory buffer with a size + * determined by aSize, aStride and aFormat. If aShare is true, it will also + * immediately attempt to share the surface with the GPU process via + * SharedSurfacesChild. + */ bool Init(const IntSize& aSize, int32_t aStride, - SurfaceFormat aFormat); + SurfaceFormat aFormat, + bool aShare = true); uint8_t* GetData() override { @@ -235,7 +242,7 @@ public: /** * Signals we have finished writing to the buffer and it may be marked as - * read only. May release the handle if possible (see CloseHandleInternal). + * read only. */ void Finalize(); diff --git a/image/RasterImage.cpp b/image/RasterImage.cpp index b7595d84bbf0..9698a06d231a 100644 --- a/image/RasterImage.cpp +++ b/image/RasterImage.cpp @@ -577,12 +577,7 @@ RasterImage::GetFrameAtSize(const IntSize& aSize, #endif auto result = GetFrameInternal(aSize, Nothing(), aWhichFrame, aFlags); - RefPtr surf = mozilla::Get<2>(result).forget(); - - // If we are here, it suggests the image is embedded in a canvas or some - // other path besides layers, and we won't need the file handle. - MarkSurfaceShared(surf); - return surf.forget(); + return mozilla::Get<2>(result).forget(); } Tuple> diff --git a/image/VectorImage.cpp b/image/VectorImage.cpp index 924049a507a9..d54fbfa408a0 100644 --- a/image/VectorImage.cpp +++ b/image/VectorImage.cpp @@ -776,12 +776,7 @@ VectorImage::GetFrameAtSize(const IntSize& aSize, #endif auto result = GetFrameInternal(aSize, Nothing(), aWhichFrame, aFlags); - RefPtr surf = Get<2>(result).forget(); - - // If we are here, it suggests the image is embedded in a canvas or some - // other path besides layers, and we won't need the file handle. - MarkSurfaceShared(surf); - return surf.forget(); + return Get<2>(result).forget(); } Tuple> @@ -1042,10 +1037,6 @@ VectorImage::Draw(gfxContext* aContext, new gfxSurfaceDrawable(sourceSurface, params.size); Show(drawable, params); SendFrameComplete(didCache, params.flags); - - // Image got put into a painted layer, it will not be shared with another - // process. - MarkSurfaceShared(sourceSurface); return ImgDrawResult::SUCCESS; } diff --git a/image/imgFrame.cpp b/image/imgFrame.cpp index 9d201b8a4be5..13f66d494ca2 100644 --- a/image/imgFrame.cpp +++ b/image/imgFrame.cpp @@ -138,19 +138,6 @@ ClearSurface(DataSourceSurface* aSurface, const IntSize& aSize, SurfaceFormat aF return true; } -void -MarkSurfaceShared(SourceSurface* aSurface) -{ - // Depending on what requested the image decoding, the buffer may or may not - // end up being shared with another process (e.g. put in a painted layer, - // used inside a canvas). If not shared, we should ensure are not keeping the - // handle only because we have yet to share it. - if (aSurface && aSurface->GetType() == SurfaceType::DATA_SHARED) { - auto sharedSurface = static_cast(aSurface); - sharedSurface->FinishedSharing(); - } -} - // Returns true if an image of aWidth x aHeight is allowed and legal. static bool AllowedImageSize(int32_t aWidth, int32_t aHeight) @@ -586,9 +573,6 @@ bool imgFrame::Draw(gfxContext* aContext, const ImageRegion& aRegion, aSamplingFilter, aImageFlags, aOpacity); } - // Image got put into a painted layer, it will not be shared with another - // process. - MarkSurfaceShared(surf); return true; } diff --git a/image/imgFrame.h b/image/imgFrame.h index bd858e088dcf..4ad106a289d3 100644 --- a/image/imgFrame.h +++ b/image/imgFrame.h @@ -527,8 +527,6 @@ private: RefPtr mFrame; }; -void MarkSurfaceShared(gfx::SourceSurface* aSurface); - } // namespace image } // namespace mozilla