Bug 1432375 - Part 2. Images decoded into an SourceSurfaceSharedData should be shared immediately. r=nical
authorAndrew Osmond <aosmond@mozilla.com>
Mon, 12 Feb 2018 07:59:58 -0500
changeset 403442 87ba2465c82e5b354d8f4aafe37deba610cf192b
parent 403441 07e679fc2e70dc451f8d2c129d11f0bcfd247dd0
child 403443 e1daef0276f6506fd41a31fe2f9a10e7187c2934
push id33433
push useraciure@mozilla.com
push dateMon, 12 Feb 2018 22:08:56 +0000
treeherdermozilla-central@6d8f470b2579 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical
bugs1432375
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1432375 - Part 2. Images decoded into an SourceSurfaceSharedData should be shared immediately. r=nical
gfx/layers/SourceSurfaceSharedData.cpp
gfx/layers/SourceSurfaceSharedData.h
image/RasterImage.cpp
image/VectorImage.cpp
image/imgFrame.cpp
image/imgFrame.h
--- a/gfx/layers/SourceSurfaceSharedData.cpp
+++ b/gfx/layers/SourceSurfaceSharedData.cpp
@@ -3,16 +3,27 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "SourceSurfaceSharedData.h"
 
 #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 {
 
 bool
 SourceSurfaceSharedDataWrapper::Init(const IntSize& aSize,
                                      int32_t aStride,
                                      SurfaceFormat aFormat,
@@ -47,30 +58,35 @@ SourceSurfaceSharedDataWrapper::Init(Sou
   mFormat = aSurface->mFormat;
   mCreatorPid = base::GetCurrentProcId();
   mBuf = aSurface->mBuf;
 }
 
 bool
 SourceSurfaceSharedData::Init(const IntSize &aSize,
                               int32_t aStride,
-                              SurfaceFormat aFormat)
+                              SurfaceFormat aFormat,
+                              bool aShare /* = true */)
 {
   mSize = aSize;
   mStride = aStride;
   mFormat = aFormat;
 
   size_t len = GetAlignedDataLength();
   mBuf = new SharedMemoryBasic();
   if (NS_WARN_IF(!mBuf->Create(len)) ||
       NS_WARN_IF(!mBuf->Map(len))) {
     mBuf = nullptr;
     return false;
   }
 
+  if (aShare) {
+    layers::SharedSurfacesChild::Share(this);
+  }
+
   return true;
 }
 
 void
 SourceSurfaceSharedData::GuaranteePersistance()
 {
   // Shared memory is not unmapped until we release SourceSurfaceSharedData.
 }
@@ -121,63 +137,71 @@ SourceSurfaceSharedData::ShareToProcess(
 
 void
 SourceSurfaceSharedData::CloseHandleInternal()
 {
   mMutex.AssertCurrentThreadOwns();
 
   if (mClosed) {
     MOZ_ASSERT(mHandleCount == 0);
-    MOZ_ASSERT(mFinalized);
     MOZ_ASSERT(mShared);
     return;
   }
 
-  if (mFinalized && mShared) {
+  if (mShared) {
     mBuf->CloseHandle();
     mClosed = true;
   }
 }
 
 bool
 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<SharedMemoryBasic> buf = new SharedMemoryBasic();
   if (NS_WARN_IF(!buf->Create(len)) ||
       NS_WARN_IF(!buf->Map(len))) {
     return false;
   }
 
   size_t copyLen = GetDataLength();
   memcpy(buf->memory(), mBuf->memory(), copyLen);
+#ifdef SHARED_SURFACE_PROTECT_FINALIZED
   buf->Protect(static_cast<char*>(buf->memory()), len, RightsRead);
+#endif
 
   if (mMapCount > 0 && !mOldBuf) {
     mOldBuf = Move(mBuf);
   }
   mBuf = Move(buf);
   mClosed = false;
   mShared = false;
   return true;
 }
 
 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<char*>(mBuf->memory()), len, RightsRead);
+#endif
 
   mFinalized = true;
-  CloseHandleInternal();
 }
 
 } // namespace gfx
 } // namespace mozilla
--- a/gfx/layers/SourceSurfaceSharedData.h
+++ b/gfx/layers/SourceSurfaceSharedData.h
@@ -131,19 +131,26 @@ public:
     , mInvalidations(0)
     , mFormat(SurfaceFormat::UNKNOWN)
     , mClosed(false)
     , mFinalized(false)
     , mShared(false)
   {
   }
 
+  /**
+   * 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
   {
     MutexAutoLock lock(mMutex);
     return GetDataInternal();
   }
 
   int32_t Stride() override { return mStride; }
@@ -230,17 +237,17 @@ public:
    * sharing to new processes. ShareToProcess must have failed with
    * NS_ERROR_NOT_AVAILABLE in order for this to be safe to call. Returns true
    * if the operation succeeds. If it fails, there is no state change.
    */
   bool ReallocHandle();
 
   /**
    * 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();
 
   /**
    * Indicates whether or not the buffer can change. If this returns true, it is
    * guaranteed to continue to do so for the remainder of the surface's life.
    */
   bool IsFinalized() const
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -572,22 +572,17 @@ RasterImage::GetFrameAtSize(const IntSiz
                             uint32_t aWhichFrame,
                             uint32_t aFlags)
 {
 #ifdef DEBUG
   NotifyDrawingObservers();
 #endif
 
   auto result = GetFrameInternal(aSize, Nothing(), aWhichFrame, aFlags);
-  RefPtr<SourceSurface> 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<ImgDrawResult, IntSize, RefPtr<SourceSurface>>
 RasterImage::GetFrameInternal(const IntSize& aSize,
                               const Maybe<SVGImageContext>& aSVGContext,
                               uint32_t aWhichFrame,
                               uint32_t aFlags)
 {
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -771,22 +771,17 @@ VectorImage::GetFrameAtSize(const IntSiz
                             uint32_t aWhichFrame,
                             uint32_t aFlags)
 {
 #ifdef DEBUG
   NotifyDrawingObservers();
 #endif
 
   auto result = GetFrameInternal(aSize, Nothing(), aWhichFrame, aFlags);
-  RefPtr<SourceSurface> 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<ImgDrawResult, IntSize, RefPtr<SourceSurface>>
 VectorImage::GetFrameInternal(const IntSize& aSize,
                               const Maybe<SVGImageContext>& aSVGContext,
                               uint32_t aWhichFrame,
                               uint32_t aFlags)
 {
@@ -1037,20 +1032,16 @@ VectorImage::Draw(gfxContext* aContext,
     Show(svgDrawable, params);
     return ImgDrawResult::SUCCESS;
   }
 
   RefPtr<gfxDrawable> drawable =
     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;
 }
 
 already_AddRefed<gfxDrawable>
 VectorImage::CreateSVGDrawable(const SVGDrawingParameters& aParams)
 {
   RefPtr<gfxDrawingCallback> cb =
     new SVGDrawingCallback(mSVGDocumentWrapper,
--- a/image/imgFrame.cpp
+++ b/image/imgFrame.cpp
@@ -133,29 +133,16 @@ ClearSurface(DataSourceSurface* aSurface
     // Otherwise, it's allocated via mmap and refers to a zeroed page and will
     // be COW once it's written to.
     memset(data, 0, stride * aSize.height);
   }
 
   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<SourceSurfaceSharedData*>(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)
 {
   // reject over-wide or over-tall images
   const int32_t k64KLimit = 0x0000FFFF;
   if (MOZ_UNLIKELY(aWidth > k64KLimit || aHeight > k64KLimit )) {
     NS_WARNING("image too big");
@@ -581,19 +568,16 @@ bool imgFrame::Draw(gfxContext* aContext
     SurfaceForDrawing(doPartialDecode, doTile, region, surf);
 
   if (surfaceResult.IsValid()) {
     gfxUtils::DrawPixelSnapped(aContext, surfaceResult.mDrawable,
                                imageRect.Size(), region, surfaceResult.mFormat,
                                aSamplingFilter, aImageFlags, aOpacity);
   }
 
-  // Image got put into a painted layer, it will not be shared with another
-  // process.
-  MarkSurfaceShared(surf);
   return true;
 }
 
 nsresult
 imgFrame::ImageUpdated(const nsIntRect& aUpdateRect)
 {
   MonitorAutoLock lock(mMonitor);
   return ImageUpdatedInternal(aUpdateRect);
--- a/image/imgFrame.h
+++ b/image/imgFrame.h
@@ -522,14 +522,12 @@ public:
   }
 
 private:
   RawAccessFrameRef(const RawAccessFrameRef& aOther) = delete;
 
   RefPtr<imgFrame> mFrame;
 };
 
-void MarkSurfaceShared(gfx::SourceSurface* aSurface);
-
 } // namespace image
 } // namespace mozilla
 
 #endif // mozilla_image_imgFrame_h