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 374939 4a2a58be8c6c54dbf85b90aa85e4d4281e72088f
parent 374938 a57ff8701dc2a1132e404cf4c56a571eb1713c6d
child 374940 a25b3225e35191dbd368195852b86da57cdcc7a5
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1338921
milestone54.0a1
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]