Bug 780792 - Fix save/restore leak in basic layer when needsGroup, is2D, and clipIsEmpty are all true. r=roc

gfx/layers/basic/BasicLayerManager.cpp

@@ -100,6 +100,111 @@ ToInsideIntRect(const gfxRect& aRect)
   return nsIntRect(r.X(), r.Y(), r.Width(), r.Height());
 }
 
+// A context helper for BasicLayerManager::PaintLayer() that holds all the
+// painting context together in a data structure so it can be easily passed
+// around. It also uses ensures that the Transform and Opaque rect are restored
+// to their former state on destruction.
+
+class PaintContext {
+public:
+  PaintContext(gfxContext* aTarget, Layer* aLayer,
+               LayerManager::DrawThebesLayerCallback aCallback,
+               void* aCallbackData, ReadbackProcessor* aReadback)
+   : mTarget(aTarget)
+   , mTargetMatrixSR(aTarget)
+   , mLayer(aLayer)
+   , mCallback(aCallback)
+   , mCallbackData(aCallbackData)
+   , mReadback(aReadback)
+   , mPushedOpaqueRect(false)
+  {}
+
+  ~PaintContext()
+  {
+    // Matrix is restored by mTargetMatrixSR
+    if (mPushedOpaqueRect)
+    {
+      ClearOpaqueRect();
+    }
+  }
+
+  // Applies the effective 2D transform and returns true if it is a 2D
+  // transform. If it's a 3D transform then it applies an identity and returns
+  // false.
+  bool Apply2DTransform()
+  {
+    const gfx3DMatrix& effectiveTransform = mLayer->GetEffectiveTransform();
+
+    // Will return an identity matrix for 3d transforms.
+    bool is2D = effectiveTransform.CanDraw2D(&mTransform);
+    mTarget->SetMatrix(mTransform);
+
+    return is2D;
+  }
+
+  // Set the opaque rect to match the bounds of the visible region.
+  void AnnotateOpaqueRect()
+  {
+    const nsIntRegion& visibleRegion = mLayer->GetEffectiveVisibleRegion();
+    const nsIntRect& bounds = visibleRegion.GetBounds();
+    nsRefPtr<gfxASurface> currentSurface = mTarget->CurrentSurface();
+
+    if (mTarget->IsCairo()) {
+      const gfxRect& targetOpaqueRect = currentSurface->GetOpaqueRect();
+
+      // Try to annotate currentSurface with a region of pixels that have been
+      // (or will be) painted opaque, if no such region is currently set.
+      if (targetOpaqueRect.IsEmpty() && visibleRegion.GetNumRects() == 1 &&
+          (mLayer->GetContentFlags() & Layer::CONTENT_OPAQUE) &&
+          !mTransform.HasNonAxisAlignedTransform()) {
+        currentSurface->SetOpaqueRect(
+            mTarget->UserToDevice(gfxRect(bounds.x, bounds.y, bounds.width, bounds.height)));
+        mPushedOpaqueRect = true;
+      }
+    } else {
+      DrawTarget *dt = mTarget->GetDrawTarget();
+      const IntRect& targetOpaqueRect = dt->GetOpaqueRect();
+
+      // Try to annotate currentSurface with a region of pixels that have been
+      // (or will be) painted opaque, if no such region is currently set.
+      if (targetOpaqueRect.IsEmpty() && visibleRegion.GetNumRects() == 1 &&
+          (mLayer->GetContentFlags() & Layer::CONTENT_OPAQUE) &&
+          !mTransform.HasNonAxisAlignedTransform()) {
+
+        gfx::Rect opaqueRect = dt->GetTransform().TransformBounds(
+          gfx::Rect(bounds.x, bounds.y, bounds.width, bounds.height));
+        opaqueRect.RoundIn();
+        IntRect intOpaqueRect;
+        if (opaqueRect.ToIntRect(&intOpaqueRect)) {
+          mTarget->GetDrawTarget()->SetOpaqueRect(intOpaqueRect);
+          mPushedOpaqueRect = true;
+        }
+      }
+    }
+  }
+
+  // Clear the Opaque rect. Although this doesn't really restore it to it's
+  // previous state it will happen on the exit path of the PaintLayer() so when
+  // painting is complete the opaque rect qill be clear.
+  void ClearOpaqueRect() {
+    if (mTarget->IsCairo()) {
+      nsRefPtr<gfxASurface> currentSurface = mTarget->CurrentSurface();
+      currentSurface->SetOpaqueRect(gfxRect());
+    } else {
+      mTarget->GetDrawTarget()->SetOpaqueRect(IntRect());
+    }
+  }
+
+  gfxContext* mTarget;
+  gfxContextMatrixAutoSaveRestore mTargetMatrixSR;
+  Layer* mLayer;
+  LayerManager::DrawThebesLayerCallback mCallback;
+  void* mCallbackData;
+  ReadbackProcessor* mReadback;
+  gfxMatrix mTransform;
+  bool mPushedOpaqueRect;
+};
+
 BasicLayerManager::BasicLayerManager(nsIWidget* aWidget) :
 #ifdef DEBUG
   mPhase(PHASE_NONE),
@@ -690,7 +795,71 @@ Transform3D(gfxASurface* aSource, gfxContext* aDest,
   return destImage.forget(); 
 }
 
+void
+BasicLayerManager::PaintSelfOrChildren(PaintContext& aPaintContext,
+                                       gfxContext* aGroupTarget)
+{
+  BasicImplData* data = ToData(aPaintContext.mLayer);
 
+  if (aPaintContext.mLayer->GetFirstChild() &&
+      aPaintContext.mLayer->GetMaskLayer() &&
+      HasShadowManager()) {
+    // 'paint' the mask so that it gets sent to the shadow layer tree
+    static_cast<BasicImplData*>(aPaintContext.mLayer->GetMaskLayer()->ImplData())
+      ->Paint(nullptr, nullptr);
+  }
+
+  /* Only paint ourself, or our children - This optimization relies on this! */
+  Layer* child = aPaintContext.mLayer->GetFirstChild();
+  if (!child) {
+    if (aPaintContext.mLayer->AsThebesLayer()) {
+      data->PaintThebes(aGroupTarget, aPaintContext.mLayer->GetMaskLayer(),
+          aPaintContext.mCallback, aPaintContext.mCallbackData,
+          aPaintContext.mReadback);
+    } else {
+      data->Paint(aGroupTarget, aPaintContext.mLayer->GetMaskLayer());
+    }
+  } else {
+    ReadbackProcessor readback;
+    ContainerLayer* container =
+        static_cast<ContainerLayer*>(aPaintContext.mLayer);
+    if (IsRetained()) {
+      readback.BuildUpdates(container);
+    }
+    nsAutoTArray<Layer*, 12> children;
+    container->SortChildrenBy3DZOrder(children);
+    for (uint32_t i = 0; i < children.Length(); i++) {
+      PaintLayer(aGroupTarget, children.ElementAt(i), aPaintContext.mCallback,
+          aPaintContext.mCallbackData, &readback);
+      if (mTransactionIncomplete)
+        break;
+    }
+  }
+}
+
+void
+BasicLayerManager::FlushGroup(PaintContext& aPaintContext, bool aNeedsClipToVisibleRegion)
+{
+  // If we're doing our own double-buffering, we need to avoid drawing
+  // the results of an incomplete transaction to the destination surface ---
+  // that could cause flicker. Double-buffering is implemented using a
+  // temporary surface for one or more container layers, so we need to stop
+  // those temporary surfaces from being composited to aTarget.
+  // ApplyDoubleBuffering guarantees that this container layer can't
+  // intersect any other leaf layers, so if the transaction is not yet marked
+  // incomplete, the contents of this container layer are the final contents
+  // for the window.
+  if (!mTransactionIncomplete) {
+    if (aNeedsClipToVisibleRegion) {
+      gfxUtils::ClipToRegion(aPaintContext.mTarget,
+                             aPaintContext.mLayer->GetEffectiveVisibleRegion());
+    }
+    BasicContainerLayer* container = static_cast<BasicContainerLayer*>(aPaintContext.mLayer);
+    AutoSetOperator setOperator(aPaintContext.mTarget, container->GetOperator());
+    PaintWithMask(aPaintContext.mTarget, aPaintContext.mLayer->GetEffectiveOpacity(),
+                  HasShadowManager() ? nullptr : aPaintContext.mLayer->GetMaskLayer());
+  }
+}
 
 void
 BasicLayerManager::PaintLayer(gfxContext* aTarget,
@@ -699,10 +868,11 @@ BasicLayerManager::PaintLayer(gfxContext* aTarget,
                               void* aCallbackData,
                               ReadbackProcessor* aReadback)
 {
+  PaintContext paintContext(aTarget, aLayer, aCallback, aCallbackData, aReadback);
+
   RenderTraceScope trace("BasicLayerManager::PaintLayer", "707070");
 
   const nsIntRect* clipRect = aLayer->GetEffectiveClipRect();
-  const gfx3DMatrix& effectiveTransform = aLayer->GetEffectiveTransform();
   // aLayer might not be a container layer, but if so we take care not to use
   // the container variable
   BasicContainerLayer* container = static_cast<BasicContainerLayer*>(aLayer);
@@ -718,12 +888,10 @@ BasicLayerManager::PaintLayer(gfxContext* aTarget,
                container->UseIntermediateSurface(),
                "ContainerLayer with mask layer should force UseIntermediateSurface");
 
-  // If needsSaveRestore is false, we should still save and restore
+  gfxContextAutoSaveRestore contextSR;
-  // the CTM
   bool needsSaveRestore = needsGroup || clipRect || needsClipToVisibleRegion;
-  gfxMatrix savedMatrix;
   if (needsSaveRestore) {
-    aTarget->Save();
+    contextSR.SetContext(aTarget);
 
     if (clipRect) {
       aTarget->NewPath();
@@ -731,17 +899,9 @@ BasicLayerManager::PaintLayer(gfxContext* aTarget,
       aTarget->Clip();
     }
   }
-  savedMatrix = aTarget->CurrentMatrix();
 
-  gfxMatrix transform;
+  bool is2D = paintContext.Apply2DTransform();
-  // Will return an identity matrix for 3d transforms, and is handled separately below.
-  bool is2D = effectiveTransform.CanDraw2D(&transform);
   NS_ABORT_IF_FALSE(is2D || needsGroup || !aLayer->GetFirstChild(), "Must PushGroup for 3d transforms!");
-  if (is2D) {
-    aTarget->SetMatrix(transform);
-  } else {
-    aTarget->SetMatrix(gfxMatrix());
-  }
 
   const nsIntRegion& visibleRegion = aLayer->GetEffectiveVisibleRegion();
   // If needsGroup is true, we'll clip to the visible region after we've popped the group
@@ -750,181 +910,68 @@ BasicLayerManager::PaintLayer(gfxContext* aTarget,
     // Don't need to clip to visible region again
     needsClipToVisibleRegion = false;
   }
-
-  bool pushedTargetOpaqueRect = false;
-  nsRefPtr<gfxASurface> currentSurface = aTarget->CurrentSurface();
-  DrawTarget *dt = aTarget->GetDrawTarget();
-  const nsIntRect& bounds = visibleRegion.GetBounds();
   
   if (is2D) {
-    if (aTarget->IsCairo()) {
+    paintContext.AnnotateOpaqueRect();
-      const gfxRect& targetOpaqueRect = currentSurface->GetOpaqueRect();
-
-      // Try to annotate currentSurface with a region of pixels that have been
-      // (or will be) painted opaque, if no such region is currently set.
-      if (targetOpaqueRect.IsEmpty() && visibleRegion.GetNumRects() == 1 &&
-          (aLayer->GetContentFlags() & Layer::CONTENT_OPAQUE) &&
-          !transform.HasNonAxisAlignedTransform()) {
-        currentSurface->SetOpaqueRect(
-            aTarget->UserToDevice(gfxRect(bounds.x, bounds.y, bounds.width, bounds.height)));
-        pushedTargetOpaqueRect = true;
-      }
-    } else {
-      const IntRect& targetOpaqueRect = dt->GetOpaqueRect();
-
-      // Try to annotate currentSurface with a region of pixels that have been
-      // (or will be) painted opaque, if no such region is currently set.
-      if (targetOpaqueRect.IsEmpty() && visibleRegion.GetNumRects() == 1 &&
-          (aLayer->GetContentFlags() & Layer::CONTENT_OPAQUE) &&
-          !transform.HasNonAxisAlignedTransform()) {
-
-        gfx::Rect opaqueRect = dt->GetTransform().TransformBounds(
-          gfx::Rect(bounds.x, bounds.y, bounds.width, bounds.height));
-        opaqueRect.RoundIn();
-        IntRect intOpaqueRect;
-        if (opaqueRect.ToIntRect(&intOpaqueRect)) {
-          aTarget->GetDrawTarget()->SetOpaqueRect(intOpaqueRect);
-          pushedTargetOpaqueRect = true;
-        }
-      }
-    }
   }
 
-  nsRefPtr<gfxContext> groupTarget;
-  nsRefPtr<gfxASurface> untransformedSurface;
   bool clipIsEmpty = !aTarget || aTarget->GetClipExtents().IsEmpty();
-  if (!is2D && !clipIsEmpty) {
+  if (clipIsEmpty) {
-    untransformedSurface = 
+    PaintSelfOrChildren(paintContext, aTarget);
-      gfxPlatform::GetPlatform()->CreateOffscreenSurface(gfxIntSize(bounds.width, bounds.height), 
+    return;
+  }
+
+  if (is2D) {
+    if (needsGroup) {
+      nsRefPtr<gfxContext> groupTarget = PushGroupForLayer(aTarget, aLayer, aLayer->GetEffectiveVisibleRegion(),
+                                      &needsClipToVisibleRegion);
+      PaintSelfOrChildren(paintContext, groupTarget);
+      PopGroupToSourceWithCachedSurface(aTarget, groupTarget);
+      FlushGroup(paintContext, needsClipToVisibleRegion);
+    } else {
+      PaintSelfOrChildren(paintContext, aTarget);
+    }
+  } else {
+    const nsIntRect& bounds = visibleRegion.GetBounds();
+    nsRefPtr<gfxASurface> untransformedSurface =
+      gfxPlatform::GetPlatform()->CreateOffscreenSurface(gfxIntSize(bounds.width, bounds.height),
                                                          gfxASurface::CONTENT_COLOR_ALPHA);
     if (!untransformedSurface) {
-      if (pushedTargetOpaqueRect) {
-        if (aTarget->IsCairo()) {
-          currentSurface->SetOpaqueRect(gfxRect(0, 0, 0, 0));
-        } else {
-          dt->SetOpaqueRect(IntRect());
-        }
-      }
-      NS_ASSERTION(needsSaveRestore, "Should always need to restore with 3d transforms!");
-      aTarget->Restore();
       return;
     }
     untransformedSurface->SetDeviceOffset(gfxPoint(-bounds.x, -bounds.y));
-    groupTarget = new gfxContext(untransformedSurface);
+    nsRefPtr<gfxContext> groupTarget = new gfxContext(untransformedSurface);
-  } else if (needsGroup && !clipIsEmpty) {
-    groupTarget = PushGroupForLayer(aTarget, aLayer, aLayer->GetEffectiveVisibleRegion(),
-                                    &needsClipToVisibleRegion);
-  } else {
-    groupTarget = aTarget;
-  }
 
-  if (aLayer->GetFirstChild() &&
+    PaintSelfOrChildren(paintContext, groupTarget);
-      aLayer->GetMaskLayer() &&
-      HasShadowManager()) {
-    // 'paint' the mask so that it gets sent to the shadow layer tree
-    static_cast<BasicImplData*>(aLayer->GetMaskLayer()->ImplData())
-      ->Paint(nullptr, nullptr);
-  }
 
-  /* Only paint ourself, or our children - This optimization relies on this! */
+    // Temporary fast fix for bug 725886
-  Layer* child = aLayer->GetFirstChild();
+    // Revert these changes when 725886 is ready
-  if (!child) {
+    NS_ABORT_IF_FALSE(untransformedSurface,
-#ifdef MOZ_LAYERS_HAVE_LOG
+                      "We should always allocate an untransformed surface with 3d transforms!");
-    MOZ_LAYERS_LOG(("%s (0x%p) is covered: %i\n", __FUNCTION__,
+    gfxPoint offset;
-                   (void*)aLayer, data->IsHidden()));
+    bool dontBlit = needsClipToVisibleRegion || mTransactionIncomplete ||
-#endif
+                      aLayer->GetEffectiveOpacity() != 1.0f;
-    if (aLayer->AsThebesLayer()) {
-      data->PaintThebes(groupTarget,
-                        aLayer->GetMaskLayer(),
-                        aCallback, aCallbackData,
-                        aReadback);
-    } else {
-      data->Paint(groupTarget, aLayer->GetMaskLayer());
-    }
-  } else {
-    ReadbackProcessor readback;
-    ContainerLayer* container = static_cast<ContainerLayer*>(aLayer);
-    if (IsRetained()) {
-      readback.BuildUpdates(container);
-    }
-  
-    nsAutoTArray<Layer*, 12> children;
-    container->SortChildrenBy3DZOrder(children);
-
-    for (uint32_t i = 0; i < children.Length(); i++) {
-      PaintLayer(groupTarget, children.ElementAt(i), aCallback, aCallbackData, &readback);
-      if (mTransactionIncomplete)
-        break;
-    }
-  }
-
-  if (needsGroup) {
-    bool blitComplete = false;
-    if (is2D) {
-      PopGroupToSourceWithCachedSurface(aTarget, groupTarget);
-    } else {
-      // Temporary fast fix for bug 725886
-      // Revert these changes when 725886 is ready
-      if (!clipIsEmpty) {
-        NS_ABORT_IF_FALSE(untransformedSurface, 
-                          "We should always allocate an untransformed surface with 3d transforms!");
-        gfxPoint offset;
-        bool dontBlit = needsClipToVisibleRegion || mTransactionIncomplete ||
-                          aLayer->GetEffectiveOpacity() != 1.0f;
 #ifdef DEBUG
-        if (aLayer->GetDebugColorIndex() != 0) {
+    if (aLayer->GetDebugColorIndex() != 0) {
-          gfxRGBA  color((aLayer->GetDebugColorIndex() & 1) ? 1.0 : 0.0,
+      gfxRGBA  color((aLayer->GetDebugColorIndex() & 1) ? 1.0 : 0.0,
-                         (aLayer->GetDebugColorIndex() & 2) ? 1.0 : 0.0,
+                     (aLayer->GetDebugColorIndex() & 2) ? 1.0 : 0.0,
-                         (aLayer->GetDebugColorIndex() & 4) ? 1.0 : 0.0,
+                     (aLayer->GetDebugColorIndex() & 4) ? 1.0 : 0.0,
-                         1.0);
+                     1.0);
 
-          nsRefPtr<gfxContext> temp = new gfxContext(untransformedSurface);
+      nsRefPtr<gfxContext> temp = new gfxContext(untransformedSurface);
-          temp->SetColor(color);
+      temp->SetColor(color);
-          temp->Paint();
+      temp->Paint();
-        }
+    }
 #endif
+    const gfx3DMatrix& effectiveTransform = aLayer->GetEffectiveTransform();
+    nsRefPtr<gfxASurface> result =
+      Transform3D(untransformedSurface, aTarget, bounds,
+                  effectiveTransform, offset, dontBlit);
 
-        nsRefPtr<gfxASurface> result =
+    if (result) {
-          Transform3D(untransformedSurface, aTarget, bounds,
+      aTarget->SetSource(result, offset);
-                      effectiveTransform, offset, dontBlit);
+      FlushGroup(paintContext, needsClipToVisibleRegion);
-
-        blitComplete = !result;
-        if (result) {
-          aTarget->SetSource(result, offset);
-        }
-      }
     }
-    // If we're doing our own double-buffering, we need to avoid drawing
-    // the results of an incomplete transaction to the destination surface ---
-    // that could cause flicker. Double-buffering is implemented using a
-    // temporary surface for one or more container layers, so we need to stop
-    // those temporary surfaces from being composited to aTarget.
-    // ApplyDoubleBuffering guarantees that this container layer can't
-    // intersect any other leaf layers, so if the transaction is not yet marked
-    // incomplete, the contents of this container layer are the final contents
-    // for the window.
-    if (!mTransactionIncomplete && !blitComplete) {
-      if (needsClipToVisibleRegion) {
-        gfxUtils::ClipToRegion(aTarget, aLayer->GetEffectiveVisibleRegion());
-      }
-      AutoSetOperator setOperator(aTarget, container->GetOperator());
-      PaintWithMask(aTarget, aLayer->GetEffectiveOpacity(),
-                    HasShadowManager() ? nullptr : aLayer->GetMaskLayer());
-    }
-  }
-
-  if (pushedTargetOpaqueRect) {
-    if (aTarget->IsCairo()) {
-      currentSurface->SetOpaqueRect(gfxRect(0, 0, 0, 0));
-    } else {
-      dt->SetOpaqueRect(IntRect());
-    }
-  }
-
-  if (needsSaveRestore) {
-    aTarget->Restore();
-  } else {
-    aTarget->SetMatrix(savedMatrix);
   }
 }
 

gfx/layers/basic/BasicLayers.h

@@ -28,6 +28,7 @@ class ShadowCanvasLayer;
 class ShadowColorLayer;
 class ReadbackProcessor;
 class ImageFactory;
+class PaintContext;
 
 /**
  * This is a cairo/Thebes-only, main-thread-only implementation of layers.
@@ -159,6 +160,15 @@ protected:
   };
   TransactionPhase mPhase;
 
+  // This is the main body of the PaintLayer routine which will if it has
+  // children, recurse into PaintLayer() otherwise it will paint using the
+  // underlying Paint() method of the Layer. It will not do both.
+  void PaintSelfOrChildren(PaintContext& aPaintContext, gfxContext* aGroupTarget);
+
+  // Paint the group onto the underlying target. This is used by PaintLayer to
+  // flush the group to the underlying target.
+  void FlushGroup(PaintContext& aPaintContext, bool aNeedsClipToVisibleRegion);
+
   // Paints aLayer to mTarget.
   void PaintLayer(gfxContext* aTarget,
                   Layer* aLayer,