Bug 996796 patch 9 - Make nsStyleSet::ResolveStyleWithReplacement handle changing between having and not having animation or transition rules, make it set IsImportantRule on rule nodes correctly, and merge the bogus ResolveStyleForRules into it. r=heycam
authorL. David Baron <dbaron@dbaron.org>
Sat, 02 Aug 2014 19:37:44 -0700
changeset 197590 4a395d400f602f4c3bcb32603af4374ee2b5346d
parent 197589 d9db9020d57ace77058e987ad5e8bef422da6b50
child 197591 720eed827027169056170ddf8ecf43be85b02fe6
push id27249
push userryanvm@gmail.com
push dateMon, 04 Aug 2014 20:14:35 +0000
treeherdermozilla-central@7f81be7db528 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs996796
milestone34.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 996796 patch 9 - Make nsStyleSet::ResolveStyleWithReplacement handle changing between having and not having animation or transition rules, make it set IsImportantRule on rule nodes correctly, and merge the bogus ResolveStyleForRules into it. r=heycam ResolveStyleForRules had various problems: it failed to set importance correctly and really only handled replacing a path in the rule tree since it didn't handle creating important rules. (Possibly more.)
layout/style/nsStyleSet.cpp
layout/style/nsStyleSet.h
--- a/layout/style/nsStyleSet.cpp
+++ b/layout/style/nsStyleSet.cpp
@@ -1250,40 +1250,16 @@ nsStyleSet::ResolveStyleForRules(nsStyle
   }
 
   return GetContext(aParentContext, ruleWalker.CurrentNode(), nullptr,
                     nullptr, nsCSSPseudoElements::ePseudo_NotPseudoElement,
                     nullptr, eNoFlags);
 }
 
 already_AddRefed<nsStyleContext>
-nsStyleSet::ResolveStyleForRules(nsStyleContext* aParentContext,
-                                 nsStyleContext* aOldStyle,
-                                 const nsTArray<RuleAndLevel>& aRules)
-{
-  nsRuleWalker ruleWalker(mRuleTree, mAuthorStyleDisabled);
-  for (int32_t i = aRules.Length() - 1; i >= 0; --i) {
-    ruleWalker.SetLevel(aRules[i].mLevel, false, false);
-    ruleWalker.ForwardOnPossiblyCSSRule(aRules[i].mRule);
-  }
-
-  uint32_t flags = eNoFlags;
-  if (aOldStyle->IsLinkContext()) {
-    flags |= eIsLink;
-  }
-  if (aOldStyle->RelevantLinkVisited()) {
-    flags |= eIsVisitedLink;
-  }
-
-  return GetContext(aParentContext, ruleWalker.CurrentNode(), nullptr,
-                    nullptr, nsCSSPseudoElements::ePseudo_NotPseudoElement,
-                    nullptr, flags);
-}
-
-already_AddRefed<nsStyleContext>
 nsStyleSet::ResolveStyleByAddingRules(nsStyleContext* aBaseContext,
                                       const nsCOMArray<nsIStyleRule> &aRules)
 {
   NS_ENSURE_FALSE(mInShutdown, nullptr);
 
   nsRuleWalker ruleWalker(mRuleTree, mAuthorStyleDisabled);
   ruleWalker.SetCurrentNode(aBaseContext->RuleNode());
   // FIXME: Perhaps this should be passed in, but it probably doesn't
@@ -1316,86 +1292,147 @@ nsStyleSet::ResolveStyleByAddingRules(ns
     }
   }
   return GetContext(aBaseContext->GetParent(), ruleNode, visitedRuleNode,
                     aBaseContext->GetPseudo(),
                     aBaseContext->GetPseudoType(),
                     nullptr, flags);
 }
 
+struct RuleNodeInfo {
+  nsIStyleRule* mRule;
+  uint8_t mLevel;
+  bool mIsImportant;
+};
+
+struct CascadeLevel {
+  uint8_t mLevel;
+  bool mIsImportant;
+  nsRestyleHint mLevelReplacementHint;
+};
+
+static const CascadeLevel gCascadeLevels[] = {
+  { nsStyleSet::eAgentSheet,      false, nsRestyleHint(0) },
+  { nsStyleSet::eUserSheet,       false, nsRestyleHint(0) },
+  { nsStyleSet::ePresHintSheet,   false, nsRestyleHint(0) },
+  { nsStyleSet::eDocSheet,        false, nsRestyleHint(0) },
+  { nsStyleSet::eScopedDocSheet,  false, nsRestyleHint(0) },
+  { nsStyleSet::eStyleAttrSheet,  false, nsRestyleHint(0) },
+  { nsStyleSet::eOverrideSheet,   false, nsRestyleHint(0) },
+  { nsStyleSet::eAnimationSheet,  false, eRestyle_CSSAnimations },
+  { nsStyleSet::eScopedDocSheet,  true,  nsRestyleHint(0) },
+  { nsStyleSet::eDocSheet,        true,  nsRestyleHint(0) },
+  { nsStyleSet::eStyleAttrSheet,  true,  nsRestyleHint(0) },
+  { nsStyleSet::eOverrideSheet,   true,  nsRestyleHint(0) },
+  { nsStyleSet::eUserSheet,       true,  nsRestyleHint(0) },
+  { nsStyleSet::eAgentSheet,      true,  nsRestyleHint(0) },
+  { nsStyleSet::eTransitionSheet, false, eRestyle_CSSTransitions },
+};
+
 already_AddRefed<nsStyleContext>
 nsStyleSet::ResolveStyleWithReplacement(Element* aElement,
                                         nsStyleContext* aNewParentContext,
                                         nsStyleContext* aOldStyleContext,
                                         nsRestyleHint aReplacements)
 {
   NS_ABORT_IF_FALSE(!(aReplacements & ~(eRestyle_CSSTransitions |
                                         eRestyle_CSSAnimations)),
                     // FIXME: Once bug 931668 lands we'll have a better
                     // way to print these.
                     nsPrintfCString("unexpected replacement bits 0x%lX",
                                     uint32_t(aReplacements)).get());
 
-  nsRuleNode* ruleNode = aOldStyleContext->RuleNode();
-  nsTArray<nsStyleSet::RuleAndLevel> rules;
-  do {
-    if (ruleNode->IsRoot()) {
-      break;
+  nsTArray<RuleNodeInfo> rules;
+  for (nsRuleNode* ruleNode = aOldStyleContext->RuleNode(); !ruleNode->IsRoot();
+       ruleNode = ruleNode->GetParent()) {
+    RuleNodeInfo* curRule = rules.AppendElement();
+    curRule->mRule = ruleNode->GetRule();
+    curRule->mLevel = ruleNode->GetLevel();
+    curRule->mIsImportant = ruleNode->IsImportantRule();
+  }
+
+  nsRuleWalker ruleWalker(mRuleTree, mAuthorStyleDisabled);
+  auto rulesIndex = rules.Length();
+
+  for (const CascadeLevel *level = gCascadeLevels,
+                       *levelEnd = ArrayEnd(gCascadeLevels);
+       level != levelEnd; ++level) {
+    ruleWalker.SetLevel(level->mLevel, level->mIsImportant, false);
+
+    bool doReplace = level->mLevelReplacementHint & aReplacements;
+    if (doReplace) {
+      switch (level->mLevelReplacementHint) {
+        case eRestyle_CSSAnimations: {
+          nsAnimationManager* animationManager =
+            PresContext()->AnimationManager();
+          ElementAnimationCollection* collection = animationManager->GetElementAnimations(
+            aElement, aOldStyleContext->GetPseudoType(), false);
+
+          if (collection) {
+            animationManager->UpdateStyleAndEvents(
+              collection, PresContext()->RefreshDriver()->MostRecentRefresh(),
+              EnsureStyleRule_IsNotThrottled);
+            if (collection->mStyleRule) {
+              ruleWalker.ForwardOnPossiblyCSSRule(collection->mStyleRule);
+            }
+          }
+          break;
+        }
+        case eRestyle_CSSTransitions: {
+          nsPresContext* presContext = PresContext();
+          ElementAnimationCollection* collection =
+            presContext->TransitionManager()->GetElementTransitions(
+              aElement,
+              aOldStyleContext->GetPseudoType(),
+              false);
+
+          if (collection) {
+            collection->EnsureStyleRuleFor(
+              presContext->RefreshDriver()->MostRecentRefresh(),
+              EnsureStyleRule_IsNotThrottled);
+            if (collection->mStyleRule) {
+              ruleWalker.ForwardOnPossiblyCSSRule(collection->mStyleRule);
+            }
+          }
+          break;
+        }
+        default:
+          break;
+      }
     }
 
-    nsStyleSet::RuleAndLevel curRule;
-    curRule.mLevel = ruleNode->GetLevel();
-    curRule.mRule = ruleNode->GetRule();
-
-    // FIXME: This will eventually need to handle adding a rule where we
-    // don't currently have one!
+    while (rulesIndex != 0) {
+      --rulesIndex;
+      const RuleNodeInfo& ruleInfo = rules[rulesIndex];
 
-    switch (curRule.mLevel) {
-    case nsStyleSet::eAnimationSheet:
-      if (aReplacements & eRestyle_CSSAnimations) {
-        nsAnimationManager* animationManager = PresContext()->AnimationManager();
-        ElementAnimationCollection* collection = animationManager->GetElementAnimations(
-          aElement, aOldStyleContext->GetPseudoType(), false);
-        NS_ASSERTION(collection,
-          "Rule has level eAnimationSheet without animation on manager");
-
-        animationManager->UpdateStyleAndEvents(
-          collection, PresContext()->RefreshDriver()->MostRecentRefresh(),
-          EnsureStyleRule_IsNotThrottled);
-        curRule.mRule = collection->mStyleRule;
+      if (ruleInfo.mLevel != level->mLevel ||
+          ruleInfo.mIsImportant != level->mIsImportant) {
+        ++rulesIndex;
+        break;
       }
-      break;
-    case nsStyleSet::eTransitionSheet:
-      if (aReplacements & eRestyle_CSSTransitions) {
-        nsPresContext* presContext = PresContext();
-        ElementAnimationCollection* collection =
-          presContext->TransitionManager()->GetElementTransitions(
-            aElement,
-            aOldStyleContext->GetPseudoType(),
-            false);
-        NS_ASSERTION(collection,
-          "Rule has level eTransitionSheet without transition on manager");
 
-        collection->EnsureStyleRuleFor(
-          presContext->RefreshDriver()->MostRecentRefresh(),
-          EnsureStyleRule_IsNotThrottled);
-        curRule.mRule = collection->mStyleRule;
+      if (!doReplace) {
+        ruleWalker.ForwardOnPossiblyCSSRule(ruleInfo.mRule);
       }
-      break;
-    default:
-      break;
     }
-
-    if (curRule.mRule) {
-      rules.AppendElement(curRule);
-    }
-  } while ((ruleNode = ruleNode->GetParent()));
+  }
 
   // FIXME: Does this handle visited contexts correctly???
-  return ResolveStyleForRules(aNewParentContext, aOldStyleContext, rules);
+
+  uint32_t flags = eNoFlags;
+  if (aOldStyleContext->IsLinkContext()) {
+    flags |= eIsLink;
+  }
+  if (aOldStyleContext->RelevantLinkVisited()) {
+    flags |= eIsVisitedLink;
+  }
+
+  return GetContext(aNewParentContext, ruleWalker.CurrentNode(), nullptr,
+                    nullptr, nsCSSPseudoElements::ePseudo_NotPseudoElement,
+                    nullptr, flags);
 }
 
 
 already_AddRefed<nsStyleContext>
 nsStyleSet::ResolveStyleForNonElement(nsStyleContext* aParentContext)
 {
   return GetContext(aParentContext, mRuleTree, nullptr,
                     nsCSSAnonBoxes::mozNonElement,
--- a/layout/style/nsStyleSet.h
+++ b/layout/style/nsStyleSet.h
@@ -107,31 +107,16 @@ class nsStyleSet
                   TreeMatchContext& aTreeMatchContext);
 
   // Get a style context (with the given parent) for the
   // sequence of style rules in the |aRules| array.
   already_AddRefed<nsStyleContext>
   ResolveStyleForRules(nsStyleContext* aParentContext,
                        const nsTArray< nsCOMPtr<nsIStyleRule> > &aRules);
 
-  // used in ResolveStyleForRules below
-  struct RuleAndLevel
-  {
-    nsIStyleRule* mRule;
-    uint8_t mLevel;
-  };
-
-  // Get a new style context for aElement for the rules in aRules
-  // aRules is an array of rules and their levels in reverse order,
-  // that is from the leaf-most to the root-most rule in the rule tree.
-  already_AddRefed<nsStyleContext>
-  ResolveStyleForRules(nsStyleContext* aParentContext,
-                       nsStyleContext* aOldStyle,
-                       const nsTArray<RuleAndLevel>& aRules);
-
   // Get a style context that represents aBaseContext, but as though
   // it additionally matched the rules in the aRules array (in that
   // order, as more specific than any other rules).
   already_AddRefed<nsStyleContext>
   ResolveStyleByAddingRules(nsStyleContext* aBaseContext,
                             const nsCOMArray<nsIStyleRule> &aRules);
 
   // Resolve style by making replacements in the list of style rules as