Bug 1376406 - Make ::before and ::after the only pseudos that do parent display-based style fixup. r=dbaron, a=lizzard
authorCameron McCormack <cam@mcc.id.au>
Tue, 11 Jul 2017 10:38:23 +0800
changeset 414290 87127a0ad8e6849dbb9e039f93082966d70e2766
parent 414289 5e694949cac6fa216cb0579814a9c13d538542c2
child 414291 fdedc974cf3b3eea4ed3bca0c55624dcc85c010c
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron, lizzard
bugs1376406
milestone55.0
Bug 1376406 - Make ::before and ::after the only pseudos that do parent display-based style fixup. r=dbaron, a=lizzard MozReview-Commit-ID: Hxq8v1NSYvc
layout/style/nsCSSPseudoElementList.h
layout/style/nsCSSPseudoElements.h
layout/style/nsStyleSet.cpp
--- a/layout/style/nsCSSPseudoElementList.h
+++ b/layout/style/nsCSSPseudoElementList.h
@@ -20,18 +20,20 @@
  * value_ : The pseudo-element as a string, with single-colon syntax,
  *          used as the string value of the atom.
  * flags_ : A bitfield containing flags defined in nsCSSPseudoElements.h
  */
 
 // OUTPUT_CLASS=nsCSSPseudoElements
 // MACRO_NAME=CSS_PSEUDO_ELEMENT
 
-CSS_PSEUDO_ELEMENT(after, ":after", CSS_PSEUDO_ELEMENT_IS_CSS2)
-CSS_PSEUDO_ELEMENT(before, ":before", CSS_PSEUDO_ELEMENT_IS_CSS2)
+CSS_PSEUDO_ELEMENT(after, ":after", CSS_PSEUDO_ELEMENT_IS_CSS2 |
+                                    CSS_PSEUDO_ELEMENT_IS_FLEX_OR_GRID_ITEM)
+CSS_PSEUDO_ELEMENT(before, ":before", CSS_PSEUDO_ELEMENT_IS_CSS2 |
+                                      CSS_PSEUDO_ELEMENT_IS_FLEX_OR_GRID_ITEM)
 
 CSS_PSEUDO_ELEMENT(backdrop, ":backdrop", 0)
 
 CSS_PSEUDO_ELEMENT(cue, ":cue", CSS_PSEUDO_ELEMENT_IS_JS_CREATED_NAC |
                                 CSS_PSEUDO_ELEMENT_SUPPORTS_STYLE_ATTRIBUTE)
 
 CSS_PSEUDO_ELEMENT(firstLetter, ":first-letter",
                    CSS_PSEUDO_ELEMENT_IS_CSS2 |
--- a/layout/style/nsCSSPseudoElements.h
+++ b/layout/style/nsCSSPseudoElements.h
@@ -35,16 +35,19 @@
 // http://dev.w3.org/csswg/selectors4/#pseudo-elements.
 #define CSS_PSEUDO_ELEMENT_SUPPORTS_USER_ACTION_STATE  (1<<3)
 // Is content prevented from parsing selectors containing this pseudo-element?
 #define CSS_PSEUDO_ELEMENT_UA_SHEET_ONLY               (1<<4)
 // Can we use the ChromeOnly document.createElement(..., { pseudo: "::foo" })
 // API for creating pseudo-implementing native anonymous content in JS with this
 // pseudo-element?
 #define CSS_PSEUDO_ELEMENT_IS_JS_CREATED_NAC           (1<<5)
+// Does this pseudo-element act like an item for containers (such as flex and
+// grid containers) and thus needs parent display-based style fixup?
+#define CSS_PSEUDO_ELEMENT_IS_FLEX_OR_GRID_ITEM        (1<<6)
 
 namespace mozilla {
 
 // The total count of CSSPseudoElement is less than 256,
 // so use uint8_t as its underlying type.
 typedef uint8_t CSSPseudoElementTypeBase;
 enum class CSSPseudoElementType : CSSPseudoElementTypeBase {
   // If the actual pseudo-elements stop being first here, change
@@ -107,16 +110,22 @@ public:
 
   static bool PseudoElementSupportsUserActionState(const Type aType);
 
   static bool PseudoElementIsJSCreatedNAC(Type aType)
   {
     return PseudoElementHasFlags(aType, CSS_PSEUDO_ELEMENT_IS_JS_CREATED_NAC);
   }
 
+  static bool PseudoElementIsFlexOrGridItem(const Type aType)
+  {
+    return PseudoElementHasFlags(aType,
+                                 CSS_PSEUDO_ELEMENT_IS_FLEX_OR_GRID_ITEM);
+  }
+
   static bool IsEnabled(Type aType, EnabledState aEnabledState)
   {
     return !PseudoElementHasFlags(aType, CSS_PSEUDO_ELEMENT_UA_SHEET_ONLY) ||
            (aEnabledState & EnabledState::eInUASheets);
   }
 
 private:
   // Does the given pseudo-element have all of the flags given?
--- a/layout/style/nsStyleSet.cpp
+++ b/layout/style/nsStyleSet.cpp
@@ -1702,16 +1702,33 @@ nsStyleSet::RuleNodeWithReplacement(Elem
 
   NS_ASSERTION(rulesIndex == 0,
                "rules are in incorrect cascading order, "
                "which means we replaced them incorrectly");
 
   return ruleWalker.CurrentNode();
 }
 
+static bool
+SkipsParentDisplayBasedStyleFixup(nsStyleContext* aStyleContext)
+{
+  CSSPseudoElementType type = aStyleContext->GetPseudoType();
+  switch (type) {
+    case CSSPseudoElementType::InheritingAnonBox:
+       return nsCSSAnonBoxes::AnonBoxSkipsParentDisplayBasedStyleFixup(
+                aStyleContext->GetPseudo());
+    case CSSPseudoElementType::NonInheritingAnonBox:
+       return true;
+    case CSSPseudoElementType::NotPseudo:
+       return false;
+    default:
+       return !nsCSSPseudoElements::PseudoElementIsFlexOrGridItem(type);
+  }
+}
+
 already_AddRefed<nsStyleContext>
 nsStyleSet::ResolveStyleWithReplacement(Element* aElement,
                                         Element* aPseudoElement,
                                         nsStyleContext* aNewParentContext,
                                         nsStyleContext* aOldStyleContext,
                                         nsRestyleHint aReplacements,
                                         uint32_t aFlags)
 {
@@ -1772,21 +1789,23 @@ nsStyleSet::ResolveStyleWithReplacement(
                    !styleFrame ||
                    styleFrame->StyleContext()->GetPseudoType() ==
                      CSSPseudoElementType::NotPseudo,
                    "aElement should be the element and not the pseudo-element");
     }
 #endif
   }
 
-  if (aElement && aElement->IsRootOfAnonymousSubtree()) {
+  if ((aElement && aElement->IsRootOfAnonymousSubtree()) ||
+      SkipsParentDisplayBasedStyleFixup(aOldStyleContext)) {
     // For anonymous subtree roots, don't tweak "display" value based on whether
     // or not the parent is styled as a flex/grid container. (If the parent
     // has anonymous-subtree kids, then we know it's not actually going to get
-    // a flex/grid container frame, anyway.)
+    // a flex/grid container frame, anyway.)  Same for certain anonymous boxes
+    // and most pseudos.
     flags |= eSkipParentDisplayBasedStyleFixup;
   }
 
   return GetContext(aNewParentContext, ruleNode, visitedRuleNode,
                     aOldStyleContext->GetPseudo(), pseudoType,
                     elementForAnimation, flags);
 }
 
@@ -1948,21 +1967,21 @@ nsStyleSet::ResolvePseudoElementStyleInt
   // For pseudos, |data.IsLink()| being true means that
   // our parent node is a link.
   uint32_t flags = eNoFlags;
   if (aType == CSSPseudoElementType::before ||
       aType == CSSPseudoElementType::after) {
     if (aAnimationFlag == eWithAnimation) {
       flags |= eDoAnimation;
     }
-  } else {
-    // Flex and grid containers don't expect to have any pseudo-element children
-    // aside from ::before and ::after.  So if we have such a child, we're not
-    // actually in a flex/grid container, and we should skip flex/grid item
-    // style fixup.
+  }
+
+  if (!nsCSSPseudoElements::PseudoElementIsFlexOrGridItem(aType)) {
+    // Only pseudo-elements that act as items in flex and grid containers
+    // have parent display-based style fixup.
     flags |= eSkipParentDisplayBasedStyleFixup;
   }
 
   return GetContext(aParentContext, ruleNode, visitedRuleNode,
                     nsCSSPseudoElements::GetPseudoAtom(aType), aType,
                     aParentElement, flags);
 }
 
@@ -2047,21 +2066,21 @@ nsStyleSet::ProbePseudoElementStyle(Elem
   }
 
   // For pseudos, |data.IsLink()| being true means that
   // our parent node is a link.
   uint32_t flags = eNoFlags;
   if (aType == CSSPseudoElementType::before ||
       aType == CSSPseudoElementType::after) {
     flags |= eDoAnimation;
-  } else {
-    // Flex and grid containers don't expect to have any pseudo-element children
-    // aside from ::before and ::after.  So if we have such a child, we're not
-    // actually in a flex/grid container, and we should skip flex/grid item
-    // style fixup.
+  }
+
+  if (!nsCSSPseudoElements::PseudoElementIsFlexOrGridItem(aType)) {
+    // Only pseudo-elements that act as items in flex and grid containers
+    // have parent display-based style fixup.
     flags |= eSkipParentDisplayBasedStyleFixup;
   }
 
   RefPtr<nsStyleContext> result =
     GetContext(aParentContext, ruleNode, visitedRuleNode,
                pseudoTag, aType,
                aParentElement, flags);
 
@@ -2506,23 +2525,22 @@ nsStyleSet::ReparentStyleContext(nsStyle
 
   if (pseudoType == CSSPseudoElementType::NotPseudo ||
       pseudoType == CSSPseudoElementType::before ||
       pseudoType == CSSPseudoElementType::after) {
     flags |= eDoAnimation;
   }
 
   if ((aElement && aElement->IsRootOfAnonymousSubtree()) ||
-      (aStyleContext->IsAnonBox() &&
-       nsCSSAnonBoxes::AnonBoxSkipsParentDisplayBasedStyleFixup(
-         aStyleContext->GetPseudo()))) {
+      SkipsParentDisplayBasedStyleFixup(aStyleContext)) {
     // For anonymous subtree roots, don't tweak "display" value based on whether
     // or not the parent is styled as a flex/grid container. (If the parent
     // has anonymous-subtree kids, then we know it's not actually going to get
-    // a flex/grid container frame, anyway.)  Same for certain anonymous boxes.
+    // a flex/grid container frame, anyway.)  Same for certain anonymous boxes
+    // and most pseudos.
     flags |= eSkipParentDisplayBasedStyleFixup;
   }
 
   return GetContext(aNewParentContext, ruleNode, visitedRuleNode,
                     pseudoTag, pseudoType,
                     aElement, flags);
 }