Bug 1414448: Protect the SourceSurfaceSkia destructor from racing with DrawTargetWillChange. r=dvander

MozReview-Commit-ID: 6UYDLUEoJZy

gfx/2d/DrawTargetSkia.cpp

@@ -50,6 +50,8 @@
 #include "ScaledFontDWrite.h"
 #endif
 
+using namespace std;
+
 namespace mozilla {
 namespace gfx {
 
@@ -276,6 +278,7 @@ GetSkImageForSurface(SourceSurface* aSurface, const Rect* aBounds = nullptr, con
 
 DrawTargetSkia::DrawTargetSkia()
   : mSnapshot(nullptr)
+  , mSnapshotLock{make_shared<Mutex>("DrawTargetSkia::mSnapshotLock")}
 #ifdef MOZ_WIDGET_COCOA
   , mCG(nullptr)
   , mColorSpace(nullptr)
@@ -316,7 +319,7 @@ DrawTargetSkia::Snapshot()
     } else {
       image = mSurface->makeImageSnapshot();
     }
-    if (!snapshot->InitFromImage(image, mFormat, this)) {
+    if (!snapshot->InitFromImage(image, mFormat, this, mSnapshotLock)) {
       return nullptr;
     }
     mSnapshot = snapshot;
@@ -2214,6 +2217,7 @@ DrawTargetSkia::CreateFilter(FilterType aType)
 void
 DrawTargetSkia::MarkChanged()
 {
+  MutexAutoLock lock(*mSnapshotLock);
   if (mSnapshot) {
     mSnapshot->DrawTargetWillChange();
     mSnapshot = nullptr;

gfx/2d/DrawTargetSkia.h

@@ -206,6 +206,7 @@ private:
   sk_sp<SkSurface> mSurface;
   SkCanvas* mCanvas;
   SourceSurfaceSkia* mSnapshot;
+  std::shared_ptr<Mutex> mSnapshotLock;
 
 #ifdef MOZ_WIDGET_COCOA
   friend class BorrowedCGContext;

gfx/2d/SourceSurfaceSkia.cpp

@@ -13,6 +13,8 @@
 #include "skia/include/core/SkData.h"
 #include "mozilla/CheckedInt.h"
 
+using namespace std;
+
 namespace mozilla {
 namespace gfx {
 
@@ -24,9 +26,12 @@ SourceSurfaceSkia::SourceSurfaceSkia()
 
 SourceSurfaceSkia::~SourceSurfaceSkia()
 {
-  if (mDrawTarget) {
-    mDrawTarget->SnapshotDestroyed();
-    mDrawTarget = nullptr;
+  if (mSnapshotLock) {
+    MutexAutoLock lock{*mSnapshotLock};
+    if (mDrawTarget) {
+      mDrawTarget->SnapshotDestroyed();
+      mDrawTarget = nullptr;
+    }
   }
 }
 
@@ -104,7 +109,8 @@ SourceSurfaceSkia::InitFromData(unsigned char* aData,
 bool
 SourceSurfaceSkia::InitFromImage(const sk_sp<SkImage>& aImage,
                                  SurfaceFormat aFormat,
-                                 DrawTargetSkia* aOwner)
+                                 DrawTargetSkia* aOwner,
+                                 shared_ptr<Mutex> aSnapshotLock)
 {
   if (!aImage) {
     return false;
@@ -137,6 +143,8 @@ SourceSurfaceSkia::InitFromImage(const sk_sp<SkImage>& aImage,
   mImage = aImage;
 
   if (aOwner) {
+    MOZ_ASSERT(aSnapshotLock);
+    mSnapshotLock = move(aSnapshotLock);
     mDrawTarget = aOwner;
   }
 
@@ -186,6 +194,10 @@ SourceSurfaceSkia::Unmap()
 void
 SourceSurfaceSkia::DrawTargetWillChange()
 {
+  // In this case synchronisation on destroy should be guaranteed!
+  MOZ_ASSERT(mSnapshotLock);
+  mSnapshotLock->AssertCurrentThreadOwns();
+
   MutexAutoLock lock(mChangeMutex);
   if (mDrawTarget) {
     // Raster snapshots do not use Skia's internal copy-on-write mechanism,

gfx/2d/SourceSurfaceSkia.h

@@ -18,6 +18,7 @@ namespace mozilla {
 namespace gfx {
 
 class DrawTargetSkia;
+class SnapshotLock;
 
 class SourceSurfaceSkia : public DataSourceSurface
 {
@@ -39,7 +40,8 @@ public:
 
   bool InitFromImage(const sk_sp<SkImage>& aImage,
                      SurfaceFormat aFormat = SurfaceFormat::UNKNOWN,
-                     DrawTargetSkia* aOwner = nullptr);
+                     DrawTargetSkia* aOwner = nullptr,
+                     std::shared_ptr<Mutex> aSnapshotLock = std::shared_ptr<Mutex>{});
 
   virtual uint8_t* GetData();
 
@@ -62,6 +64,7 @@ private:
   IntSize mSize;
   int32_t mStride;
   RefPtr<DrawTargetSkia> mDrawTarget;
+  std::shared_ptr<Mutex> mSnapshotLock;
   Mutex mChangeMutex;
 };