Bug 1338921 - Handle lazy frame construction in the regular post-servo pass. r=emilio
authorBobby Holley <bobbyholley@gmail.com>
Mon, 27 Feb 2017 19:06:07 -0800
changeset 394889 4a2a58be8c6c54dbf85b90aa85e4d4281e72088f
parent 394888 a57ff8701dc2a1132e404cf4c56a571eb1713c6d
child 394890 a25b3225e35191dbd368195852b86da57cdcc7a5
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1338921
milestone54.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 1338921 - Handle lazy frame construction in the regular post-servo pass. r=emilio MozReview-Commit-ID: FSXKAiyZDzt
layout/base/RestyleManager.cpp
layout/base/ServoRestyleManager.cpp
layout/base/ServoRestyleManager.h
layout/base/nsStyleChangeList.cpp
layout/style/test/stylo-failures.md
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -1399,23 +1399,22 @@ RestyleManager::ProcessRestyledFrames(ns
     while (i < aChangeList.Length() &&
            aChangeList[i].mContent &&
            aChangeList[i].mContent->HasFlag(NODE_NEEDS_FRAME) &&
            (i == lazyRangeStart ||
             aChangeList[i - 1].mContent->GetNextSibling() == aChangeList[i].mContent))
     {
       MOZ_ASSERT(aChangeList[i].mHint & nsChangeHint_ReconstructFrame);
       MOZ_ASSERT(!aChangeList[i].mFrame);
-      MOZ_ASSERT(!aChangeList[i].mContent->GetPrimaryFrame());
       ++i;
     }
     if (i != lazyRangeStart) {
       nsIContent* start = aChangeList[lazyRangeStart].mContent;
       nsIContent* end = aChangeList[i-1].mContent->GetNextSibling();
-      nsIContent* container = start->GetFlattenedTreeParent();
+      nsIContent* container = start->GetParent();
       MOZ_ASSERT(container);
       if (!end) {
         frameConstructor->ContentAppended(container, start, false);
       } else {
         frameConstructor->ContentRangeInserted(container, start, end, nullptr, false);
       }
     }
     for (size_t j = lazyRangeStart; j < i; ++j) {
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -122,33 +122,46 @@ ClearRestyleStateFromSubtree(Element* aE
       if (n->IsElement()) {
         ClearRestyleStateFromSubtree(n->AsElement());
       }
     }
   }
 
   Unused << Servo_TakeChangeHint(aElement);
   aElement->UnsetHasDirtyDescendantsForServo();
+  aElement->UnsetFlags(NODE_DESCENDANTS_NEED_FRAMES);
 }
 
 void
-ServoRestyleManager::RecreateStyleContexts(Element* aElement,
-                                           nsStyleContext* aParentContext,
-                                           ServoStyleSet* aStyleSet,
-                                           nsStyleChangeList& aChangeListToProcess)
+ServoRestyleManager::ProcessPostTraversal(Element* aElement,
+                                          nsStyleContext* aParentContext,
+                                          ServoStyleSet* aStyleSet,
+                                          nsStyleChangeList& aChangeList)
 {
   nsIFrame* styleFrame = nsLayoutUtils::GetStyleFrame(aElement);
 
+  // Grab the change hint from Servo.
   nsChangeHint changeHint = Servo_TakeChangeHint(aElement);
+
+  // Handle lazy frame construction by posting a reconstruct for any lazily-
+  // constructed roots.
+  if (aElement->HasFlag(NODE_NEEDS_FRAME)) {
+    changeHint |= nsChangeHint_ReconstructFrame;
+    // The only time the primary frame is non-null is when image maps do hacky
+    // SetPrimaryFrame calls.
+    MOZ_ASSERT_IF(styleFrame, styleFrame->GetType() == nsGkAtoms::imageFrame);
+    styleFrame = nullptr;
+  }
+
   // Although we shouldn't generate non-ReconstructFrame hints for elements with
   // no frames, we can still get them here if they were explicitly posted by
   // PostRestyleEvent, such as a RepaintFrame hint when a :link changes to be
   // :visited.  Skip processing these hints if there is no frame.
   if ((styleFrame || (changeHint & nsChangeHint_ReconstructFrame)) && changeHint) {
-    aChangeListToProcess.AppendChange(styleFrame, aElement, changeHint);
+    aChangeList.AppendChange(styleFrame, aElement, changeHint);
   }
 
   // If our change hint is reconstruct, we delegate to the frame constructor,
   // which consumes the new style and expects the old style to be on the frame.
   //
   // XXXbholley: We should teach the frame constructor how to clear the dirty
   // descendants bit to avoid the traversal here.
   if (changeHint & nsChangeHint_ReconstructFrame) {
@@ -214,18 +227,17 @@ ServoRestyleManager::RecreateStyleContex
     }
 
     if (MOZ_UNLIKELY(displayContentsNode)) {
       MOZ_ASSERT(!styleFrame);
       displayContentsNode->mStyle = newContext;
     }
 
     if (styleFrame) {
-      styleFrame->UpdateStyleOfOwnedAnonBoxes(*aStyleSet, aChangeListToProcess,
-                                              changeHint);
+      styleFrame->UpdateStyleOfOwnedAnonBoxes(*aStyleSet, aChangeList, changeHint);
     }
 
     // Update pseudo-elements state if appropriate.
     const static CSSPseudoElementType pseudosToRestyle[] = {
       CSSPseudoElementType::before,
       CSSPseudoElementType::after,
     };
 
@@ -253,41 +265,52 @@ ServoRestyleManager::RecreateStyleContex
                        "How? This node is created at FC time!");
             n->GetPrimaryFrame()->SetStyleContext(childContext);
           }
         }
       }
     }
   }
 
-  bool traverseElementChildren = aElement->HasDirtyDescendantsForServo();
-  bool traverseTextChildren = recreateContext;
+  bool descendantsNeedFrames = aElement->HasFlag(NODE_DESCENDANTS_NEED_FRAMES);
+  bool traverseElementChildren =
+    aElement->HasDirtyDescendantsForServo() || descendantsNeedFrames;
+  bool traverseTextChildren = recreateContext || descendantsNeedFrames;
   if (traverseElementChildren || traverseTextChildren) {
     nsStyleContext* upToDateContext =
       recreateContext ? newContext : oldStyleContext;
 
     StyleChildrenIterator it(aElement);
     for (nsIContent* n = it.GetNextChild(); n; n = it.GetNextChild()) {
       if (traverseElementChildren && n->IsElement()) {
-        RecreateStyleContexts(n->AsElement(), upToDateContext,
-                              aStyleSet, aChangeListToProcess);
+        ProcessPostTraversal(n->AsElement(), upToDateContext,
+                             aStyleSet, aChangeList);
       } else if (traverseTextChildren && n->IsNodeOfType(nsINode::eTEXT)) {
-        RecreateStyleContextsForText(n, upToDateContext, aStyleSet);
+        ProcessPostTraversalForText(n, upToDateContext, aStyleSet, aChangeList);
       }
     }
   }
 
   aElement->UnsetHasDirtyDescendantsForServo();
+  aElement->UnsetFlags(NODE_DESCENDANTS_NEED_FRAMES);
 }
 
 void
-ServoRestyleManager::RecreateStyleContextsForText(nsIContent* aTextNode,
-                                                  nsStyleContext* aParentContext,
-                                                  ServoStyleSet* aStyleSet)
+ServoRestyleManager::ProcessPostTraversalForText(nsIContent* aTextNode,
+                                                 nsStyleContext* aParentContext,
+                                                 ServoStyleSet* aStyleSet,
+                                                 nsStyleChangeList& aChangeList)
 {
+  // Handle lazy frame construction.
+  if (aTextNode->HasFlag(NODE_NEEDS_FRAME)) {
+    aChangeList.AppendChange(nullptr, aTextNode, nsChangeHint_ReconstructFrame);
+    return;
+  }
+
+  // Handle restyle.
   nsIFrame* primaryFrame = aTextNode->GetPrimaryFrame();
   if (primaryFrame) {
     RefPtr<nsStyleContext> oldStyleContext = primaryFrame->StyleContext();
     RefPtr<nsStyleContext> newContext =
       aStyleSet->ResolveStyleForText(aTextNode, aParentContext);
 
     for (nsIFrame* f = primaryFrame; f;
          f = GetNextContinuationWithSameStyle(f, oldStyleContext)) {
@@ -355,34 +378,24 @@ ServoRestyleManager::ProcessPendingResty
   PresContext()->RefreshDriver()->MostRecentRefresh();
 
 
   // Perform the Servo traversal, and the post-traversal if required. We do this
   // in a loop because certain rare paths in the frame constructor (like
   // uninstalling XBL bindings) can trigger additional style validations.
   mInStyleRefresh = true;
   while (styleSet->StyleDocument()) {
-
     PresContext()->EffectCompositor()->ClearElementsToRestyle();
 
-    // First do any queued-up frame creation. (see bugs 827239 and 997506).
-    //
-    // XXXEmilio I'm calling this to avoid random behavior changes, since we
-    // delay frame construction after styling we should re-check once our
-    // model is more stable whether we can skip this call.
-    //
-    // Note this has to be *after* restyling, because otherwise frame
-    // construction will find unstyled nodes, and that's not funny.
-    PresContext()->FrameConstructor()->CreateNeededFrames();
-
-    // Recreate style contexts and queue up change hints.
+    // Recreate style contexts, and queue up change hints (which also handle
+    // lazy frame construction).
     nsStyleChangeList currentChanges;
     DocumentStyleRootIterator iter(doc);
     while (Element* root = iter.GetNextStyleRoot()) {
-      RecreateStyleContexts(root, nullptr, styleSet, currentChanges);
+      ProcessPostTraversal(root, nullptr, styleSet, currentChanges);
     }
 
     // Process the change hints.
     //
     // Unfortunately, the frame constructor can generate new change hints while
     // processing existing ones. We redirect those into a secondary queue and
     // iterate until there's nothing left.
     ReentrantChangeList newChanges;
--- a/layout/base/ServoRestyleManager.h
+++ b/layout/base/ServoRestyleManager.h
@@ -95,27 +95,27 @@ public:
 protected:
   ~ServoRestyleManager() override
   {
     MOZ_ASSERT(!mReentrantChanges);
   }
 
 private:
   /**
-   * Traverses a tree of content that Servo has just restyled, recreating style
-   * contexts for their frames with the new style data.
+   * Performs post-Servo-traversal processing on this element and its descendants.
    */
-  void RecreateStyleContexts(Element* aElement,
-                             nsStyleContext* aParentContext,
-                             ServoStyleSet* aStyleSet,
-                             nsStyleChangeList& aChangeList);
+  void ProcessPostTraversal(Element* aElement,
+                            nsStyleContext* aParentContext,
+                            ServoStyleSet* aStyleSet,
+                            nsStyleChangeList& aChangeList);
 
-  void RecreateStyleContextsForText(nsIContent* aTextNode,
-                                    nsStyleContext* aParentContext,
-                                    ServoStyleSet* aStyleSet);
+  void ProcessPostTraversalForText(nsIContent* aTextNode,
+                                   nsStyleContext* aParentContext,
+                                   ServoStyleSet* aStyleSet,
+                                   nsStyleChangeList& aChangeList);
 
   inline ServoStyleSet* StyleSet() const
   {
     MOZ_ASSERT(PresContext()->StyleSet()->IsServo(),
                "ServoRestyleManager should only be used with a Servo-flavored "
                "style backend");
     return PresContext()->StyleSet()->AsServo();
   }
--- a/layout/base/nsStyleChangeList.cpp
+++ b/layout/base/nsStyleChangeList.cpp
@@ -24,19 +24,24 @@ nsStyleChangeList::AppendChange(nsIFrame
              "and should be stripped out");
   MOZ_ASSERT(aContent || !(aHint & nsChangeHint_ReconstructFrame),
              "must have content");
   // XXXbz we should make this take Element instead of nsIContent
   MOZ_ASSERT(!aContent || aContent->IsElement() ||
              // display:contents elements posts the changes for their children:
              (aFrame && aContent->GetParent() &&
              aFrame->PresContext()->FrameManager()->
-               GetDisplayContentsStyleFor(aContent->GetParent())),
+               GetDisplayContentsStyleFor(aContent->GetParent())) ||
+             (aContent->IsNodeOfType(nsINode::eTEXT) &&
+              aContent->IsStyledByServo() &&
+              aContent->HasFlag(NODE_NEEDS_FRAME) &&
+              aHint & nsChangeHint_ReconstructFrame),
              "Shouldn't be trying to restyle non-elements directly, "
-             "except if it's a display:contents child");
+             "except if it's a display:contents child or a text node "
+             "doing lazy frame construction");
   MOZ_ASSERT(!(aHint & nsChangeHint_AllReflowHints) ||
              (aHint & nsChangeHint_NeedReflow),
              "Reflow hint bits set without actually asking for a reflow");
 
   // Filter out all other changes for same content
   if (!IsEmpty() && (aHint & nsChangeHint_ReconstructFrame)) {
     if (aContent) {
       // NOTE: This is captured by reference to please static analysis.
--- a/layout/style/test/stylo-failures.md
+++ b/layout/style/test/stylo-failures.md
@@ -33,20 +33,20 @@ Any line which doesn't follow the format
   * test_bug453896_deck.html: &lt;style media&gt; support [8]
   * test_media_queries.html [1]
   * test_media_queries_dynamic.html [17]
   * test_media_queries_dynamic_xbl.html [2]
   * test_webkit_device_pixel_ratio.html: -webkit-device-pixel-ratio [3]
 * test_all_shorthand.html: all shorthand servo/servo#15055 [*]
 * Animation support:
   * test_animations.html [277]
-  * test_animations_async_tests.html [3]
+  * test_animations_async_tests.html [2]
   * test_animations_dynamic_changes.html [1]
   * test_bug716226.html [1]
-  * test_flexbox_flex_grow_and_shrink.html [16]
+  * test_flexbox_flex_grow_and_shrink.html [8]
   * OMTA
     * test_animations_effect_timing_duration.html [1]
     * test_animations_effect_timing_enddelay.html [1]
     * test_animations_effect_timing_iterations.html [1]
     * test_animations_iterationstart.html [1]
     * test_animations_omta.html [1]
     * test_animations_omta_start.html [1]
     * test_animations_pausing.html [1]