1 files changed, 33 insertions, 15 deletions

diff --git a/gfx/layers/client/TextureRecorded.cpp b/gfx/layers/client/TextureRecorded.cpp
index da4ca4f8f3..b3596efdc3 100644
--- a/gfx/layers/client/TextureRecorded.cpp
+++ b/gfx/layers/client/TextureRecorded.cpp
@@ -33,7 +33,7 @@ RecordedTextureData::~RecordedTextureData() {
   // We need the translator to drop its reference for the DrawTarget first,
   // because the TextureData might need to destroy its DrawTarget within a lock.
   mSnapshot = nullptr;
-  mSnapshotWrapper = nullptr;
+  DetachSnapshotWrapper();
   mDT = nullptr;
   mCanvasChild->CleanupTexture(mTextureId);
   mCanvasChild->RecordEvent(RecordedTextureDestruction(
@@ -92,10 +92,25 @@ bool RecordedTextureData::Lock(OpenMode aMode) {
   return true;
 }
 
+void RecordedTextureData::DetachSnapshotWrapper(bool aInvalidate,
+                                                bool aRelease) {
+  if (mSnapshotWrapper) {
+    // If the snapshot only has one ref, then we don't need to worry about
+    // copying before invalidation since it is about to be deleted. Otherwise,
+    // we need to ensure any internal data is appropriately copied before
+    // shmems are potentially overwritten if there are still existing users.
+    mCanvasChild->DetachSurface(mSnapshotWrapper,
+                                aInvalidate && !mSnapshotWrapper->hasOneRef());
+    if (aRelease) {
+      mSnapshotWrapper = nullptr;
+    }
+  }
+}
+
 void RecordedTextureData::Unlock() {
   if ((mLockedMode == OpenMode::OPEN_READ_WRITE) &&
       mCanvasChild->ShouldCacheDataSurface()) {
-    mSnapshotWrapper = nullptr;
+    DetachSnapshotWrapper();
     mSnapshot = mDT->Snapshot();
     mDT->DetachAllSnapshots();
     mCanvasChild->RecordEvent(RecordedCacheDataSurface(mSnapshot.get()));
@@ -108,11 +123,9 @@ void RecordedTextureData::Unlock() {
 
 already_AddRefed<gfx::DrawTarget> RecordedTextureData::BorrowDrawTarget() {
   if (mLockedMode & OpenMode::OPEN_WRITE) {
+    // The snapshot will be invalidated.
     mSnapshot = nullptr;
-    if (mSnapshotWrapper) {
-      mCanvasChild->DetachSurface(mSnapshotWrapper);
-      mSnapshotWrapper = nullptr;
-    }
+    DetachSnapshotWrapper(true);
   }
   return do_AddRef(mDT);
 }
@@ -122,18 +135,22 @@ void RecordedTextureData::EndDraw() {
   MOZ_ASSERT(mLockedMode == OpenMode::OPEN_READ_WRITE);
 
   if (mCanvasChild->ShouldCacheDataSurface()) {
-    mSnapshotWrapper = nullptr;
+    DetachSnapshotWrapper();
     mSnapshot = mDT->Snapshot();
     mCanvasChild->RecordEvent(RecordedCacheDataSurface(mSnapshot.get()));
   }
 }
 
 already_AddRefed<gfx::SourceSurface> RecordedTextureData::BorrowSnapshot() {
-  if (mSnapshotWrapper && (!mDT || !mDT->IsDirty())) {
-    // The DT is unmodified since the last time snapshot was borrowed, so it
-    // is safe to reattach the snapshot for shmem readbacks.
-    mCanvasChild->AttachSurface(mSnapshotWrapper);
-    return do_AddRef(mSnapshotWrapper);
+  if (mSnapshotWrapper) {
+    if (!mDT || !mDT->IsDirty()) {
+      // The DT is unmodified since the last time snapshot was borrowed, so it
+      // is safe to reattach the snapshot for shmem readbacks.
+      mCanvasChild->AttachSurface(mSnapshotWrapper);
+      return do_AddRef(mSnapshotWrapper);
+    }
+
+    DetachSnapshotWrapper();
   }
 
   // There are some failure scenarios where we have no DrawTarget and
@@ -153,9 +170,10 @@ already_AddRefed<gfx::SourceSurface> RecordedTextureData::BorrowSnapshot() {
 void RecordedTextureData::ReturnSnapshot(
     already_AddRefed<gfx::SourceSurface> aSnapshot) {
   RefPtr<gfx::SourceSurface> snapshot = aSnapshot;
-  if (mSnapshotWrapper) {
-    mCanvasChild->DetachSurface(mSnapshotWrapper);
-  }
+  // The snapshot needs to be marked detached but we keep the wrapper around
+  // so that it can be reused without repeatedly creating it and accidentally
+  // reading back data for each new instantiation.
+  DetachSnapshotWrapper(false, false);
 }
 
 void RecordedTextureData::Deallocate(LayersIPCChannel* aAllocator) {}