Bug 1187851 patch 1 - Make dynamic changes to filter and perspective change fixed position containing block for descendants. r=roc
☠☠ backed out by d4d2b60cd412 ☠ ☠
authorL. David Baron <dbaron@dbaron.org>
Sun, 02 Aug 2015 21:03:09 -0700
changeset 287468 f24dbdeeaef1272ee276f66f62b40736a572e62b
parent 287467 80ef9bb2c2e9ebaf375e078428e9615d46adbde9
child 287469 5dcb38c7f1b84d7fc4d0255a2b10165dfaceb941
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs1187851
milestone42.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 1187851 patch 1 - Make dynamic changes to filter and perspective change fixed position containing block for descendants. r=roc Note that this now uses AddAndRemoveTransform hints for changes that are other than adding and removing a transform. Since there's still a little bit of transform-related stuff there too (which I did make conditional), I figure it's probably best to leave the name as-is, although I'd be open to renaming it as well. As expected, without the patch, the filter and perspective tests fail, but the added transform test passes. All the tests pass locally with the patch.
layout/base/RestyleManager.cpp
layout/reftests/w3c-css/submitted/filters/filter-containing-block-dynamic-1-ref.html
layout/reftests/w3c-css/submitted/filters/filter-containing-block-dynamic-1a.html
layout/reftests/w3c-css/submitted/filters/filter-containing-block-dynamic-1b.html
layout/reftests/w3c-css/submitted/filters/reftest.list
layout/reftests/w3c-css/submitted/reftest.list
layout/reftests/w3c-css/submitted/transforms/containing-block-dynamic-1-ref.html
layout/reftests/w3c-css/submitted/transforms/perspective-containing-block-dynamic-1a.html
layout/reftests/w3c-css/submitted/transforms/perspective-containing-block-dynamic-1b.html
layout/reftests/w3c-css/submitted/transforms/reftest.list
layout/reftests/w3c-css/submitted/transforms/transform-containing-block-dynamic-1a.html
layout/reftests/w3c-css/submitted/transforms/transform-containing-block-dynamic-1b.html
layout/style/nsStyleStruct.cpp
layout/style/nsStyleStruct.h
layout/style/nsStyleStructInlines.h
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -771,20 +771,19 @@ RestyleManager::ProcessRestyledFrames(ns
         for (nsIFrame *cont = frame; cont;
              cont = nsLayoutUtils::GetNextContinuationOrIBSplitSibling(cont)) {
           // Normally frame construction would set state bits as needed,
           // but we're not going to reconstruct the frame so we need to set them.
           // It's because we need to set this state on each affected frame
           // that we can't coalesce nsChangeHint_AddOrRemoveTransform hints up
           // to ancestors (i.e. it can't be an inherited change hint).
           if (cont->IsAbsPosContaininingBlock()) {
-            // If a transform has been added, we'll be taking this path,
-            // but we may be taking this path even if a transform has been
-            // removed. It's OK to add the bit even if it's not needed.
-            cont->AddStateBits(NS_FRAME_MAY_BE_TRANSFORMED);
+            if (cont->StyleDisplay()->HasTransform(cont)) {
+              cont->AddStateBits(NS_FRAME_MAY_BE_TRANSFORMED);
+            }
             if (!cont->IsAbsoluteContainer() &&
                 (cont->GetStateBits() & NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN)) {
               cont->MarkAsAbsoluteContainingBlock();
             }
           } else {
             // Don't remove NS_FRAME_MAY_BE_TRANSFORMED since it may still by
             // transformed by other means. It's OK to have the bit even if it's
             // not needed.
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/filters/filter-containing-block-dynamic-1-ref.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS filters: Creating containing block for fixed positioned elements</title>
+<link rel="author" title="L. David Baron" href="http://dbaron.org/">
+<link rel="author" title="Mozilla" href="http://www.mozilla.org/">
+<style>
+  html, body { margin: 0; padding: 0 }
+  #fixedmoves {
+    position: absolute;
+    top: 150px;
+    left: 150px;
+    background: green;
+    height: 100px;
+    width: 100px;
+  }
+</style>
+<body>
+  <div id="fixedmoves"></div>
+</body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/filters/filter-containing-block-dynamic-1a.html
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS filters: Creating containing block for fixed positioned elements</title>
+<link rel="author" title="L. David Baron" href="http://dbaron.org/">
+<link rel="author" title="Mozilla" href="http://www.mozilla.org/">
+<link rel="help" href="https://drafts.fxtf.org/filters/#FilterProperty">
+<link rel="help" href="http://www.w3.org/2015/02/10-fx-minutes.html#action02">
+<link rel="match" href="filter-containing-block-dynamic-1-ref.html">
+<meta name="flags" content="dom">
+<style>
+  html, body { margin: 0; padding: 0 }
+  #changefilter {
+    position: absolute;
+    top: 100px;
+    left: 100px;
+  }
+
+  #abscovered {
+    position: absolute;
+    top: 50px;
+    left: 50px;
+    background: red;
+    height: 100px;
+    width: 100px;
+  }
+
+  #fixedmoves {
+    position: fixed;
+    top: 150px;
+    left: 150px;
+    background: green;
+    height: 100px;
+    width: 100px;
+  }
+</style>
+<body>
+  <div id="changefilter" style="filter: blur(4px)">
+    <div id="abscovered"></div>
+    <div id="fixedmoves"></div>
+  </div>
+  <script>
+    var changefilter = document.getElementById("changefilter");
+    var fixedmoves = document.getElementById("fixedmoves");
+    var causeFlush = fixedmoves.offsetTop;
+    changefilter.style.filter = "";
+  </script>
+</body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/filters/filter-containing-block-dynamic-1b.html
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS filters: Creating containing block for fixed positioned elements</title>
+<link rel="author" title="L. David Baron" href="http://dbaron.org/">
+<link rel="author" title="Mozilla" href="http://www.mozilla.org/">
+<link rel="help" href="https://drafts.fxtf.org/filters/#FilterProperty">
+<link rel="help" href="http://www.w3.org/2015/02/10-fx-minutes.html#action02">
+<link rel="match" href="filter-containing-block-dynamic-1-ref.html">
+<meta name="flags" content="dom">
+<style>
+  html, body { margin: 0; padding: 0 }
+  #changefilter {
+    position: absolute;
+    top: 100px;
+    left: 100px;
+  }
+
+  #abscovered {
+    position: absolute;
+    top: 50px;
+    left: 50px;
+    background: red;
+    height: 100px;
+    width: 100px;
+  }
+
+  #fixedmoves {
+    position: fixed;
+    top: 50px;
+    left: 50px;
+    background: green;
+    height: 100px;
+    width: 100px;
+  }
+</style>
+<body>
+  <div id="changefilter">
+    <div id="abscovered"></div>
+    <div id="fixedmoves"></div>
+  </div>
+  <script>
+    var changefilter = document.getElementById("changefilter");
+    var fixedmoves = document.getElementById("fixedmoves");
+    var causeFlush = fixedmoves.offsetTop;
+    changefilter.style.filter = "blur(0px)";
+  </script>
+</body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/filters/reftest.list
@@ -0,0 +1,2 @@
+== filter-containing-block-dynamic-1a.html filter-containing-block-dynamic-1-ref.html
+== filter-containing-block-dynamic-1b.html filter-containing-block-dynamic-1-ref.html
--- a/layout/reftests/w3c-css/submitted/reftest.list
+++ b/layout/reftests/w3c-css/submitted/reftest.list
@@ -14,16 +14,19 @@ include css21/reftest.list
 # include animations/reftest.list
 
 # Backgrounds and Borders Level 3
 # include background3/reftest.list
 
 # Conditional Rules Level 3
 include conditional3/reftest.list
 
+# Filter Effects Module
+include filters/reftest.list
+
 # Flexible Box Layout Module
 include flexbox/reftest.list
 
 # Fonts Level 3
 include fonts3/reftest.list
 
 # Image Values and Replaced Content Level 3
 include images3/reftest.list
@@ -48,17 +51,17 @@ include ruby/reftest.list
 
 # Text Level 3
 include text3/reftest.list
 
 # Text Decoration Level 3
 include text-decor-3/reftest.list
 
 # Transforms
-# include transforms/reftest.list
+include transforms/reftest.list
 
 # Transitions
 # include transitions/reftest.list
 
 # User Interface Level 3
 include ui3/reftest.list
 
 # Values and Units Level 3
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/transforms/containing-block-dynamic-1-ref.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS transforms: Creating containing block for fixed positioned elements</title>
+<link rel="author" title="L. David Baron" href="http://dbaron.org/">
+<link rel="author" title="Mozilla" href="http://www.mozilla.org/">
+<style>
+  html, body { margin: 0; padding: 0 }
+  #fixedmoves {
+    position: absolute;
+    top: 150px;
+    left: 150px;
+    background: green;
+    height: 100px;
+    width: 100px;
+  }
+</style>
+<body>
+  <div id="fixedmoves"></div>
+</body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/transforms/perspective-containing-block-dynamic-1a.html
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS transforms: Creating containing block for fixed positioned elements</title>
+<link rel="author" title="L. David Baron" href="http://dbaron.org/">
+<link rel="author" title="Mozilla" href="http://www.mozilla.org/">
+<link rel="help" href="https://drafts.csswg.org/css-transforms/#perspective-property">
+<link rel="match" href="containing-block-dynamic-1-ref.html">
+<meta name="assert" content="It also establishes a containing block (somewhat similar to position: relative), just like the transform property does.">
+<meta name="flags" content="dom">
+<style>
+  html, body { margin: 0; padding: 0 }
+  #changeperspective {
+    position: absolute;
+    top: 100px;
+    left: 100px;
+  }
+
+  #abscovered {
+    position: absolute;
+    top: 50px;
+    left: 50px;
+    background: red;
+    height: 100px;
+    width: 100px;
+  }
+
+  #fixedmoves {
+    position: fixed;
+    top: 150px;
+    left: 150px;
+    background: green;
+    height: 100px;
+    width: 100px;
+  }
+</style>
+<body>
+  <div id="changeperspective" style="perspective: 1000px">
+    <div id="abscovered"></div>
+    <div id="fixedmoves"></div>
+  </div>
+  <script>
+    var changeperspective = document.getElementById("changeperspective");
+    var fixedmoves = document.getElementById("fixedmoves");
+    var causeFlush = fixedmoves.offsetTop;
+    changeperspective.style.perspective = "";
+  </script>
+</body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/transforms/perspective-containing-block-dynamic-1b.html
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS transforms: Creating containing block for fixed positioned elements</title>
+<link rel="author" title="L. David Baron" href="http://dbaron.org/">
+<link rel="author" title="Mozilla" href="http://www.mozilla.org/">
+<link rel="help" href="https://drafts.csswg.org/css-transforms/#perspective-property">
+<link rel="match" href="containing-block-dynamic-1-ref.html">
+<meta name="assert" content="It also establishes a containing block (somewhat similar to position: relative), just like the transform property does.">
+<meta name="flags" content="dom">
+<style>
+  html, body { margin: 0; padding: 0 }
+  #changeperspective {
+    position: absolute;
+    top: 100px;
+    left: 100px;
+  }
+
+  #abscovered {
+    position: absolute;
+    top: 50px;
+    left: 50px;
+    background: red;
+    height: 100px;
+    width: 100px;
+  }
+
+  #fixedmoves {
+    position: fixed;
+    top: 50px;
+    left: 50px;
+    background: green;
+    height: 100px;
+    width: 100px;
+  }
+</style>
+<body>
+  <div id="changeperspective">
+    <div id="abscovered"></div>
+    <div id="fixedmoves"></div>
+  </div>
+  <script>
+    var changeperspective = document.getElementById("changeperspective");
+    var fixedmoves = document.getElementById("fixedmoves");
+    var causeFlush = fixedmoves.offsetTop;
+    changeperspective.style.perspective = "1000px";
+  </script>
+</body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/transforms/reftest.list
@@ -0,0 +1,4 @@
+== transform-containing-block-dynamic-1a.html containing-block-dynamic-1-ref.html
+== transform-containing-block-dynamic-1b.html containing-block-dynamic-1-ref.html
+== perspective-containing-block-dynamic-1a.html containing-block-dynamic-1-ref.html
+== perspective-containing-block-dynamic-1b.html containing-block-dynamic-1-ref.html
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/transforms/transform-containing-block-dynamic-1a.html
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS transforms: Creating containing block for fixed positioned elements</title>
+<link rel="author" title="L. David Baron" href="http://dbaron.org/">
+<link rel="author" title="Mozilla" href="http://www.mozilla.org/">
+<link rel="help" href="https://drafts.csswg.org/css-transforms/#transform-rendering">
+<link rel="help" href="https://drafts.csswg.org/css-transforms/#transform-property">
+<link rel="match" href="containing-block-dynamic-1-ref.html">
+<meta name="assert" content="For elements whose layout is governed by the CSS box model, any value other than none for the transform results in the creation of both a stacking context and a containing block. The object acts as a containing block for fixed positioned descendants.">
+<meta name="assert" content="The object acts as a containing block for fixed positioned descendants."
+<meta name="flags" content="dom">
+<style>
+  html, body { margin: 0; padding: 0 }
+  #changetransform {
+    position: absolute;
+    top: 100px;
+    left: 100px;
+  }
+
+  #abscovered {
+    position: absolute;
+    top: 50px;
+    left: 50px;
+    background: red;
+    height: 100px;
+    width: 100px;
+  }
+
+  #fixedmoves {
+    position: fixed;
+    top: 150px;
+    left: 150px;
+    background: green;
+    height: 100px;
+    width: 100px;
+  }
+</style>
+<body>
+  <div id="changetransform" style="transform: translateX(4px)">
+    <div id="abscovered"></div>
+    <div id="fixedmoves"></div>
+  </div>
+  <script>
+    var changetransform = document.getElementById("changetransform");
+    var fixedmoves = document.getElementById("fixedmoves");
+    var causeFlush = fixedmoves.offsetTop;
+    changetransform.style.transform = "";
+  </script>
+</body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/transforms/transform-containing-block-dynamic-1b.html
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS transforms: Creating containing block for fixed positioned elements</title>
+<link rel="author" title="L. David Baron" href="http://dbaron.org/">
+<link rel="author" title="Mozilla" href="http://www.mozilla.org/">
+<link rel="help" href="https://drafts.csswg.org/css-transforms/#transform-rendering">
+<link rel="help" href="https://drafts.csswg.org/css-transforms/#transform-property">
+<link rel="match" href="containing-block-dynamic-1-ref.html">
+<meta name="assert" content="For elements whose layout is governed by the CSS box model, any value other than none for the transform results in the creation of both a stacking context and a containing block. The object acts as a containing block for fixed positioned descendants.">
+<meta name="assert" content="The object acts as a containing block for fixed positioned descendants."
+<meta name="flags" content="dom">
+<style>
+  html, body { margin: 0; padding: 0 }
+  #changetransform {
+    position: absolute;
+    top: 100px;
+    left: 100px;
+  }
+
+  #abscovered {
+    position: absolute;
+    top: 50px;
+    left: 50px;
+    background: red;
+    height: 100px;
+    width: 100px;
+  }
+
+  #fixedmoves {
+    position: fixed;
+    top: 50px;
+    left: 50px;
+    background: green;
+    height: 100px;
+    width: 100px;
+  }
+</style>
+<body>
+  <div id="changetransform">
+    <div id="abscovered"></div>
+    <div id="fixedmoves"></div>
+  </div>
+  <script>
+    var changetransform = document.getElementById("changetransform");
+    var fixedmoves = document.getElementById("fixedmoves");
+    var causeFlush = fixedmoves.offsetTop;
+    changetransform.style.transform = "translateX(0px)";
+  </script>
+</body>
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -1300,16 +1300,21 @@ nsStyleSVGReset::nsStyleSVGReset(const n
   mVectorEffect = aSource.mVectorEffect;
   mMaskType = aSource.mMaskType;
 }
 
 nsChangeHint nsStyleSVGReset::CalcDifference(const nsStyleSVGReset& aOther) const
 {
   nsChangeHint hint = nsChangeHint(0);
 
+  if (HasFilters() != aOther.HasFilters()) {
+    // A change from/to being a containing block for position:fixed.
+    NS_UpdateHint(hint, nsChangeHint_AddOrRemoveTransform);
+  }
+
   if (mClipPath != aOther.mClipPath ||
       !EqualURIs(mMask, aOther.mMask) ||
       mFilters != aOther.mFilters) {
     NS_UpdateHint(hint, nsChangeHint_UpdateEffects);
     NS_UpdateHint(hint, nsChangeHint_RepaintFrame);
     // We only actually need to update the overflow area for filter
     // changes.  However, mask and clip-path changes require that we
     // update the PreEffectsBBoxProperty, which is done during overflow
@@ -2842,16 +2847,21 @@ nsChangeHint nsStyleDisplay::CalcDiffere
       }
     
     for (uint8_t index = 0; index < 2; ++index)
       if (mPerspectiveOrigin[index] != aOther.mPerspectiveOrigin[index]) {
         NS_UpdateHint(transformHint, kUpdateOverflowAndRepaintHint);
         break;
       }
 
+    if (HasPerspectiveStyle() != aOther.HasPerspectiveStyle()) {
+      // A change from/to being a containing block for position:fixed.
+      NS_UpdateHint(hint, nsChangeHint_AddOrRemoveTransform);
+    }
+
     if (mChildPerspective != aOther.mChildPerspective ||
         mTransformStyle != aOther.mTransformStyle ||
         mTransformBox != aOther.mTransformBox)
       NS_UpdateHint(transformHint, kUpdateOverflowAndRepaintHint);
 
     if (mBackfaceVisibility != aOther.mBackfaceVisibility)
       NS_UpdateHint(transformHint, nsChangeHint_RepaintFrame);
 
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -2128,17 +2128,17 @@ struct nsStyleDisplay {
   // specified, or null to indicate there is no transform.  (inherit or
   // initial are replaced by an actual list of transform functions, or
   // null, as appropriate.)
   uint8_t mBackfaceVisibility;
   uint8_t mTransformStyle;
   uint8_t mTransformBox;        // [reset] see nsStyleConsts.h
   nsRefPtr<nsCSSValueSharedList> mSpecifiedTransform; // [reset]
   nsStyleCoord mTransformOrigin[3]; // [reset] percent, coord, calc, 3rd param is coord, calc only
-  nsStyleCoord mChildPerspective; // [reset] coord
+  nsStyleCoord mChildPerspective; // [reset] none, coord
   nsStyleCoord mPerspectiveOrigin[2]; // [reset] percent, coord, calc
 
   nsAutoTArray<mozilla::StyleTransition, 1> mTransitions; // [reset]
   // The number of elements in mTransitions that are not from repeating
   // a list due to another property being longer.
   uint32_t mTransitionTimingFunctionCount,
            mTransitionDurationCount,
            mTransitionDelayCount,
@@ -3158,29 +3158,31 @@ struct nsStyleSVGReset {
   void Destroy(nsPresContext* aContext) {
     this->~nsStyleSVGReset();
     aContext->PresShell()->
       FreeByObjectID(nsPresArena::nsStyleSVGReset_id, this);
   }
 
   nsChangeHint CalcDifference(const nsStyleSVGReset& aOther) const;
   static nsChangeHint MaxDifference() {
-    return NS_CombineHint(nsChangeHint_UpdateEffects,
-            NS_CombineHint(nsChangeHint_UpdateOverflow, NS_STYLE_HINT_REFLOW));
+    return nsChangeHint_UpdateEffects |
+           nsChangeHint_UpdateOverflow |
+           nsChangeHint_AddOrRemoveTransform |
+           NS_STYLE_HINT_REFLOW;
   }
   static nsChangeHint DifferenceAlwaysHandledForDescendants() {
     // CalcDifference never returns the reflow hints that are sometimes
     // handled for descendants as hints not handled for descendants.
     return nsChangeHint_NeedReflow |
            nsChangeHint_ReflowChangesSizeOrPosition |
            nsChangeHint_ClearAncestorIntrinsics;
   }
 
   bool HasFilters() const {
-    return mFilters.Length() > 0;
+    return !mFilters.IsEmpty();
   }
 
   bool HasNonScalingStroke() const {
     return mVectorEffect == NS_STYLE_VECTOR_EFFECT_NON_SCALING_STROKE;
   }
 
   nsStyleClipPath mClipPath;          // [reset]
   nsTArray<nsStyleFilter> mFilters;   // [reset]
--- a/layout/style/nsStyleStructInlines.h
+++ b/layout/style/nsStyleStructInlines.h
@@ -133,18 +133,20 @@ nsStyleDisplay::HasTransform(const nsIFr
 {
   NS_ASSERTION(aContextFrame->StyleDisplay() == this, "unexpected aContextFrame");
   return HasTransformStyle() && aContextFrame->IsFrameOfType(nsIFrame::eSupportsCSSTransforms);
 }
 
 bool
 nsStyleDisplay::IsFixedPosContainingBlock(const nsIFrame* aContextFrame) const
 {
+  NS_ASSERTION(aContextFrame->StyleDisplay() == this,
+               "unexpected aContextFrame");
   return (HasTransform(aContextFrame) || HasPerspectiveStyle() ||
-          !aContextFrame->StyleSVGReset()->mFilters.IsEmpty()) &&
+          aContextFrame->StyleSVGReset()->HasFilters()) &&
       !aContextFrame->IsSVGText();
 }
 
 bool
 nsStyleDisplay::IsAbsPosContainingBlock(const nsIFrame* aContextFrame) const
 {
   NS_ASSERTION(aContextFrame->StyleDisplay() == this,
                "unexpected aContextFrame");