Bug 842114 - Part 1. Correct assert condition and promote NS_ASSERTION to MOZ_ASSERT. r=heycam
☠☠ backed out by 09a2cd12ac20 ☠ ☠
authorcku <cku@mozilla.com>
Fri, 03 Feb 2017 21:37:08 +0800
changeset 343108 95eabf0aa83b70bd943676e5f394002d9b9815df
parent 343107 9d1b280faca1c227646818c3f32e43120683977f
child 343109 d50d32601e7f7a78163fab0f99bfe3335f659e02
push id31369
push userkwierso@gmail.com
push dateThu, 16 Feb 2017 00:18:40 +0000
treeherdermozilla-central@e9b926463f9e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs842114
milestone54.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 842114 - Part 1. Correct assert condition and promote NS_ASSERTION to MOZ_ASSERT. r=heycam 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 aCheckPreEffectsBBoxPropChache)
     : mFirstContinuation(aFirstContinuation)
     , mCurrentFrame(aCurrentFrame)
     , mCurrentFrameOverflowArea(aCurrentFrameOverflowArea)
+    , mCheckPreEffectsBBoxPropChache(aCheckPreEffectsBBoxPropChache)
   {
     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, mCheckPreEffectsBBoxPropChache);
     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()->AsGecko()->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          mCheckPreEffectsBBoxPropChache;
 };
 
 /**
  * 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);