Bug 1200021 - Part 3: Add DrawTarget::IsValid and don't let Cairo version snapshot invalid surface. r=bas

gfx/2d/2D.h

@@ -694,6 +694,7 @@ public:
   DrawTarget() : mTransformDirty(false), mPermitSubpixelAA(false) {}
   virtual ~DrawTarget() {}
 
+  virtual bool IsValid() const { return true; };
   virtual DrawTargetType GetType() const = 0;
 
   virtual BackendType GetBackendType() const = 0;

gfx/2d/DrawTargetCairo.cpp

@@ -222,6 +222,7 @@ CopyToImageSurface(unsigned char *aData,
   // investigation hasn't been done to determine the underlying cause.  We
   // will just handle the failure to allocate the surface to avoid a crash.
   if (cairo_surface_status(surf)) {
+    gfxWarning() << "Invalid surface DTC " << cairo_surface_status(surf);
     return nullptr;
   }
 
@@ -600,10 +601,17 @@ DrawTargetCairo::~DrawTargetCairo()
   cairo_destroy(mContext);
   if (mSurface) {
     cairo_surface_destroy(mSurface);
+    mSurface = nullptr;
   }
   MOZ_ASSERT(!mLockedBits);
 }
 
+bool
+DrawTargetCairo::IsValid() const
+{
+  return mSurface && !cairo_surface_status(mSurface);
+}
+
 DrawTargetType
 DrawTargetCairo::GetType() const
 {
@@ -681,6 +689,10 @@ GfxFormatForCairoSurface(cairo_surface_t* surface)
 already_AddRefed<SourceSurface>
 DrawTargetCairo::Snapshot()
 {
+  if (!IsValid()) {
+    gfxCriticalNote << "DrawTargetCairo::Snapshot with bad surface " << cairo_surface_status(mSurface);
+    return nullptr;
+  }
   if (mSnapshot) {
     RefPtr<SourceSurface> snapshot(mSnapshot);
     return snapshot.forget();
@@ -780,6 +792,11 @@ DrawTargetCairo::DrawSurface(SourceSurface *aSurface,
     return;
   }
 
+  if (!IsValid()) {
+    gfxCriticalNote << "DrawSurface with bad surface " << cairo_surface_status(mSurface);
+    return;
+  }
+
   AutoPrepareForDrawing prep(this, mContext);
   AutoClearDeviceOffset clear(aSurface);
 
@@ -1031,7 +1048,7 @@ DrawTargetCairo::CopySurfaceInternal(cairo_surface_t* aSurface,
                                      const IntPoint &aDest)
 {
   if (cairo_surface_status(aSurface)) {
-    gfxWarning() << "Invalid surface";
+    gfxWarning() << "Invalid surface" << cairo_surface_status(aSurface);
     return;
   }
 
@@ -1234,6 +1251,11 @@ DrawTargetCairo::FillGlyphs(ScaledFont *aFont,
     return;
   }
 
+  if (!IsValid()) {
+    gfxDebug() << "FillGlyphs bad surface " << cairo_surface_status(mSurface);
+    return;
+  }
+
   AutoPrepareForDrawing prep(this, mContext);
   AutoClearDeviceOffset clear(aPattern);
 
@@ -1266,6 +1288,10 @@ DrawTargetCairo::FillGlyphs(ScaledFont *aFont,
   }
 
   cairo_show_glyphs(mContext, &glyphs[0], aBuffer.mNumGlyphs);
+
+  if (mSurface && cairo_surface_status(mSurface)) {
+    gfxDebug() << "Ending FillGlyphs with a bad surface " << cairo_surface_status(mSurface);
+  }
 }
 
 void
@@ -1627,7 +1653,7 @@ bool
 DrawTargetCairo::InitAlreadyReferenced(cairo_surface_t* aSurface, const IntSize& aSize, SurfaceFormat* aFormat)
 {
   if (cairo_surface_status(aSurface)) {
-    gfxCriticalError(CriticalLog::DefaultOptions(Factory::ReasonableSurfaceSize(aSize)))
+    gfxCriticalNote
       << "Attempt to create DrawTarget for invalid surface. "
       << aSize << " Cairo Status: " << cairo_surface_status(aSurface);
     cairo_surface_destroy(aSurface);

gfx/2d/DrawTargetCairo.h

@@ -59,6 +59,7 @@ public:
   DrawTargetCairo();
   virtual ~DrawTargetCairo();
 
+  virtual bool IsValid() const override;
   virtual DrawTargetType GetType() const override;
   virtual BackendType GetBackendType() const override { return BackendType::CAIRO; }
   virtual already_AddRefed<SourceSurface> Snapshot() override;

gfx/layers/TextureDIB.cpp

@@ -366,6 +366,11 @@ DIBTextureHost::UpdatedInternal(const nsIntRegion* aRegion)
     mTextureSource = mCompositor->CreateDataTextureSource(mFlags);
   }
 
+  if (mSurface->CairoStatus()) {
+      gfxWarning() << "Bad Cairo surface internal update " << mSurface->CairoStatus();
+      mTextureSource = nullptr;
+      return;
+  }
   RefPtr<gfxImageSurface> imgSurf = mSurface->GetAsImageSurface();
 
   RefPtr<DataSourceSurface> surf = Factory::CreateWrappingDataSourceSurface(imgSurf->Data(), imgSurf->Stride(), mSize, mFormat);

gfx/layers/basic/BasicPaintedLayer.cpp

@@ -162,7 +162,8 @@ BasicPaintedLayer::Validate(LayerManager::DrawPaintedLayerCallback aCallback,
     mContentClient->BeginPaintBuffer(this, flags);
   mValidRegion.Sub(mValidRegion, state.mRegionToInvalidate);
 
-  if (DrawTarget* target = mContentClient->BorrowDrawTargetForPainting(state)) {
+  DrawTarget* target = mContentClient->BorrowDrawTargetForPainting(state);
+  if (target && target->IsValid()) {
     // The area that became invalid and is visible needs to be repainted
     // (this could be the whole visible area if our buffer switched
     // from RGB to RGBA, because we might need to repaint with
@@ -183,16 +184,22 @@ BasicPaintedLayer::Validate(LayerManager::DrawPaintedLayerCallback aCallback,
     Mutated();
     ctx = nullptr;
     mContentClient->ReturnDrawTargetToBuffer(target);
+    target = nullptr;
 
     RenderTraceInvalidateEnd(this, "FFFF00");
   } else {
+    if (target) {
+      mContentClient->ReturnDrawTargetToBuffer(target);
+      target = nullptr;
+    }
+
     // It's possible that state.mRegionToInvalidate is nonempty here,
     // if we are shrinking the valid region to nothing. So use mRegionToDraw
     // instead.
     NS_WARN_IF_FALSE(state.mRegionToDraw.IsEmpty(),
                      "No context when we have something to draw, resource exhaustion?");
   }
-  
+
   for (uint32_t i = 0; i < readbackUpdates.Length(); ++i) {
     ReadbackProcessor::Update& update = readbackUpdates[i];
     nsIntPoint offset = update.mLayer->GetBackgroundLayerOffset();

gfx/layers/client/ClientPaintedLayer.cpp

@@ -80,6 +80,13 @@ ClientPaintedLayer::PaintThebes()
   bool didUpdate = false;
   RotatedContentBuffer::DrawIterator iter;
   while (DrawTarget* target = mContentClient->BorrowDrawTargetForPainting(state, &iter)) {
+    if (!target || !target->IsValid()) {
+      if (target) {
+        mContentClient->ReturnDrawTargetToBuffer(target);
+      }
+      continue;
+    }
+    
     SetAntialiasingFlags(this, target);
 
     RefPtr<gfxContext> ctx = gfxContext::ContextForDrawTarget(target);

gfx/layers/client/ContentClient.cpp

@@ -586,15 +586,21 @@ ContentClientDoubleBuffered::FinalizeFrame(const nsIntRegion& aRegionToDraw)
   // Restrict the DrawTargets and frontBuffer to a scope to make
   // sure there is no more external references to the DrawTargets
   // when we Unlock the TextureClients.
-  RefPtr<SourceSurface> surf = mFrontClient->BorrowDrawTarget()->Snapshot();
-  RefPtr<SourceSurface> surfOnWhite = mFrontClientOnWhite
-    ? mFrontClientOnWhite->BorrowDrawTarget()->Snapshot()
-    : nullptr;
-  SourceRotatedBuffer frontBuffer(surf,
-                                  surfOnWhite,
-                                  mFrontBufferRect,
-                                  mFrontBufferRotation);
-  UpdateDestinationFrom(frontBuffer, updateRegion);
+  gfx::DrawTarget* dt = mFrontClient->BorrowDrawTarget();
+  gfx::DrawTarget* dtw = mFrontClientOnWhite ? mFrontClientOnWhite->BorrowDrawTarget() : nullptr;
+  if (dt && dt->IsValid()) {
+    RefPtr<SourceSurface> surf = dt->Snapshot();
+    RefPtr<SourceSurface> surfOnWhite = dtw ? dtw->Snapshot() : nullptr;
+    SourceRotatedBuffer frontBuffer(surf,
+                                    surfOnWhite,
+                                    mFrontBufferRect,
+                                    mFrontBufferRotation);
+    UpdateDestinationFrom(frontBuffer, updateRegion);
+  } else {
+    // We know this can happen, but we want to track it somewhat, in case it leads
+    // to other problems.
+    gfxCriticalNote << "Invalid draw target(s) " << hexa(dt) << " and " << hexa(dtw);
+  }
 }
 
 void

gfx/thebes/gfxContext.cpp

@@ -85,6 +85,11 @@ gfxContext::gfxContext(DrawTarget *aTarget, const Point& aDeviceOffset)
 /* static */ already_AddRefed<gfxContext>
 gfxContext::ContextForDrawTarget(DrawTarget* aTarget)
 {
+  if (!aTarget || !aTarget->IsValid()) {
+    gfxWarning() << "Invalid target in gfxContext::ContextForDrawTarget";
+    return nullptr;
+  }
+
   Matrix transform = aTarget->GetTransform();
   RefPtr<gfxContext> result = new gfxContext(aTarget);
   result->SetMatrix(ThebesMatrix(transform));

gfx/thebes/gfxImageSurface.cpp

@@ -29,6 +29,11 @@ gfxImageSurface::gfxImageSurface()
 void
 gfxImageSurface::InitFromSurface(cairo_surface_t *csurf)
 {
+    if (!csurf || cairo_surface_status(csurf)) {
+        MakeInvalid();
+        return;
+    }
+
     mSize.width = cairo_image_surface_get_width(csurf);
     mSize.height = cairo_image_surface_get_height(csurf);
     mData = cairo_image_surface_get_data(csurf);

gfx/thebes/gfxImageSurface.h

@@ -77,8 +77,18 @@ public:
     gfxImageFormat Format() const { return mFormat; }
 
     virtual const mozilla::gfx::IntSize GetSize() const override { return mSize; }
-    int32_t Width() const { return mSize.width; }
-    int32_t Height() const { return mSize.height; }
+    int32_t Width() const {
+        if (mSize.width < 0) {
+            return 0;
+        }
+        return mSize.width;
+    }
+    int32_t Height() const {
+        if (mSize.height < 0) {
+            return 0;
+        }
+        return mSize.height;
+    }
 
     /**
      * Distance in bytes between the start of a line and the start of the
@@ -93,7 +103,12 @@ public:
     /**
      * Returns the total size of the image data.
      */
-    int32_t GetDataSize() const { return mStride*mSize.height; }
+    int32_t GetDataSize() const {
+        if (mStride < 0 || mSize.height < 0) {
+            return 0;
+        }
+        return mStride*mSize.height;
+    }
 
     /* Fast copy from another image surface; returns TRUE if successful, FALSE otherwise */
     bool CopyFrom (gfxImageSurface *other);
@@ -144,8 +159,12 @@ protected:
     void AllocateAndInit(long aStride, int32_t aMinimalAllocation, bool aClear);
     void InitFromSurface(cairo_surface_t *csurf);
 
-    long ComputeStride() const { return ComputeStride(mSize, mFormat); }
-
+    long ComputeStride() const { 
+        if (mSize.height < 0 || mSize.width < 0) {
+            return 0;
+        }
+        return ComputeStride(mSize, mFormat);
+    }
 
     void MakeInvalid();