Bug 1224199 - Destroy SharedSurfaces before ~GLContext(). - r=jrmuizel a=lizzard FIREFOX_45_2_0esr_BUILD2 FIREFOX_45_2_0esr_RELEASE
authorJeff Gilbert <jgilbert@mozilla.com>
Thu, 14 Apr 2016 13:50:04 -0700
changeset 329275 dc190bd03d24c00fdccafdff0298c92f89060b6b
parent 329274 b24e1cc592ecd4a34548809262acad9e297fc921
child 329492 c49261fc0fc141a2469702c028edfd49767caa33
push idunknown
push userunknown
push dateunknown
reviewersjrmuizel, lizzard
bugs1224199
milestone45.2.0
Bug 1224199 - Destroy SharedSurfaces before ~GLContext(). - r=jrmuizel a=lizzard
gfx/gl/GLBlitHelper.cpp
gfx/gl/GLContext.cpp
gfx/gl/GLContext.h
gfx/gl/GLReadTexImageHelper.cpp
gfx/gl/SharedSurfaceANGLE.cpp
gfx/gl/SharedSurfaceD3D11Interop.cpp
gfx/gl/SharedSurfaceEGL.cpp
gfx/gl/SharedSurfaceGLX.cpp
gfx/gl/SharedSurfaceGralloc.cpp
gfx/gl/SharedSurfaceIO.cpp
gfx/gl/TextureGarbageBin.cpp
--- a/gfx/gl/GLBlitHelper.cpp
+++ b/gfx/gl/GLBlitHelper.cpp
@@ -63,16 +63,19 @@ GLBlitHelper::GLBlitHelper(GLContext* gl
     , mTexHeight(0)
     , mCurYScale(1.0f)
     , mCurCbCrScale(1.0f)
 {
 }
 
 GLBlitHelper::~GLBlitHelper()
 {
+    if (!mGL->MakeCurrent())
+        return;
+
     DeleteTexBlitProgram();
 
     GLuint tex[] = {
         mSrcTexY,
         mSrcTexCb,
         mSrcTexCr,
         mSrcTexEGL,
     };
--- a/gfx/gl/GLContext.cpp
+++ b/gfx/gl/GLContext.cpp
@@ -2333,22 +2333,23 @@ GLContext::ClearSafely()
 }
 
 void
 GLContext::MarkDestroyed()
 {
     if (IsDestroyed())
         return;
 
+    // Null these before they're naturally nulled after dtor, as we want GLContext to
+    // still be alive in *their* dtors.
+    mScreen = nullptr;
+    mBlitHelper = nullptr;
+    mReadTexImageHelper = nullptr;
+
     if (MakeCurrent()) {
-        DestroyScreenBuffer();
-
-        mBlitHelper = nullptr;
-        mReadTexImageHelper = nullptr;
-
         mTexGarbageBin->GLContextTeardown();
     } else {
         NS_WARNING("MakeCurrent() failed during MarkDestroyed! Skipping GL object teardown.");
     }
 
     mSymbols.Zero();
 }
 
@@ -2582,18 +2583,16 @@ GLContext::CreateScreenBufferImpl(const 
     UniquePtr<GLScreenBuffer> newScreen = GLScreenBuffer::Create(this, size, caps);
     if (!newScreen)
         return false;
 
     if (!newScreen->Resize(size)) {
         return false;
     }
 
-    DestroyScreenBuffer();
-
     // This will rebind to 0 (Screen) if needed when
     // it falls out of scope.
     ScopedBindFramebuffer autoFB(this);
 
     mScreen = Move(newScreen);
 
     return true;
 }
@@ -2603,22 +2602,16 @@ GLContext::ResizeScreenBuffer(const IntS
 {
     if (!IsOffscreenSizeAllowed(size))
         return false;
 
     return mScreen->Resize(size);
 }
 
 void
-GLContext::DestroyScreenBuffer()
-{
-    mScreen = nullptr;
-}
-
-void
 GLContext::ForceDirtyScreen()
 {
     ScopedBindFramebuffer autoFB(0);
 
     BeforeGLDrawCall();
     // no-op; just pretend we did something
     AfterGLDrawCall();
 }
--- a/gfx/gl/GLContext.h
+++ b/gfx/gl/GLContext.h
@@ -3449,18 +3449,16 @@ public:
                               const GLuint texture,
                               GLuint* drawFB,
                               GLuint* readFB);
 
 protected:
     friend class GLScreenBuffer;
     UniquePtr<GLScreenBuffer> mScreen;
 
-    void DestroyScreenBuffer();
-
     SharedSurface* mLockedSurface;
 
 public:
     void LockSurface(SharedSurface* surf) {
         MOZ_ASSERT(!mLockedSurface);
         mLockedSurface = surf;
     }
 
--- a/gfx/gl/GLReadTexImageHelper.cpp
+++ b/gfx/gl/GLReadTexImageHelper.cpp
@@ -27,16 +27,19 @@ GLReadTexImageHelper::GLReadTexImageHelp
     mPrograms[0] = 0;
     mPrograms[1] = 0;
     mPrograms[2] = 0;
     mPrograms[3] = 0;
 }
 
 GLReadTexImageHelper::~GLReadTexImageHelper()
 {
+    if (!mGL->MakeCurrent())
+        return;
+
     mGL->fDeleteProgram(mPrograms[0]);
     mGL->fDeleteProgram(mPrograms[1]);
     mGL->fDeleteProgram(mPrograms[2]);
     mGL->fDeleteProgram(mPrograms[3]);
 }
 
 static const GLchar
 readTextureImageVS[] =
--- a/gfx/gl/SharedSurfaceANGLE.cpp
+++ b/gfx/gl/SharedSurfaceANGLE.cpp
@@ -112,18 +112,20 @@ SharedSurface_ANGLEShareHandle::SharedSu
 {
 }
 
 
 SharedSurface_ANGLEShareHandle::~SharedSurface_ANGLEShareHandle()
 {
     mEGL->fDestroySurface(Display(), mPBuffer);
 
+    if (!mGL->MakeCurrent())
+        return;
+
     if (mFence) {
-        mGL->MakeCurrent();
         mGL->fDeleteFences(1, &mFence);
     }
 }
 
 void
 SharedSurface_ANGLEShareHandle::LockProdImpl()
 {
     GLContextEGL::Cast(mGL)->SetEGLSurfaceOverride(mPBuffer);
--- a/gfx/gl/SharedSurfaceD3D11Interop.cpp
+++ b/gfx/gl/SharedSurfaceD3D11Interop.cpp
@@ -301,27 +301,29 @@ SharedSurface_D3D11Interop::SharedSurfac
     , mFence(fence)
     , mLockedForGL(false)
 { }
 
 SharedSurface_D3D11Interop::~SharedSurface_D3D11Interop()
 {
     MOZ_ASSERT(!mLockedForGL);
 
-    mGL->fDeleteRenderbuffers(1, &mProdRB);
-
     if (!mDXGL->UnregisterObject(mObjectWGL)) {
         NS_WARNING("Failed to release a DXGL object, possibly leaking it.");
     }
 
+    if (!mGL->MakeCurrent())
+        return;
+
     if (mFence) {
-        mGL->MakeCurrent();
         mGL->fDeleteFences(1, &mFence);
     }
 
+    mGL->fDeleteRenderbuffers(1, &mProdRB);
+
     // mDXGL is closed when it runs out of refs.
 }
 
 void
 SharedSurface_D3D11Interop::LockProdImpl()
 { }
 
 void
--- a/gfx/gl/SharedSurfaceEGL.cpp
+++ b/gfx/gl/SharedSurfaceEGL.cpp
@@ -83,32 +83,34 @@ SharedSurface_EGLImage::SharedSurface_EG
     , mConsTex(0)
     , mSync(0)
 {}
 
 SharedSurface_EGLImage::~SharedSurface_EGLImage()
 {
     mEGL->fDestroyImage(Display(), mImage);
 
-    mGL->MakeCurrent();
-    mGL->fDeleteTextures(1, &mProdTex);
-    mProdTex = 0;
+    if (mSync) {
+        // We can't call this unless we have the ext, but we will always have
+        // the ext if we have something to destroy.
+        mEGL->fDestroySync(Display(), mSync);
+        mSync = 0;
+    }
 
     if (mConsTex) {
         MOZ_ASSERT(mGarbageBin);
         mGarbageBin->Trash(mConsTex);
         mConsTex = 0;
     }
 
-    if (mSync) {
-        // We can't call this unless we have the ext, but we will always have
-        // the ext if we have something to destroy.
-        mEGL->fDestroySync(Display(), mSync);
-        mSync = 0;
-    }
+    if (!mGL->MakeCurrent())
+        return;
+
+    mGL->fDeleteTextures(1, &mProdTex);
+    mProdTex = 0;
 }
 
 layers::TextureFlags
 SharedSurface_EGLImage::GetTextureFlags() const
 {
     return layers::TextureFlags::DEALLOCATE_CLIENT;
 }
 
--- a/gfx/gl/SharedSurfaceGLX.cpp
+++ b/gfx/gl/SharedSurfaceGLX.cpp
@@ -72,21 +72,21 @@ void
 SharedSurface_GLXDrawable::UnlockProdImpl()
 {
     GLContextGLX::Cast(mGL)->RestoreDrawable();
 }
 
 bool
 SharedSurface_GLXDrawable::ToSurfaceDescriptor(layers::SurfaceDescriptor* const out_descriptor)
 {
-  if (!mXlibSurface)
-      return false;
+    if (!mXlibSurface)
+        return false;
 
-   *out_descriptor = layers::SurfaceDescriptorX11(mXlibSurface, mInSameProcess);
-   return true;
+    *out_descriptor = layers::SurfaceDescriptorX11(mXlibSurface, mInSameProcess);
+    return true;
 }
 
 bool
 SharedSurface_GLXDrawable::ReadbackBySharedHandle(gfx::DataSourceSurface* out_surface)
 {
     MOZ_ASSERT(out_surface);
     RefPtr<gfx::DataSourceSurface> dataSurf =
         new gfx::DataSourceSurfaceCairo(mXlibSurface->CairoSurface());
--- a/gfx/gl/SharedSurfaceGralloc.cpp
+++ b/gfx/gl/SharedSurfaceGralloc.cpp
@@ -130,30 +130,30 @@ SharedSurface_Gralloc::SharedSurface_Gra
     , mEGL(egl)
     , mSync(0)
     , mAllocator(allocator)
     , mTextureClient(textureClient)
     , mProdTex(prodTex)
 {
 }
 
-
 bool
 SharedSurface_Gralloc::HasExtensions(GLLibraryEGL* egl, GLContext* gl)
 {
     return egl->HasKHRImageBase() &&
            gl->IsExtensionSupported(GLContext::OES_EGL_image);
 }
 
 SharedSurface_Gralloc::~SharedSurface_Gralloc()
 {
-
     DEBUG_PRINT("[SharedSurface_Gralloc %p] destroyed\n", this);
 
-    mGL->MakeCurrent();
+    if (!mGL->MakeCurrent())
+        return;
+
     mGL->fDeleteTextures(1, &mProdTex);
 
     if (mSync) {
         MOZ_ALWAYS_TRUE( mEGL->fDestroySync(mEGL->Display(), mSync) );
         mSync = 0;
     }
 }
 
--- a/gfx/gl/SharedSurfaceIO.cpp
+++ b/gfx/gl/SharedSurfaceIO.cpp
@@ -160,21 +160,20 @@ SharedSurface_IOSurface::SharedSurface_I
     gl->MakeCurrent();
     mProdTex = 0;
     gl->fGenTextures(1, &mProdTex);
     BackTextureWithIOSurf(gl, mProdTex, mIOSurf);
 }
 
 SharedSurface_IOSurface::~SharedSurface_IOSurface()
 {
-    if (mProdTex) {
-        DebugOnly<bool> success = mGL->MakeCurrent();
-        MOZ_ASSERT(success);
-        mGL->fDeleteTextures(1, &mProdTex);
-    }
+    if (!mGL->MakeCurrent())
+        return;
+
+    mGL->fDeleteTextures(1, &mProdTex);
 }
 
 bool
 SharedSurface_IOSurface::ToSurfaceDescriptor(layers::SurfaceDescriptor* const out_descriptor)
 {
     bool isOpaque = !mHasAlpha;
     *out_descriptor = layers::SurfaceDescriptorMacIOSurface(mIOSurf->GetIOSurfaceID(),
                                                             mIOSurf->GetContentsScaleFactor(),
--- a/gfx/gl/TextureGarbageBin.cpp
+++ b/gfx/gl/TextureGarbageBin.cpp
@@ -31,14 +31,15 @@ TextureGarbageBin::Trash(GLuint tex)
 
 void
 TextureGarbageBin::EmptyGarbage()
 {
     MutexAutoLock lock(mMutex);
     if (!mGL)
         return;
 
+    MOZ_RELEASE_ASSERT(mGL->IsCurrent());
     while (!mGarbageTextures.empty()) {
         GLuint tex = mGarbageTextures.top();
         mGarbageTextures.pop();
         mGL->fDeleteTextures(1, &tex);
     }
 }