Bug 1200021 - Part 3: Add DrawTarget::IsValid and don't let Cairo version snapshot invalid surface. r=bas
authorMilan Sreckovic <msreckovic@mozilla.com>
Fri, 04 Dec 2015 13:43:00 -0500
changeset 309941 bb05a1110e4c7b9fa6eaca8aeaad1dfff18acf41
parent 309940 fea8e8cb105e34fefbea7521afe53e69f32c6aa8
child 309942 d4053d8be57b0fa4c57c025e69914f825e582bb9
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbas
bugs1200021
milestone45.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 1200021 - Part 3: Add DrawTarget::IsValid and don't let Cairo version snapshot invalid surface. r=bas
gfx/2d/2D.h
gfx/2d/DrawTargetCairo.cpp
gfx/2d/DrawTargetCairo.h
gfx/layers/TextureDIB.cpp
gfx/layers/basic/BasicPaintedLayer.cpp
gfx/layers/client/ClientPaintedLayer.cpp
gfx/layers/client/ContentClient.cpp
gfx/thebes/gfxContext.cpp
gfx/thebes/gfxImageSurface.cpp
gfx/thebes/gfxImageSurface.h
--- a/gfx/2d/2D.h
+++ b/gfx/2d/2D.h
@@ -689,16 +689,17 @@ class DrawTargetCapture;
  */
 class DrawTarget : public RefCounted<DrawTarget>
 {
 public:
   MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(DrawTarget)
   DrawTarget() : mTransformDirty(false), mPermitSubpixelAA(false) {}
   virtual ~DrawTarget() {}
 
+  virtual bool IsValid() const { return true; };
   virtual DrawTargetType GetType() const = 0;
 
   virtual BackendType GetBackendType() const = 0;
   /**
    * Returns a SourceSurface which is a snapshot of the current contents of the DrawTarget.
    * Multiple calls to Snapshot() without any drawing operations in between will
    * normally return the same SourceSurface object.
    */
--- a/gfx/2d/DrawTargetCairo.cpp
+++ b/gfx/2d/DrawTargetCairo.cpp
@@ -217,16 +217,17 @@ CopyToImageSurface(unsigned char *aData,
   cairo_surface_t* surf = cairo_image_surface_create(GfxFormatToCairoFormat(aFormat),
                                                      aRect.width,
                                                      aRect.height);
   // In certain scenarios, requesting larger than 8k image fails.  Bug 803568
   // covers the details of how to run into it, but the full detailed
   // 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;
   }
 
   unsigned char* surfData = cairo_image_surface_get_data(surf);
   int surfStride = cairo_image_surface_get_stride(surf);
   int32_t pixelWidth = BytesPerPixel(aFormat);
 
   unsigned char* source = aData +
@@ -595,20 +596,27 @@ DrawTargetCairo::DrawTargetCairo()
 {
 }
 
 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
 {
   if (mContext) {
     cairo_surface_type_t type = cairo_surface_get_type(mSurface);
     if (type == CAIRO_SURFACE_TYPE_TEE) {
       type = cairo_surface_get_type(cairo_tee_surface_index(mSurface, 0));
       MOZ_ASSERT(type != CAIRO_SURFACE_TYPE_TEE, "C'mon!");
@@ -676,16 +684,20 @@ GfxFormatForCairoSurface(cairo_surface_t
   }
 #endif
   return CairoContentToGfxFormat(cairo_surface_get_content(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();
   }
 
   IntSize size = GetSize();
 
   mSnapshot = new SourceSurfaceCairo(mSurface,
@@ -775,16 +787,21 @@ DrawTargetCairo::DrawSurface(SourceSurfa
                              const Rect &aSource,
                              const DrawSurfaceOptions &aSurfOptions,
                              const DrawOptions &aOptions)
 {
   if (mTransformSingular) {
     return;
   }
 
+  if (!IsValid()) {
+    gfxCriticalNote << "DrawSurface with bad surface " << cairo_surface_status(mSurface);
+    return;
+  }
+
   AutoPrepareForDrawing prep(this, mContext);
   AutoClearDeviceOffset clear(aSurface);
 
   float sx = aSource.Width() / aDest.Width();
   float sy = aSource.Height() / aDest.Height();
 
   cairo_matrix_t src_mat;
   cairo_matrix_init_translate(&src_mat, aSource.X(), aSource.Y());
@@ -1026,17 +1043,17 @@ DrawTargetCairo::FillRect(const Rect &aR
 }
 
 void
 DrawTargetCairo::CopySurfaceInternal(cairo_surface_t* aSurface,
                                      const IntRect &aSource,
                                      const IntPoint &aDest)
 {
   if (cairo_surface_status(aSurface)) {
-    gfxWarning() << "Invalid surface";
+    gfxWarning() << "Invalid surface" << cairo_surface_status(aSurface);
     return;
   }
 
   cairo_identity_matrix(mContext);
 
   cairo_set_source_surface(mContext, aSurface, aDest.x - aSource.x, aDest.y - aSource.y);
   cairo_set_operator(mContext, CAIRO_OPERATOR_SOURCE);
   cairo_set_antialias(mContext, CAIRO_ANTIALIAS_NONE);
@@ -1229,16 +1246,21 @@ DrawTargetCairo::FillGlyphs(ScaledFont *
                             const Pattern &aPattern,
                             const DrawOptions &aOptions,
                             const GlyphRenderingOptions*)
 {
   if (mTransformSingular) {
     return;
   }
 
+  if (!IsValid()) {
+    gfxDebug() << "FillGlyphs bad surface " << cairo_surface_status(mSurface);
+    return;
+  }
+
   AutoPrepareForDrawing prep(this, mContext);
   AutoClearDeviceOffset clear(aPattern);
 
   ScaledFontBase* scaledFont = static_cast<ScaledFontBase*>(aFont);
   cairo_set_scaled_font(mContext, scaledFont->GetCairoScaledFont());
 
   cairo_pattern_t* pat = GfxPatternToCairoPattern(aPattern, aOptions.mAlpha, GetTransform());
   if (!pat)
@@ -1261,16 +1283,20 @@ DrawTargetCairo::FillGlyphs(ScaledFont *
   }
   for (uint32_t i = 0; i < aBuffer.mNumGlyphs; ++i) {
     glyphs[i].index = aBuffer.mGlyphs[i].mIndex;
     glyphs[i].x = aBuffer.mGlyphs[i].mPosition.x;
     glyphs[i].y = aBuffer.mGlyphs[i].mPosition.y;
   }
 
   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
 DrawTargetCairo::Mask(const Pattern &aSource,
                       const Pattern &aMask,
                       const DrawOptions &aOptions /* = DrawOptions() */)
 {
   if (mTransformSingular) {
@@ -1622,17 +1648,17 @@ DrawTargetCairo::CreateSimilarDrawTarget
 
   return nullptr;
 }
 
 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);
     return false;
   }
 
   mContext = cairo_create(aSurface);
   mSurface = aSurface;
--- a/gfx/2d/DrawTargetCairo.h
+++ b/gfx/2d/DrawTargetCairo.h
@@ -54,16 +54,17 @@ class DrawTargetCairo final : public Dra
 public:
   MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(DrawTargetCairo, override)
   friend class BorrowedCairoContext;
   friend class BorrowedXlibDrawable;
 
   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;
   virtual IntSize GetSize() override;
 
   virtual void SetPermitSubpixelAA(bool aPermitSubpixelAA) override;
 
   virtual bool LockBits(uint8_t** aData, IntSize* aSize,
--- a/gfx/layers/TextureDIB.cpp
+++ b/gfx/layers/TextureDIB.cpp
@@ -361,16 +361,21 @@ DIBTextureHost::UpdatedInternal(const ns
     // attached to a layer.
     return;
   }
 
   if (!mTextureSource) {
     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);
 
   if (!mTextureSource->Update(surf, const_cast<nsIntRegion*>(aRegion))) {
     mTextureSource = nullptr;
   }
 }
--- a/gfx/layers/basic/BasicPaintedLayer.cpp
+++ b/gfx/layers/basic/BasicPaintedLayer.cpp
@@ -157,17 +157,18 @@ BasicPaintedLayer::Validate(LayerManager
 #endif
   if (mDrawAtomically) {
     flags |= RotatedContentBuffer::PAINT_NO_ROTATION;
   }
   PaintState state =
     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
     // subpixel AA)
     state.mRegionToInvalidate.And(state.mRegionToInvalidate,
                                   GetEffectiveVisibleRegion().ToUnknownRegion());
     SetAntialiasingFlags(this, target);
 
@@ -178,26 +179,32 @@ BasicPaintedLayer::Validate(LayerManager
                 state.mRegionToDraw, state.mRegionToDraw, state.mRegionToInvalidate,
                 state.mDidSelfCopy,
                 state.mClip,
                 aCallback, aCallbackData);
     MOZ_LAYERS_LOG_IF_SHADOWABLE(this, ("Layer::Mutated(%p) PaintThebes", this));
     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();
     RefPtr<gfxContext> ctx =
       update.mLayer->GetSink()->BeginUpdate(update.mUpdateRect + offset,
                                             update.mSequenceCounter);
     if (ctx) {
       NS_ASSERTION(GetEffectiveOpacity() == 1.0, "Should only read back opaque layers");
--- a/gfx/layers/client/ClientPaintedLayer.cpp
+++ b/gfx/layers/client/ClientPaintedLayer.cpp
@@ -75,16 +75,23 @@ ClientPaintedLayer::PaintThebes()
   // from RGB to RGBA, because we might need to repaint with
   // subpixel AA)
   state.mRegionToInvalidate.And(state.mRegionToInvalidate,
                                 GetEffectiveVisibleRegion().ToUnknownRegion());
 
   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);
 
     ClientManager()->GetPaintedLayerCallback()(this,
                                               ctx,
                                               iter.mDrawRegion,
                                               iter.mDrawRegion,
--- a/gfx/layers/client/ContentClient.cpp
+++ b/gfx/layers/client/ContentClient.cpp
@@ -581,25 +581,31 @@ ContentClientDoubleBuffered::FinalizeFra
     if (!frontOnWhiteLock->Succeeded()) {
       return;
     }
   }
 
   // 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
 ContentClientDoubleBuffered::EnsureBackBufferIfFrontBuffer()
 {
   if (!mTextureClient && mFrontClient) {
     CreateBackBuffer(mFrontBufferRect);
 
--- a/gfx/thebes/gfxContext.cpp
+++ b/gfx/thebes/gfxContext.cpp
@@ -80,16 +80,21 @@ gfxContext::gfxContext(DrawTarget *aTarg
   CurrentState().drawTarget = mDT;
   CurrentState().deviceOffset = aDeviceOffset;
   mDT->SetTransform(GetDTTransform());
 }
 
 /* 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));
   return result.forget();
 }
 
 gfxContext::~gfxContext()
 {
--- a/gfx/thebes/gfxImageSurface.cpp
+++ b/gfx/thebes/gfxImageSurface.cpp
@@ -24,16 +24,21 @@ gfxImageSurface::gfxImageSurface()
     mFormat(gfxImageFormat::Unknown),
     mStride(0)
 {
 }
 
 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);
     mFormat = gfxCairoFormatToImageFormat(cairo_image_surface_get_format(csurf));
     mOwnsData = false;
     mStride = cairo_image_surface_get_stride(csurf);
 
     Init(csurf, true);
--- a/gfx/thebes/gfxImageSurface.h
+++ b/gfx/thebes/gfxImageSurface.h
@@ -72,33 +72,48 @@ public:
     explicit gfxImageSurface(cairo_surface_t *csurf);
 
     virtual ~gfxImageSurface();
 
     // ImageSurface methods
     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
      * next line.
      */
     int32_t Stride() const { return mStride; }
     /**
      * Returns a pointer for the image data. Users of this function can
      * write to it, but must not attempt to free the buffer.
      */
     unsigned char* Data() const { return mData; } // delete this data under us and die.
     /**
      * 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);
 
     /**
      * Fast copy from a source surface; returns TRUE if successful, FALSE otherwise
      * Assumes that the format of this surface is compatable with aSurface
      */
@@ -139,18 +154,22 @@ protected:
     /**
      * See the parameters to the matching constructor.  This should only
      * be called once, in the constructor, which has already set mSize
      * and mFormat.
      */
     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();
 
     mozilla::gfx::IntSize mSize;
     bool mOwnsData;
     unsigned char *mData;
     gfxImageFormat mFormat;
     long mStride;