Bug 1224199 - SharedSurfaces shouldn't outlive their GLContext. - r=jrmuizel a=ritu
authorJeff Gilbert <jgilbert@mozilla.com>
Fri, 22 Apr 2016 15:50:46 -0700
changeset 324117 1385868f8904b7e5f0cc338d2b79225f35e406cd
parent 324116 922334c70324e4295dcfcfafe6da4ae2e1154834
child 324118 d10b04bf30160a46f2053338a33877639c511299
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel, ritu
bugs1224199
milestone47.0a2
Bug 1224199 - SharedSurfaces shouldn't outlive their GLContext. - r=jrmuizel a=ritu
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
@@ -2350,22 +2350,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();
 }
 
@@ -2599,18 +2600,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;
 }
@@ -2620,22 +2619,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
@@ -3452,18 +3452,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
@@ -307,21 +307,25 @@ SharedSurface_D3D11Interop::~SharedSurfa
     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);
     }
 }