Bug 842114 - Part 1. Correct assert condition and promote NS_ASSERTION to MOZ_ASSERT. draft
authorcku <cku@mozilla.com>
Fri, 03 Feb 2017 21:37:08 +0800
changeset 485099 5775b9347bf941d6f5d3b90bf1a94b0bac8b7035
parent 485098 a9ec72f82299250e6023988e238931bbca0ef7fa
child 485100 52660ce91463b958fea87be652a3cecf0387008d
push id45631
push userbmo:cku@mozilla.com
push dateThu, 16 Feb 2017 04:03:52 +0000
bugs842114
milestone54.0a1
Bug 842114 - Part 1. Correct assert condition and promote NS_ASSERTION to MOZ_ASSERT. Except during restyle process, we should skip this checking in reflow as well. But what really should do is to skip this checking if this function is called from ComputeEffectsRect. The reason is explained in the beginning of ComputePostEffectsVisualOverflowRect. Also promote NS_ASSERTION to MOZ_ASSERTION in this patch. MozReview-Commit-ID: 3CuKkdR4kTK
layout/svg/nsSVGIntegrationUtils.cpp
--- a/layout/svg/nsSVGIntegrationUtils.cpp
+++ b/layout/svg/nsSVGIntegrationUtils.cpp
@@ -50,38 +50,42 @@ public:
    * If the pre-effects visual overflow rect of the frame being examined
    * happens to be known, it can be passed in as aCurrentFrame and its
    * pre-effects visual overflow rect can be passed in as
    * aCurrentFrameOverflowArea. This is just an optimization to save a
    * frame property lookup - these arguments are optional.
    */
   PreEffectsVisualOverflowCollector(nsIFrame* aFirstContinuation,
                                     nsIFrame* aCurrentFrame,
-                                    const nsRect& aCurrentFrameOverflowArea)
+                                    const nsRect& aCurrentFrameOverflowArea,
+                                    bool aCheckPreEffectsBBoxPropCache)
     : mFirstContinuation(aFirstContinuation)
     , mCurrentFrame(aCurrentFrame)
     , mCurrentFrameOverflowArea(aCurrentFrameOverflowArea)
+    , mCheckPreEffectsBBoxPropCache(aCheckPreEffectsBBoxPropCache)
   {
     NS_ASSERTION(!mFirstContinuation->GetPrevContinuation(),
                  "We want the first continuation here");
   }
 
   virtual void AddBox(nsIFrame* aFrame) override {
-    nsRect overflow = (aFrame == mCurrentFrame) ?
-      mCurrentFrameOverflowArea : GetPreEffectsVisualOverflowRect(aFrame);
+    nsRect overflow = (aFrame == mCurrentFrame)
+      ? mCurrentFrameOverflowArea
+      : GetPreEffectsVisualOverflowRect(aFrame, mCheckPreEffectsBBoxPropCache);
     mResult.UnionRect(mResult, overflow + aFrame->GetOffsetTo(mFirstContinuation));
   }
 
   nsRect GetResult() const {
     return mResult;
   }
 
 private:
 
-  static nsRect GetPreEffectsVisualOverflowRect(nsIFrame* aFrame) {
+  static nsRect GetPreEffectsVisualOverflowRect(nsIFrame* aFrame,
+                                                bool aCheckPropCache) {
     nsRect* r = aFrame->Properties().Get(nsIFrame::PreEffectsBBoxProperty());
     if (r) {
       return *r;
     }
     // Despite the fact that we're invoked for frames with SVG effects applied,
     // we can actually get here. All continuations and IB split siblings of a
     // frame with SVG effects applied will have the PreEffectsBBoxProperty
     // property set on them. Therefore, the frames that are passed to us will
@@ -107,65 +111,65 @@ private:
     //
     //  * reftests/svg/svg-integration/clipPath-html-06.xhtml
     //  * reftests/svg/svg-integration/clipPath-html-06-extref.xhtml
     //
     // If we ever got passed a frame with the PreTransformOverflowAreasProperty
     // property set, that would be bad, since then our GetVisualOverflowRect()
     // call would give us the post-effects, and post-transform, overflow rect.
     //
-    // There are two more exceptions:
-    // 1. In nsStyleImageLayers::Layer::CalcDifference, we do not add
-    //    nsChangeHint_UpdateOverflow hint when image mask(not SVG mask)
-    //    property value changed, since replace image mask does not cause
-    //    layout change. So even if we apply a new mask image to this frame,
-    //    PreEffectsBBoxProperty might still left empty.
-    // 2. During restyling: before the last continuation is restyled, there
-    //    is no guarantee that every continuation carries a
-    //    PreEffectsBBoxProperty property.
+    // There is one more exceptions, in
+    // nsStyleImageLayers::Layer::CalcDifference, we do not add
+    // nsChangeHint_UpdateOverflow hint when image mask(not SVG mask) property
+    // value changed, since replace image mask does not cause layout change.
+    // So even if we apply a new mask image to this frame,
+    // PreEffectsBBoxProperty might still left empty.
 #ifdef DEBUG
-    nsIFrame* firstFrame =
-      nsLayoutUtils::FirstContinuationOrIBSplitSibling(aFrame);
-    bool mightHaveNoneSVGMask =
-      nsSVGEffects::GetEffectProperties(firstFrame).MightHaveNoneSVGMask();
-    bool inRestyle =
-      aFrame->PresContext()->RestyleManager()->IsInStyleRefresh();
+    if (aCheckPropCache) {
+      nsIFrame* firstFrame =
+        nsLayoutUtils::FirstContinuationOrIBSplitSibling(aFrame);
+      bool mightHaveNoneSVGMask =
+        nsSVGEffects::GetEffectProperties(firstFrame).MightHaveNoneSVGMask();
 
-    NS_ASSERTION(mightHaveNoneSVGMask || inRestyle ||
+      MOZ_ASSERT(mightHaveNoneSVGMask ||
                  aFrame->GetParent()->StyleContext()->GetPseudo() ==
-                   nsCSSAnonBoxes::mozAnonymousBlock,
+                 nsCSSAnonBoxes::mozAnonymousBlock,
                  "How did we getting here, then?");
+    }
 #endif
     NS_ASSERTION(!aFrame->Properties().Get(
                    aFrame->PreTransformOverflowAreasProperty()),
                  "GetVisualOverflowRect() won't return the pre-effects rect!");
     return aFrame->GetVisualOverflowRect();
   }
 
   nsIFrame*     mFirstContinuation;
   nsIFrame*     mCurrentFrame;
   const nsRect& mCurrentFrameOverflowArea;
   nsRect        mResult;
+  bool          mCheckPreEffectsBBoxPropCache;
 };
 
 /**
  * Gets the union of the pre-effects visual overflow rects of all of a frame's
  * continuations, in "user space".
  */
 static nsRect
 GetPreEffectsVisualOverflowUnion(nsIFrame* aFirstContinuation,
                                  nsIFrame* aCurrentFrame,
                                  const nsRect& aCurrentFramePreEffectsOverflow,
-                                 const nsPoint& aFirstContinuationToUserSpace)
+                                 const nsPoint& aFirstContinuationToUserSpace,
+                                 bool aCheckPreEffectsBBoxPropCache)
 {
   NS_ASSERTION(!aFirstContinuation->GetPrevContinuation(),
                "Need first continuation here");
   PreEffectsVisualOverflowCollector collector(aFirstContinuation,
                                               aCurrentFrame,
-                                              aCurrentFramePreEffectsOverflow);
+                                              aCurrentFramePreEffectsOverflow,
+                                              aCheckPreEffectsBBoxPropCache);
   // Compute union of all overflow areas relative to aFirstContinuation:
   nsLayoutUtils::GetAllInFlowBoxes(aFirstContinuation, &collector);
   // Return the result in user space:
   return collector.GetResult() + aFirstContinuationToUserSpace;
 }
 
 
 bool
@@ -233,17 +237,18 @@ gfxRect
 nsSVGIntegrationUtils::GetSVGBBoxForNonSVGFrame(nsIFrame* aNonSVGFrame)
 {
   NS_ASSERTION(!aNonSVGFrame->IsFrameOfType(nsIFrame::eSVG),
                "SVG frames should not get here");
   nsIFrame* firstFrame =
     nsLayoutUtils::FirstContinuationOrIBSplitSibling(aNonSVGFrame);
   // 'r' is in "user space":
   nsRect r = GetPreEffectsVisualOverflowUnion(firstFrame, nullptr, nsRect(),
-                                              GetOffsetToBoundingBox(firstFrame));
+                                              GetOffsetToBoundingBox(firstFrame),
+                                              true);
   return nsLayoutUtils::RectToGfxRect(r,
            aNonSVGFrame->PresContext()->AppUnitsPerCSSPixel());
 }
 
 // XXX Since we're called during reflow, this method is broken for frames with
 // continuations. When we're called for a frame with continuations, we're
 // called for each continuation in turn as it's reflowed. However, it isn't
 // until the last continuation is reflowed that this method's
@@ -295,17 +300,21 @@ nsRect
   nsPoint firstFrameToBoundingBox = GetOffsetToBoundingBox(firstFrame);
   // overrideBBox is in "user space", in _CSS_ pixels:
   // XXX Why are we rounding out to pixel boundaries? We don't do that in
   // GetSVGBBoxForNonSVGFrame, and it doesn't appear to be necessary.
   gfxRect overrideBBox =
     nsLayoutUtils::RectToGfxRect(
       GetPreEffectsVisualOverflowUnion(firstFrame, aFrame,
                                        aPreEffectsOverflowRect,
-                                       firstFrameToBoundingBox),
+                                       firstFrameToBoundingBox,
+                                       false /* See the beginning of the
+                                                comment above this function to
+                                                know why we skip this
+                                                checking. */),
       aFrame->PresContext()->AppUnitsPerCSSPixel());
   overrideBBox.RoundOut();
 
   nsRect overflowRect =
     nsFilterInstance::GetPostFilterBounds(firstFrame, &overrideBBox);
 
   // Return overflowRect relative to aFrame, rather than "user space":
   return overflowRect - (aFrame->GetOffsetTo(firstFrame) + firstFrameToBoundingBox);