Bug 1167235 - Part 2 - Detach DrawTarget snapshots before unlocking TextureClient. r=Bas
authorNicolas Silva <nsilva@mozilla.com>
Fri, 01 Jul 2016 10:58:13 +0200
changeset 383203 53487e6b475af7829d3aa14a9d99ffe9e443774e
parent 383202 fff0d7d295e5bb6cb2eab7134bbabedc55d502d8
child 383204 31f3f2c5a759b92ac41f55c9616fb7a15700abf5
push id21963
push userdmitchell@mozilla.com
push dateFri, 01 Jul 2016 19:54:18 +0000
reviewersBas
bugs1167235
milestone50.0a1
Bug 1167235 - Part 2 - Detach DrawTarget snapshots before unlocking TextureClient. r=Bas
gfx/2d/2D.h
gfx/2d/DrawTargetCG.h
gfx/2d/DrawTargetCairo.h
gfx/2d/DrawTargetCapture.cpp
gfx/2d/DrawTargetCapture.h
gfx/2d/DrawTargetD2D1.h
gfx/2d/DrawTargetDual.cpp
gfx/2d/DrawTargetDual.h
gfx/2d/DrawTargetRecording.cpp
gfx/2d/DrawTargetRecording.h
gfx/2d/DrawTargetSkia.h
gfx/2d/DrawTargetTiled.cpp
gfx/2d/DrawTargetTiled.h
gfx/layers/client/TextureClient.cpp
gfx/layers/d3d11/TextureD3D11.cpp
--- a/gfx/2d/2D.h
+++ b/gfx/2d/2D.h
@@ -1170,16 +1170,27 @@ public:
   virtual void SetPermitSubpixelAA(bool aPermitSubpixelAA) {
     mPermitSubpixelAA = aPermitSubpixelAA;
   }
 
   bool GetPermitSubpixelAA() {
     return mPermitSubpixelAA;
   }
 
+  /**
+   * Ensures that no snapshot is still pointing to this DrawTarget's surface data.
+   *
+   * This can be useful if the DrawTarget is wrapped around data that it does not
+   * own, and for some reason the owner of the data has to make it temporarily
+   * unavailable without the DrawTarget knowing about it.
+   * This can cause costly surface copies, so it should not be used without a
+   * a good reason.
+   */
+  virtual void DetachAllSnapshots() = 0;
+
 #ifdef USE_SKIA_GPU
   virtual bool InitWithGrContext(GrContext* aGrContext,
                                  const IntSize &aSize,
                                  SurfaceFormat aFormat)
   {
     MOZ_CRASH("GFX: InitWithGrContext");
   }
 #endif
--- a/gfx/2d/DrawTargetCG.h
+++ b/gfx/2d/DrawTargetCG.h
@@ -123,16 +123,17 @@ public:
   friend class UnboundnessFixer;
   friend class SourceSurfaceCGBitmapContext;
   DrawTargetCG();
   virtual ~DrawTargetCG();
 
   virtual DrawTargetType GetType() const override;
   virtual BackendType GetBackendType() const override;
   virtual already_AddRefed<SourceSurface> Snapshot() override;
+  virtual void DetachAllSnapshots() override { MarkChanged(); }
 
   virtual void DrawSurface(SourceSurface *aSurface,
                            const Rect &aDest,
                            const Rect &aSource,
                            const DrawSurfaceOptions &aSurfOptions = DrawSurfaceOptions(),
                            const DrawOptions &aOptions = DrawOptions()) override;
   virtual void DrawFilter(FilterNode *aNode,
                           const Rect &aSourceRect,
--- a/gfx/2d/DrawTargetCairo.h
+++ b/gfx/2d/DrawTargetCairo.h
@@ -172,16 +172,18 @@ public:
   virtual void *GetNativeSurface(NativeSurfaceType aType) override;
 
   bool Init(cairo_surface_t* aSurface, const IntSize& aSize, SurfaceFormat* aFormat = nullptr);
   bool Init(const IntSize& aSize, SurfaceFormat aFormat);
   bool Init(unsigned char* aData, const IntSize &aSize, int32_t aStride, SurfaceFormat aFormat);
 
   virtual void SetTransform(const Matrix& aTransform) override;
 
+  virtual void DetachAllSnapshots() override { MarkSnapshotIndependent(); }
+
   // Call to set up aContext for drawing (with the current transform, etc).
   // Pass the path you're going to be using if you have one.
   // Implicitly calls WillChange(aPath).
   void PrepareForDrawing(cairo_t* aContext, const Path* aPath = nullptr);
 
   static cairo_surface_t *GetDummySurface();
 
   // Cairo hardcodes this as its maximum surface size.
--- a/gfx/2d/DrawTargetCapture.cpp
+++ b/gfx/2d/DrawTargetCapture.cpp
@@ -40,16 +40,20 @@ DrawTargetCaptureImpl::Snapshot()
 {
   RefPtr<DrawTarget> dt = mRefDT->CreateSimilarDrawTarget(mSize, mRefDT->GetFormat());
 
   ReplayToDrawTarget(dt, Matrix());
 
   return dt->Snapshot();
 }
 
+void
+DrawTargetCaptureImpl::DetachAllSnapshots()
+{}
+
 #define AppendCommand(arg) new (AppendToCommandList<arg>()) arg
 
 void
 DrawTargetCaptureImpl::DrawSurface(SourceSurface *aSurface,
                                    const Rect &aDest,
                                    const Rect &aSource,
                                    const DrawSurfaceOptions &aSurfOptions,
                                    const DrawOptions &aOptions)
--- a/gfx/2d/DrawTargetCapture.h
+++ b/gfx/2d/DrawTargetCapture.h
@@ -23,16 +23,19 @@ public:
   {}
 
   bool Init(const IntSize& aSize, DrawTarget* aRefDT);
 
   virtual BackendType GetBackendType() const { return mRefDT->GetBackendType(); }
   virtual DrawTargetType GetType() const { return mRefDT->GetType(); }
 
   virtual already_AddRefed<SourceSurface> Snapshot();
+
+  virtual void DetachAllSnapshots();
+
   virtual IntSize GetSize() { return mSize; }
 
   virtual void Flush() {}
   virtual void DrawSurface(SourceSurface *aSurface,
                            const Rect &aDest,
                            const Rect &aSource,
                            const DrawSurfaceOptions &aSurfOptions,
                            const DrawOptions &aOptions);
--- a/gfx/2d/DrawTargetD2D1.h
+++ b/gfx/2d/DrawTargetD2D1.h
@@ -122,16 +122,18 @@ public:
 
   virtual already_AddRefed<FilterNode> CreateFilter(FilterType aType) override;
 
   virtual bool SupportsRegionClipping() const override { return false; }
   virtual bool IsCurrentGroupOpaque() override { return CurrentLayer().mIsOpaque; }
 
   virtual void *GetNativeSurface(NativeSurfaceType aType) override { return nullptr; }
 
+  virtual void DetachAllSnapshots() override { MarkChanged(); }
+
   bool Init(const IntSize &aSize, SurfaceFormat aFormat);
   bool Init(ID3D11Texture2D* aTexture, SurfaceFormat aFormat);
   uint32_t GetByteSize() const;
 
   // This function will get an image for a surface, it may adjust the source
   // transform for any transformation of the resulting image relative to the
   // oritingal SourceSurface.
   already_AddRefed<ID2D1Image> GetImageForSurface(SourceSurface *aSurface, Matrix &aSourceTransform,
--- a/gfx/2d/DrawTargetDual.cpp
+++ b/gfx/2d/DrawTargetDual.cpp
@@ -82,16 +82,23 @@ public:
 
   const Pattern *mA;
   const Pattern *mB;
 
   bool mPatternsInitialized;
 };
 
 void
+DrawTargetDual::DetachAllSnapshots()
+{
+  mA->DetachAllSnapshots();
+  mB->DetachAllSnapshots();
+}
+
+void
 DrawTargetDual::DrawSurface(SourceSurface *aSurface, const Rect &aDest, const Rect &aSource,
                             const DrawSurfaceOptions &aSurfOptions, const DrawOptions &aOptions)
 {
   DualSurface surface(aSurface);
   mA->DrawSurface(surface.mA, aDest, aSource, aSurfOptions, aOptions);
   mB->DrawSurface(surface.mB, aDest, aSource, aSurfOptions, aOptions);
 }
 
--- a/gfx/2d/DrawTargetDual.h
+++ b/gfx/2d/DrawTargetDual.h
@@ -44,17 +44,19 @@ public:
   }
      
   virtual DrawTargetType GetType() const override { return mA->GetType(); }
   virtual BackendType GetBackendType() const override { return mA->GetBackendType(); }
   virtual already_AddRefed<SourceSurface> Snapshot() override {
     return MakeAndAddRef<SourceSurfaceDual>(mA, mB);
   }
   virtual IntSize GetSize() override { return mA->GetSize(); }
-     
+
+  virtual void DetachAllSnapshots() override;
+
   FORWARD_FUNCTION(Flush)
   FORWARD_FUNCTION1(PushClip, const Path *, aPath)
   FORWARD_FUNCTION1(PushClipRect, const Rect &, aRect)
   FORWARD_FUNCTION(PopClip)
   FORWARD_FUNCTION(PopLayer)
   FORWARD_FUNCTION1(ClearRect, const Rect &, aRect)
 
   virtual void SetTransform(const Matrix &aTransform) override {
--- a/gfx/2d/DrawTargetRecording.cpp
+++ b/gfx/2d/DrawTargetRecording.cpp
@@ -473,16 +473,22 @@ DrawTargetRecording::Snapshot()
   RefPtr<SourceSurface> retSurf = new SourceSurfaceRecording(surf, mRecorder);
 
   mRecorder->RecordEvent(RecordedSnapshot(retSurf, this));
 
   return retSurf.forget();
 }
 
 void
+DrawTargetRecording::DetachAllSnapshots()
+{
+  mFinalDT->DetachAllSnapshots();
+}
+
+void
 DrawTargetRecording::DrawSurface(SourceSurface *aSurface,
                                  const Rect &aDest,
                                  const Rect &aSource,
                                  const DrawSurfaceOptions &aSurfOptions,
                                  const DrawOptions &aOptions)
 {
   EnsureSurfaceStored(mRecorder, aSurface, "DrawSurface");
 
--- a/gfx/2d/DrawTargetRecording.h
+++ b/gfx/2d/DrawTargetRecording.h
@@ -32,16 +32,18 @@ public:
   ~DrawTargetRecording();
 
   virtual DrawTargetType GetType() const override { return mFinalDT->GetType(); }
   virtual BackendType GetBackendType() const override { return mFinalDT->GetBackendType(); }
   virtual bool IsRecording() const override { return true; }
 
   virtual already_AddRefed<SourceSurface> Snapshot() override;
 
+  virtual void DetachAllSnapshots() override;
+
   virtual IntSize GetSize() override { return mFinalDT->GetSize(); }
 
   /* Ensure that the DrawTarget backend has flushed all drawing operations to
    * this draw target. This must be called before using the backing surface of
    * this draw target outside of GFX 2D code.
    */
   virtual void Flush() override { mFinalDT->Flush(); }
 
--- a/gfx/2d/DrawTargetSkia.h
+++ b/gfx/2d/DrawTargetSkia.h
@@ -120,16 +120,17 @@ public:
     CreateSourceSurfaceFromNativeSurface(const NativeSurface &aSurface) const override;
   virtual already_AddRefed<DrawTarget>
     CreateSimilarDrawTarget(const IntSize &aSize, SurfaceFormat aFormat) const override;
   virtual already_AddRefed<PathBuilder> CreatePathBuilder(FillRule aFillRule = FillRule::FILL_WINDING) const override;
   virtual already_AddRefed<GradientStops> CreateGradientStops(GradientStop *aStops, uint32_t aNumStops, ExtendMode aExtendMode = ExtendMode::CLAMP) const override;
   virtual already_AddRefed<FilterNode> CreateFilter(FilterType aType) override;
   virtual void SetTransform(const Matrix &aTransform) override;
   virtual void *GetNativeSurface(NativeSurfaceType aType) override;
+  virtual void DetachAllSnapshots() override { MarkChanged(); }
 
   bool Init(const IntSize &aSize, SurfaceFormat aFormat);
   void Init(unsigned char* aData, const IntSize &aSize, int32_t aStride, SurfaceFormat aFormat, bool aUninitialized = false);
 
 #ifdef USE_SKIA_GPU
   bool InitWithGrContext(GrContext* aGrContext,
                          const IntSize &aSize,
                          SurfaceFormat aFormat,
--- a/gfx/2d/DrawTargetTiled.cpp
+++ b/gfx/2d/DrawTargetTiled.cpp
@@ -49,16 +49,20 @@ DrawTargetTiled::Init(const TileSet& aTi
 }
 
 already_AddRefed<SourceSurface>
 DrawTargetTiled::Snapshot()
 {
   return MakeAndAddRef<SnapshotTiled>(mTiles, mRect);
 }
 
+void
+DrawTargetTiled::DetachAllSnapshots()
+{}
+
 // Skip the mClippedOut check since this is only used for Flush() which
 // should happen even if we're clipped.
 #define TILED_COMMAND(command) \
   void \
   DrawTargetTiled::command() \
   { \
     for (size_t i = 0; i < mTiles.size(); i++) { \
       mTiles[i].mDrawTarget->command(); \
--- a/gfx/2d/DrawTargetTiled.h
+++ b/gfx/2d/DrawTargetTiled.h
@@ -36,16 +36,17 @@ public:
 
   bool Init(const TileSet& mTiles);
 
   virtual bool IsTiledDrawTarget() const override { return true; }
 
   virtual DrawTargetType GetType() const override { return mTiles[0].mDrawTarget->GetType(); }
   virtual BackendType GetBackendType() const override { return mTiles[0].mDrawTarget->GetBackendType(); }
   virtual already_AddRefed<SourceSurface> Snapshot() override;
+  virtual void DetachAllSnapshots() override;
   virtual IntSize GetSize() override {
     MOZ_ASSERT(mRect.width > 0 && mRect.height > 0);
     return IntSize(mRect.XMost(), mRect.YMost());
   }
 
   virtual void Flush() override;
   virtual void DrawSurface(SourceSurface *aSurface,
                            const Rect &aDest,
--- a/gfx/layers/client/TextureClient.cpp
+++ b/gfx/layers/client/TextureClient.cpp
@@ -489,27 +489,32 @@ TextureClient::Unlock()
 {
   MOZ_ASSERT(IsValid());
   MOZ_ASSERT(mIsLocked);
   if (!mIsLocked) {
     return;
   }
 
   if (mBorrowedDrawTarget) {
-    MOZ_ASSERT(mBorrowedDrawTarget->refCount() <= mExpectedDtRefs);
     if (mOpenMode & OpenMode::OPEN_WRITE) {
       mBorrowedDrawTarget->Flush();
       if (mReadbackSink && !mData->ReadBack(mReadbackSink)) {
         // Fallback implementation for reading back, because mData does not
         // have a backend-specific implementation and returned false.
         RefPtr<SourceSurface> snapshot = mBorrowedDrawTarget->Snapshot();
         RefPtr<DataSourceSurface> dataSurf = snapshot->GetDataSurface();
         mReadbackSink->ProcessReadback(dataSurf);
       }
     }
+
+    mBorrowedDrawTarget->DetachAllSnapshots();
+    // If this assertion is hit, it means something is holding a strong reference
+    // to our DrawTarget externally, which is not allowed.
+    MOZ_ASSERT(mBorrowedDrawTarget->refCount() <= mExpectedDtRefs);
+
     mBorrowedDrawTarget = nullptr;
   }
 
   if (mOpenMode & OpenMode::OPEN_WRITE) {
     mUpdated = true;
   }
 
   mData->Unlock();
--- a/gfx/layers/d3d11/TextureD3D11.cpp
+++ b/gfx/layers/d3d11/TextureD3D11.cpp
@@ -392,16 +392,17 @@ D3D11TextureData::Create(IntSize aSize, 
                               aFlags & ALLOC_CLEAR_BUFFER,
                               aFlags & ALLOC_CLEAR_BUFFER_WHITE,
                               aFlags & ALLOC_FOR_OUT_OF_BAND_CONTENT);
 }
 
 void
 D3D11TextureData::Deallocate(ClientIPCAllocator* aAllocator)
 {
+  mDrawTarget = nullptr;
   mTexture = nullptr;
 }
 
 already_AddRefed<TextureClient>
 CreateD3D11TextureClientWithDevice(IntSize aSize, SurfaceFormat aFormat,
                                    TextureFlags aTextureFlags, TextureAllocationFlags aAllocFlags,
                                    ID3D11Device* aDevice,
                                    ClientIPCAllocator* aAllocator)