Bug 1288873: Don't propagate the IS_DIRTY flag down the whole tree, just make it draft
authorEmilio Cobos Álvarez <ecoal95@gmail.com>
Fri, 22 Jul 2016 15:19:30 -0700
changeset 393050 9827d6167d6164dbc6227b598fdf6533c89816ba
parent 392094 36bd6dc09186346a0e99c638c433f7d438877d8e
child 393051 934bea8d036642e1f124feb33f8147fd31589190
push id24197
push userbmo:ealvarez@mozilla.com
push dateTue, 26 Jul 2016 23:09:19 +0000
bugs1288873
milestone50.0a1
Bug 1288873: Don't propagate the IS_DIRTY flag down the whole tree, just make it imply that all descendants are dirty. We're probably going to be a lot more smarter than this in the future, but since there is work in progress to figure out how should we avoid running selector-matching on the elements, this helps a lot with perf in the meantime. MozReview-Commit-ID: CEb15JwHAdH
layout/base/ServoRestyleManager.cpp
layout/base/ServoRestyleManager.h
layout/style/ServoStyleSet.cpp
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -11,43 +11,16 @@ using namespace mozilla::dom;
 
 namespace mozilla {
 
 ServoRestyleManager::ServoRestyleManager(nsPresContext* aPresContext)
   : RestyleManagerBase(aPresContext)
 {
 }
 
-/* static */ void
-ServoRestyleManager::DirtyTree(nsIContent* aContent, bool aIncludingRoot)
-{
-  if (aIncludingRoot) {
-    // XXX: This can in theory leave nodes not dirty, but in practice this is not
-    // a problem, at least for now, since right now element dirty implies
-    // descendants dirty. Remove this early return if this ever changes.
-    if (aContent->IsDirtyForServo()) {
-      return;
-    }
-
-    aContent->SetIsDirtyForServo();
-  }
-
-  FlattenedChildIterator it(aContent);
-
-  nsIContent* n = it.GetNextChild();
-  bool hadChildren = bool(n);
-  for (; n; n = it.GetNextChild()) {
-    DirtyTree(n, true);
-  }
-
-  if (hadChildren) {
-    aContent->SetHasDirtyDescendantsForServo();
-  }
-}
-
 void
 ServoRestyleManager::PostRestyleEvent(Element* aElement,
                                       nsRestyleHint aRestyleHint,
                                       nsChangeHint aMinChangeHint)
 {
   if (MOZ_UNLIKELY(IsDisconnected()) ||
       MOZ_UNLIKELY(PresContext()->PresShell()->IsDestroying())) {
     return;
@@ -146,37 +119,52 @@ MarkParentsAsHavingDirtyDescendants(Elem
     if (cur->HasDirtyDescendantsForServo()) {
       break;
     }
 
     cur->SetHasDirtyDescendantsForServo();
   }
 }
 
+static void
+MarkChildrenAsDirtyForServo(nsIContent* aContent)
+{
+  FlattenedChildIterator it(aContent);
+
+  nsIContent* n = it.GetNextChild();
+  bool hadChildren = bool(n);
+  for (; n; n = it.GetNextChild()) {
+    n->SetIsDirtyForServo();
+  }
+
+  if (hadChildren) {
+    aContent->SetHasDirtyDescendantsForServo();
+  }
+}
+
 void
 ServoRestyleManager::NoteRestyleHint(Element* aElement, nsRestyleHint aHint)
 {
   if (aHint & eRestyle_Self) {
     aElement->SetIsDirtyForServo();
     MarkParentsAsHavingDirtyDescendants(aElement);
-    // NB: Using Servo's style system marking the subtree as dirty is necessary
-    // so we inherit correctly the style structs.
-    aHint |= eRestyle_Subtree;
-  }
-
-  if (aHint & eRestyle_Subtree) {
-    DirtyTree(aElement, /* aIncludingRoot = */ false);
+    // NB: For Servo, at least for now, restyling and running selector-matching
+    // against the subtree is necessary as part of restyling the element, so
+    // processing eRestyle_Self will perform at least as much work as
+    // eRestyle_Subtree.
+  } else if (aHint & eRestyle_Subtree) {
+    MarkChildrenAsDirtyForServo(aElement);
     MarkParentsAsHavingDirtyDescendants(aElement);
   }
 
   if (aHint & eRestyle_LaterSiblings) {
     for (nsINode* cur = aElement->GetNextSibling(); cur;
          cur = cur->GetNextSibling()) {
       if (cur->IsContent()) {
-        DirtyTree(cur->AsContent(), /* aIncludingRoot = */ true);
+        cur->SetIsDirtyForServo();
       }
     }
   }
 
   // TODO: Handle all other nsRestyleHint values.
   if (aHint & ~(eRestyle_Self | eRestyle_Subtree | eRestyle_LaterSiblings)) {
     NS_ERROR("stylo: Unhandled restyle hint");
   }
--- a/layout/base/ServoRestyleManager.h
+++ b/layout/base/ServoRestyleManager.h
@@ -89,22 +89,16 @@ private:
    * can access to the GetContext method without making it available to everyone
    * else.
    */
   static void RecreateStyleContexts(nsIContent* aContent,
                                     nsStyleContext* aParentContext,
                                     ServoStyleSet* aStyleSet);
 
   /**
-   * Propagates the IS_DIRTY flag down to the tree, setting
-   * HAS_DIRTY_DESCENDANTS appropriately.
-   */
-  static void DirtyTree(nsIContent* aContent, bool aIncludingRoot = true);
-
-  /**
    * Marks the tree with the appropriate dirty flags for the given restyle hint.
    */
   static void NoteRestyleHint(Element* aElement, nsRestyleHint aRestyleHint);
 
   inline ServoStyleSet* StyleSet() const
   {
     MOZ_ASSERT(PresContext()->StyleSet()->IsServo(),
                "ServoRestyleManager should only be used with a Servo-flavored "
--- a/layout/style/ServoStyleSet.cpp
+++ b/layout/style/ServoStyleSet.cpp
@@ -388,13 +388,14 @@ ServoStyleSet::ComputeRestyleHint(dom::E
   return Servo_ComputeRestyleHint(aElement, aSnapshot, mRawSet.get());
 }
 
 void
 ServoStyleSet::RestyleSubtree(nsINode* aNode, bool aForce)
 {
   if (aForce) {
     MOZ_ASSERT(aNode->IsContent());
-    ServoRestyleManager::DirtyTree(aNode->AsContent());
+    aNode->SetIsDirtyForServo();
   }
 
+  MOZ_ASSERT(aNode->IsDirtyForServo() || aNode->HasDirtyDescendantsForServo());
   Servo_RestyleSubtree(aNode, mRawSet.get());
 }