Bug 13821850 - Return false in GLContext::Readback if MakeCurrent failed. r=jgilbert
authorNicolas Silva <nsilva@mozilla.com>
Mon, 24 Jul 2017 12:24:18 +0200
changeset 421686 b746e314d3d137ce61d49bc7d631bccada74227a
parent 421685 2a698a65f75938cf408d2fcc3d3135556e908624
child 421687 7928b9c4f4c3edc1087e220122e4a09e409d4b82
push id1517
push userjlorenzo@mozilla.com
push dateThu, 14 Sep 2017 16:50:54 +0000
treeherdermozilla-release@3b41fd564418 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjgilbert
bugs13821850
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 13821850 - Return false in GLContext::Readback if MakeCurrent failed. r=jgilbert
gfx/gl/GLContext.cpp
gfx/gl/GLContext.h
gfx/layers/ShareableCanvasLayer.cpp
gfx/layers/basic/BasicCanvasLayer.cpp
--- a/gfx/gl/GLContext.cpp
+++ b/gfx/gl/GLContext.cpp
@@ -2507,27 +2507,26 @@ SplitByChar(const nsACString& str, const
 
         start = end + 1;
     }
 
     nsDependentCSubstring substr(str, start);
     out->push_back(nsCString(substr));
 }
 
-void
+bool
 GLContext::Readback(SharedSurface* src, gfx::DataSourceSurface* dest)
 {
     MOZ_ASSERT(src && dest);
     MOZ_ASSERT(dest->GetSize() == src->mSize);
     MOZ_ASSERT(dest->GetFormat() == (src->mHasAlpha ? SurfaceFormat::B8G8R8A8
                                                     : SurfaceFormat::B8G8R8X8));
 
     if (!MakeCurrent()) {
-        gfxCriticalError() << "Failed to MakeCurrent in GLContext::Readback";
-        return;
+        return false;
     }
 
     SharedSurface* prev = GetLockedSurface();
 
     const bool needsSwap = src != prev;
     if (needsSwap) {
         if (prev)
             prev->UnlockProd();
@@ -2597,16 +2596,18 @@ GLContext::Readback(SharedSurface* src, 
         fDeleteTextures(1, &tempTex);
     }
 
     if (needsSwap) {
         src->UnlockProd();
         if (prev)
             prev->LockProd();
     }
+
+    return true;
 }
 
 // Do whatever tear-down is necessary after drawing to our offscreen FBO,
 // if it's bound.
 void
 GLContext::AfterGLDrawCall()
 {
     if (mScreen) {
--- a/gfx/gl/GLContext.h
+++ b/gfx/gl/GLContext.h
@@ -3670,17 +3670,17 @@ public:
 
 protected:
     bool mHeavyGLCallsSinceLastFlush;
 
 public:
     void FlushIfHeavyGLCallsSinceLastFlush();
     static bool ShouldSpew();
     static bool ShouldDumpExts();
-    void Readback(SharedSurface* src, gfx::DataSourceSurface* dest);
+    bool Readback(SharedSurface* src, gfx::DataSourceSurface* dest);
 
     ////
 
     void TexParams_SetClampNoMips(GLenum target = LOCAL_GL_TEXTURE_2D) {
         fTexParameteri(target, LOCAL_GL_TEXTURE_WRAP_S, LOCAL_GL_CLAMP_TO_EDGE);
         fTexParameteri(target, LOCAL_GL_TEXTURE_WRAP_T, LOCAL_GL_CLAMP_TO_EDGE);
         fTexParameteri(target, LOCAL_GL_TEXTURE_MAG_FILTER, LOCAL_GL_NEAREST);
         fTexParameteri(target, LOCAL_GL_TEXTURE_MIN_FILTER, LOCAL_GL_NEAREST);
--- a/gfx/layers/ShareableCanvasLayer.cpp
+++ b/gfx/layers/ShareableCanvasLayer.cpp
@@ -138,17 +138,20 @@ ShareableCanvasLayer::UpdateTarget(DrawT
   uint8_t* destData;
   IntSize destSize;
   int32_t destStride;
   SurfaceFormat destFormat;
   if (aDestTarget->LockBits(&destData, &destSize, &destStride, &destFormat)) {
     if (destSize == readSize && destFormat == format) {
       RefPtr<DataSourceSurface> data =
         Factory::CreateWrappingDataSourceSurface(destData, destStride, destSize, destFormat);
-      mGLContext->Readback(frontbuffer, data);
+      if (!mGLContext->Readback(frontbuffer, data)) {
+        aDestTarget->ReleaseBits(destData);
+        return false;
+      }
       if (needsPremult) {
         gfxUtils::PremultiplyDataSurface(data, data);
       }
       aDestTarget->ReleaseBits(destData);
       return true;
     }
     aDestTarget->ReleaseBits(destData);
   }
@@ -156,17 +159,19 @@ ShareableCanvasLayer::UpdateTarget(DrawT
   RefPtr<DataSourceSurface> resultSurf = GetTempSurface(readSize, format);
   // There will already be a warning from inside of GetTempSurface, but
   // it doesn't hurt to complain:
   if (NS_WARN_IF(!resultSurf)) {
     return false;
   }
 
   // Readback handles Flush/MarkDirty.
-  mGLContext->Readback(frontbuffer, resultSurf);
+  if (!mGLContext->Readback(frontbuffer, resultSurf)) {
+    return false;
+  }
   if (needsPremult) {
     gfxUtils::PremultiplyDataSurface(resultSurf, resultSurf);
   }
 
   aDestTarget->CopySurface(resultSurf,
                            IntRect(0, 0, readSize.width, readSize.height),
                            IntPoint(0, 0));
 
--- a/gfx/layers/basic/BasicCanvasLayer.cpp
+++ b/gfx/layers/basic/BasicCanvasLayer.cpp
@@ -63,17 +63,20 @@ BasicCanvasLayer::UpdateSurface()
   RefPtr<DataSourceSurface> resultSurf = GetTempSurface(readSize, format);
   // There will already be a warning from inside of GetTempSurface, but
   // it doesn't hurt to complain:
   if (NS_WARN_IF(!resultSurf)) {
     return nullptr;
   }
 
   // Readback handles Flush/MarkDirty.
-  mGLContext->Readback(frontbuffer, resultSurf);
+  if (!mGLContext->Readback(frontbuffer, resultSurf)) {
+    NS_WARNING("Failed to read back canvas surface.");
+    return nullptr;
+  }
   if (needsPremult) {
     gfxUtils::PremultiplyDataSurface(resultSurf, resultSurf);
   }
   MOZ_ASSERT(resultSurf);
 
   return resultSurf.forget();
 }