Bug 1315894 - Clear dirty bits on entire subtree when stopping in RecreateStyleContexts due to no frame or ReconstructFrame hint. r=emilio
authorCameron McCormack <cam@mcc.id.au>
Wed, 09 Nov 2016 14:25:58 +0800
changeset 321982 fc55188f14b6819fbf34a425d7af2a2c73ded730
parent 321981 b913f0ce1e3e3bc94ce444b286768aed15efbfd4
child 321983 25f6a27f309d85438bd3729d392291a038ad248f
push id21
push usermaklebus@msu.edu
push dateThu, 01 Dec 2016 06:22:08 +0000
reviewersemilio
bugs1315894
milestone52.0a1
Bug 1315894 - Clear dirty bits on entire subtree when stopping in RecreateStyleContexts due to no frame or ReconstructFrame hint. r=emilio MozReview-Commit-ID: 5ch5gtOn0Zr
layout/base/ServoRestyleManager.cpp
layout/style/crashtests/1315894-1.html
layout/style/crashtests/crashtests.list
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -68,28 +68,44 @@ ServoRestyleManager::RebuildAllStyleData
 
 void
 ServoRestyleManager::PostRebuildAllStyleDataEvent(nsChangeHint aExtraHint,
                                                   nsRestyleHint aRestyleHint)
 {
   NS_WARNING("stylo: ServoRestyleManager::PostRebuildAllStyleDataEvent not implemented");
 }
 
+static void
+MarkSelfAndDescendantsAsNotDirtyForServo(nsIContent* aContent)
+{
+  aContent->UnsetIsDirtyForServo();
+
+  if (aContent->HasDirtyDescendantsForServo()) {
+    aContent->UnsetHasDirtyDescendantsForServo();
+
+    StyleChildrenIterator it(aContent);
+    for (nsIContent* n = it.GetNextChild(); n; n = it.GetNextChild()) {
+      MarkSelfAndDescendantsAsNotDirtyForServo(n);
+    }
+  }
+}
+
 void
 ServoRestyleManager::RecreateStyleContexts(nsIContent* aContent,
                                            nsStyleContext* aParentContext,
                                            ServoStyleSet* aStyleSet,
                                            nsStyleChangeList& aChangeListToProcess)
 {
   MOZ_ASSERT(aContent->IsElement() || aContent->IsNodeOfType(nsINode::eTEXT));
 
   nsIFrame* primaryFrame = aContent->GetPrimaryFrame();
   if (!primaryFrame && !aContent->IsDirtyForServo()) {
     // This happens when, for example, a display: none child of a
     // HAS_DIRTY_DESCENDANTS content is reached as part of the traversal.
+    MarkSelfAndDescendantsAsNotDirtyForServo(aContent);
     return;
   }
 
   // Work on text before.
   if (!aContent->IsElement()) {
     if (primaryFrame) {
       RefPtr<nsStyleContext> oldStyleContext = primaryFrame->StyleContext();
       RefPtr<nsStyleContext> newContext =
@@ -140,17 +156,29 @@ ServoRestyleManager::RecreateStyleContex
     }
 
     // The frame reconstruction step (if needed) will ask for the descendants'
     // style correctly. If not needed, we're done too.
     //
     // Note that we must leave the old style on an existing frame that is
     // about to be reframed, since some frame constructor code wants to
     // inspect the old style to work out what to do.
-    if (!primaryFrame || (changeHint & nsChangeHint_ReconstructFrame)) {
+    if (changeHint & nsChangeHint_ReconstructFrame) {
+      // Since we might still have some dirty bits set on descendants,
+      // inconsistent with the clearing of HasDirtyDescendants we will do as
+      // we return from these recursive RecreateStyleContexts calls, we
+      // explicitly clear them here.  Otherwise we will trigger assertions
+      // when we soon process the frame reconstruction.
+      MarkSelfAndDescendantsAsNotDirtyForServo(element);
+      return;
+    }
+
+    // If there is no frame, and we didn't generate a ReconstructFrame change
+    // hint, then we don't need to do any more work.
+    if (!primaryFrame) {
       aContent->UnsetIsDirtyForServo();
       return;
     }
 
     // Hold the old style context alive, because it could become a dangling
     // pointer during the replacement. In practice it's not a huge deal (on
     // GetNextContinuationWithSameStyle the pointer is not dereferenced, only
     // compared), but better not playing with dangling pointers if not needed.
new file mode 100644
--- /dev/null
+++ b/layout/style/crashtests/1315894-1.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<div style="display: none"><span>x</span></div>
+<script>
+document.body.offsetWidth;
+document.querySelector("div").style.display = "inline";
+document.body.offsetWidth;
+document.querySelector("span").style.color = "blue";
+document.body.offsetWidth;
+</script>
--- a/layout/style/crashtests/crashtests.list
+++ b/layout/style/crashtests/crashtests.list
@@ -155,8 +155,9 @@ pref(dom.animations-api.core.enabled,tru
 load 1277908-2.html
 load 1282076-1.html
 pref(dom.animations-api.core.enabled,true) load 1282076-2.html
 pref(dom.animations-api.core.enabled,true) load 1290994-1.html
 pref(dom.animations-api.core.enabled,true) load 1290994-2.html
 pref(dom.animations-api.core.enabled,true) load 1290994-3.html
 load 1290994-4.html
 load 1314531.html
+load 1315894-1.html