Bug 1292930: stylo: Make change hint processing more straight-forward. draft
authorEmilio Cobos Álvarez <ecoal95@gmail.com>
Sat, 06 Aug 2016 14:18:12 -0700
changeset 397594 1068886e5a559d3d1b8029c59f80da79fa8b5613
parent 397593 c44d8a7e52854ff0364ab05537d8afb6369e296b
child 527491 9b5691ca8ee50cb0798c5021d930ce228e2eab2f
push id25337
push userbmo:ealvarez@mozilla.com
push dateSat, 06 Aug 2016 22:18:29 +0000
bugs1292930
milestone51.0a1
Bug 1292930: stylo: Make change hint processing more straight-forward. MozReview-Commit-ID: 4ZcCMKSc6Tv
layout/base/RestyleManagerBase.cpp
layout/base/ServoRestyleManager.cpp
--- a/layout/base/RestyleManagerBase.cpp
+++ b/layout/base/RestyleManagerBase.cpp
@@ -213,17 +213,16 @@ RestyleManagerBase::PostRestyleEventInte
   }
 
   // Unconditionally flag our document as needing a flush.  The other
   // option here would be a dedicated boolean to track whether we need
   // to do so (set here and unset in ProcessPendingRestyles).
   presShell->GetDocument()->SetNeedStyleFlush();
 }
 
-
 /**
  * Frame construction helpers follow.
  */
 #ifdef DEBUG
 static bool gInApplyRenderingChangeToTree = false;
 #endif
 
 #ifdef DEBUG
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -70,71 +70,89 @@ ServoRestyleManager::RecreateStyleContex
   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.
     return;
   }
 
   if (aContent->IsDirtyForServo()) {
-    nsChangeHint changeHint;
-    if (primaryFrame) {
-      changeHint = primaryFrame->StyleContext()->ConsumeStoredChangeHint();
-    } else {
-      // TODO: Use the frame constructor's UndisplayedNodeMap to store the old
-      // style contexts, and thus the change hints. That way we can in most
-      // cases avoid generating ReconstructFrame and push it to the list just to
-      // notice at frame construction that it doesn't need a frame.
-      changeHint = nsChangeHint_ReconstructFrame;
+    RefPtr<ServoComputedValues> computedValues =
+      dont_AddRef(Servo_GetComputedValues(aContent));
+    MOZ_ASSERT(computedValues);
+
+    // NB: Change hint processing only applies to elements, at least until we
+    // support display: contents.
+    if (aContent->IsElement()) {
+      nsChangeHint changeHint = nsChangeHint(0);
+      Element* element = aContent->AsElement();
+
+      // Add an explicit change hint if appropriate.
+      ServoElementSnapshot* snapshot;
+      if (mModifiedElements.Get(element, &snapshot)) {
+        changeHint |= snapshot->ExplicitChangeHint();
+      }
+
+      // Add the stored change hint if there's a frame. If there isn't a frame,
+      // generate a ReconstructFrame change hint except if the new display value
+      // (that we can look at in the content node) is not none.
+      if (primaryFrame) {
+        changeHint |= primaryFrame->StyleContext()->ConsumeStoredChangeHint();
+      } else {
+        const nsStyleDisplay* currentDisplay =
+          Servo_GetStyleDisplay(computedValues);
+        if (currentDisplay->mDisplay != NS_STYLE_DISPLAY_NONE) {
+          changeHint |= nsChangeHint_ReconstructFrame;
+        }
+      }
+
+      // Add the new change hint to the list of elements to process if
+      // we need to do any work.
+      if (changeHint) {
+        aChangeListToProcess.AppendChange(primaryFrame, element, changeHint);
+      }
     }
 
-    // NB: The change list only expects elements.
-    if (changeHint && aContent->IsElement()) {
-      aChangeListToProcess.AppendChange(primaryFrame, aContent, changeHint);
-    }
-
+    // The frame reconstruction step (if needed) will ask for the descendants'
+    // style correctly. If not needed, we're done too.
     if (!primaryFrame) {
-      // The frame reconstruction step will ask for the descendant's style
-      // correctly.
+      aContent->UnsetFlags(NODE_IS_DIRTY_FOR_SERVO);
       return;
     }
 
-    // Even if we don't have a change hint, we still need to swap style contexts
-    // so our new style is updated properly.
-    RefPtr<ServoComputedValues> computedValues =
-      dont_AddRef(Servo_GetComputedValues(aContent));
+    // 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.
+    RefPtr<nsStyleContext> oldStyleContext = primaryFrame->StyleContext();
+    MOZ_ASSERT(oldStyleContext);
 
     // TODO: Figure out what pseudos does this content have, and do the proper
     // thing with them.
     RefPtr<nsStyleContext> newContext =
-      aStyleSet->GetContext(computedValues.forget(),
-                            aParentContext,
-                            nullptr,
+      aStyleSet->GetContext(computedValues.forget(), aParentContext, nullptr,
                             CSSPseudoElementType::NotPseudo);
 
-    RefPtr<nsStyleContext> oldStyleContext = primaryFrame->StyleContext();
-    MOZ_ASSERT(oldStyleContext);
-
-    // XXX This could not always work as expected there are kinds of content
+    // XXX This could not always work as expected: there are kinds of content
     // with the first split and the last sharing style, but others not. We
     // should handle those properly.
     for (nsIFrame* f = primaryFrame; f;
          f = GetNextContinuationWithSameStyle(f, oldStyleContext)) {
       f->SetStyleContext(newContext);
     }
 
     // TODO: There are other continuations we still haven't restyled, mostly
     // pseudo-elements. We have to deal with those, and with anonymous boxes.
     aContent->UnsetFlags(NODE_IS_DIRTY_FOR_SERVO);
   }
 
   if (aContent->HasDirtyDescendantsForServo()) {
     MOZ_ASSERT(primaryFrame,
                "Frame construction should be scheduled, and it takes the "
-               "correct style for the children");
+               "correct style for the children, so no need to be here.");
     FlattenedChildIterator it(aContent);
     for (nsIContent* n = it.GetNextChild(); n; n = it.GetNextChild()) {
       RecreateStyleContexts(n, primaryFrame->StyleContext(),
                             aStyleSet, aChangeListToProcess);
     }
     aContent->UnsetFlags(NODE_HAS_DIRTY_DESCENDANTS_FOR_SERVO);
   }
 }