Bug 1288873: Don't propagate the IS_DIRTY flag down the whole tree, just make it
authorEmilio Cobos Álvarez <ecoal95@gmail.com>
Fri, 22 Jul 2016 15:19:30 -0700
changeset 349023 b54cf953d8c40fbe82ecef271f5f6e0ad52abaa5
parent 349022 5ef9e319b525b64c35e35fe640a08455b89fdac1
child 349024 872ec4f743e5e1c8f40089fe5f9b37ba5f4f5c20
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1288873
milestone50.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 1288873: Don't propagate the IS_DIRTY flag down the whole tree, just make it imply that all descendants are dirty. r=heycam 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());
 }