Bug 1432375 - Part 2. Images decoded into an SourceSurfaceSharedData should be shared immediately. r=nical

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<SharedMemoryBasic> 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<char*>(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<char*>(mBuf->memory()), len, RightsRead);
+#endif
 
   mFinalized = true;
-  CloseHandleInternal();
 }
 
 } // namespace gfx

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();
 

image/RasterImage.cpp

@@ -577,12 +577,7 @@ RasterImage::GetFrameAtSize(const IntSize& aSize,
 #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>>

image/VectorImage.cpp

@@ -776,12 +776,7 @@ VectorImage::GetFrameAtSize(const IntSize& aSize,
 #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>>
@@ -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;
 }
 

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<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)
@@ -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;
 }
 

image/imgFrame.h

@@ -527,8 +527,6 @@ private:
   RefPtr<imgFrame> mFrame;
 };
 
-void MarkSurfaceShared(gfx::SourceSurface* aSurface);
-
 } // namespace image
 } // namespace mozilla