Bug 1418456: Get rid of unstyled children only traversals. r=heycam

They're useless now, provided we remove the hack to not traverse XBL-bound
elements on initial styling.

This also allows us to get rid of the fallback case.

MozReview-Commit-ID: AvBVdyF1wb6

dom/xbl/nsXBLService.cpp

@@ -382,8 +382,7 @@ nsXBLService::IsChromeOrResourceURI(nsIURI* aURI)
   return false;
 }
 
-// RAII class to invoke StyleNewChildren for Elements in Servo-backed documents
-// on destruction.
+// RAII class to restyle the XBL bound element when it shuffles the flat tree.
 class MOZ_STACK_CLASS AutoStyleElement
 {
 public:
@@ -403,7 +402,7 @@ public:
     }
 
     if (ServoStyleSet* servoSet = presShell->StyleSet()->GetAsServo()) {
-      servoSet->ReresolveStyleForBindings(mElement);
+      servoSet->StyleNewSubtree(mElement);
     }
   }
 

layout/base/nsCSSFrameConstructor.cpp

@@ -2001,28 +2001,20 @@ nsCSSFrameConstructor::CreateGeneratedContentItem(nsFrameConstructorState& aStat
   }
 
   uint32_t contentCount = pseudoStyleContext->StyleContent()->ContentCount();
-  bool createdChildElement = false;
   for (uint32_t contentIndex = 0; contentIndex < contentCount; contentIndex++) {
     nsCOMPtr<nsIContent> content =
       CreateGeneratedContent(aState, aParentContent, pseudoStyleContext,
                              contentIndex);
     if (content) {
       container->AppendChildTo(content, false);
-      if (content->IsElement()) {
-        createdChildElement = true;
+      if (content->IsElement() && servoStyle) {
+        // If we created any children elements, Servo needs to traverse them, but
+        // the root is already set up.
+        mPresShell->StyleSet()->AsServo()->StyleNewSubtree(content->AsElement());
       }
     }
   }
 
-  // We may need to do a synchronous servo traversal in various uncommon cases.
-  if (servoStyle) {
-    if (createdChildElement) {
-      // If we created any children elements, Servo needs to traverse them, but
-      // the root is already set up.
-      mPresShell->StyleSet()->AsServo()->StyleNewChildren(container);
-    }
-  }
-
   AddFrameConstructionItemsInternal(aState, container, aParentFrame, elemName,
                                     kNameSpaceID_None, true,
                                     pseudoStyleContext,
@@ -2619,10 +2611,6 @@ nsCSSFrameConstructor::ConstructDocElementFrame(Element*                 aDocEle
           aDocElement, nullptr, LazyComputeBehavior::Assert);
       display = styleContext->StyleDisplay();
     }
-  } else if (display->mBinding.ForceGet() && aDocElement->IsStyledByServo()) {
-    // See the comment in AddFrameConstructionItemsInternal for why this is
-    // needed.
-    mPresShell->StyleSet()->AsServo()->StyleNewChildren(aDocElement);
   }
 
   // --------- IF SCROLLABLE WRAP IN SCROLLFRAME --------
@@ -5934,18 +5922,6 @@ nsCSSFrameConstructor::AddFrameConstructionItemsInternal(nsFrameConstructorState
       }
 
       aTag = mDocument->BindingManager()->ResolveTag(aContent, &aNameSpaceID);
-    } else if (display->mBinding.ForceGet()) {
-      if (aContent->IsStyledByServo()) {
-        // Servo's should_traverse_children skips styling descendants of
-        // elements with a -moz-binding value.  For -moz-binding URLs that can
-        // be resolved, we will load the binding above, which will style the
-        // children after they have been rearranged in the flattened tree.
-        // If the URL couldn't be resolved, we still need to style the children,
-        // so we do that here.
-        //
-        // FIXME(emilio): Again, should go away.
-        mPresShell->StyleSet()->AsServo()->StyleNewChildren(aContent->AsElement());
-      }
     }
   }
 
@@ -7519,21 +7495,12 @@ nsCSSFrameConstructor::StyleNewChildRange(nsIContent* aStartChild,
 
   for (nsIContent* child = aStartChild; child != aEndChild;
        child = child->GetNextSibling()) {
-    // Calling StyleNewChildren on one child will end up styling another child,
-    // if they share the same flattened tree parent.  So we check HasServoData()
-    // to avoid a wasteful call to GetFlattenedTreeParent (on the child) and
-    // StyleNewChildren (on the flattened tree parent) when we detect we've
-    // already handled that parent.  In the common case of inserting elements
-    // into a container that does not have an XBL binding or shadow tree with
-    // distributed children, this boils down to a single call to
-    // GetFlattenedTreeParent/StyleNewChildren, and traversing the list of
-    // children checking HasServoData (which is fast).
-    if (child->IsElement() && !child->AsElement()->HasServoData()) {
+    if (child->IsElement()) {
       Element* parent = child->AsElement()->GetFlattenedTreeParentElement();
       // NB: Parent may be null if the content is appended to a shadow root, and
       // isn't assigned to any insertion point.
       if (MOZ_LIKELY(parent) && parent->HasServoData()) {
-        styleSet->StyleNewChildren(parent);
+        styleSet->StyleNewSubtree(child->AsElement());
       }
     }
   }

layout/style/ServoStyleSet.cpp

@@ -349,20 +349,6 @@ ServoStyleSet::ResolveStyleFor(Element* aElement,
   return ResolveServoStyle(aElement);
 }
 
-void
-ServoStyleSet::ReresolveStyleForBindings(Element* aElement)
-{
-  MOZ_ASSERT(!aElement->HasServoData(), "Should've been cleared before!");
-  StyleNewSubtree(aElement);
-
-  // Servo's should_traverse_children() in traversal.rs skips
-  // styling descendants of elements with a -moz-binding the
-  // first time. Thus call StyleNewChildren() again.
-  //
-  // FIXME(emilio): This is just stupid, we should remove that now.
-  StyleNewChildren(aElement);
-}
-
 /**
  * Clears any stale Servo element data that might existing in the specified
  * element's document.  Upon destruction, asserts that the element and all
@@ -1058,7 +1044,7 @@ ServoStyleSet::StyleDocument(ServoTraversalFlags aFlags)
 void
 ServoStyleSet::StyleNewSubtree(Element* aRoot)
 {
-  MOZ_ASSERT(!aRoot->HasServoData(), "Should have called StyleNewChildren");
+  MOZ_ASSERT(!aRoot->HasServoData());
   PreTraverseSync();
   AutoPrepareTraversal guard(this);
 
@@ -1087,72 +1073,6 @@ ServoStyleSet::StyleNewSubtree(Element* aRoot)
   }
 }
 
-void
-ServoStyleSet::StyleNewChildren(Element* aParent)
-{
-  MOZ_ASSERT(aParent->HasServoData(), "Should have called StyleNewSubtree");
-  if (Servo_Element_IsDisplayNone(aParent)) {
-    return;
-  }
-
-  PreTraverseSync();
-  AutoPrepareTraversal guard(this);
-
-  // Implementing StyleNewChildren correctly is very annoying, for two reasons:
-  // (1) We have to tiptoe around existing invalidations in the tree. In rare
-  //     cases Gecko calls into the frame constructor with pending invalidations,
-  //     and in other rare cases the frame constructor needs to perform
-  //     synchronous styling rather than using the normal lazy frame
-  //     construction mechanism. If both of these cases happen together, then we
-  //     get an |aParent| with dirty style and/or dirty descendants, which we
-  //     can't process right now because we're not set up to update the frames
-  //     and process the change hints. We handle this case by passing the
-  //     UnstyledOnly flag to servo.
-  // (2) We don't have a good way to handle animations. When styling unstyled
-  //     content, a followup animation traversal may be required (for example
-  //     to change the transition style from the after-change style we used in
-  //     the animation cascade to the timeline-correct style). But once we do
-  //     the initial styling, we don't have a good way to distinguish the new
-  //     content and scope our animation processing to that. We should handle
-  //     this somehow, but for now we just don't do the followup animation
-  //     traversal, which is buggy.
-
-  // Set ourselves up to find the children by marking the parent as having
-  // dirty descendants.
-  bool hadDirtyDescendants = aParent->HasDirtyDescendantsForServo();
-  aParent->SetHasDirtyDescendantsForServo();
-
-  auto flags = ServoTraversalFlags::UnstyledOnly;
-  if (ShouldTraverseInParallel()) {
-    flags |= ServoTraversalFlags::ParallelTraversal;
-  }
-
-  // Do the traversal. The snapshots will be ignored.
-  const SnapshotTable& snapshots = Snapshots();
-  Servo_TraverseSubtree(aParent, mRawSet.get(), &snapshots, flags);
-
-  // Restore the old state of the dirty descendants bit.
-  if (!hadDirtyDescendants) {
-    aParent->UnsetHasDirtyDescendantsForServo();
-  }
-}
-
-void
-ServoStyleSet::StyleNewlyBoundElement(Element* aElement)
-{
-  // In general the element is always styled by the time we're applying XBL
-  // bindings, because we need to style the element to know what the binding
-  // URI is. However, programmatic consumers of the XBL service (like the
-  // XML pretty printer) _can_ apply bindings without having styled the bound
-  // element. We could assert against this and require the callers manually
-  // resolve the style first, but it's easy enough to just handle here.
-  if (MOZ_LIKELY(aElement->HasServoData())) {
-    StyleNewChildren(aElement);
-  } else {
-    StyleNewSubtree(aElement);
-  }
-}
-
 void
 ServoStyleSet::MarkOriginsDirty(OriginFlags aChangedOrigins)
 {

layout/style/ServoStyleSet.h

@@ -181,12 +181,6 @@ public:
                   ServoStyleContext* aParentContext,
                   LazyComputeBehavior aMayCompute);
 
-  // Clear style data and resolve style for the given element and its
-  // subtree for changes to -moz-binding.
-  //
-  // TODO(emilio): Remove.
-  void ReresolveStyleForBindings(Element* aElement);
-
   // Get a style context for a text node (which no rules will match).
   //
   // The returned style context will have nsCSSAnonBoxes::mozText as its pseudo.
@@ -317,22 +311,6 @@ public:
    */
   void StyleNewSubtree(dom::Element* aRoot);
 
-  /**
-   * Like the above, but skips the root node, and only styles unstyled children.
-   * When potentially appending multiple children, it's preferable to call
-   * StyleNewChildren on the node rather than making multiple calls to
-   * StyleNewSubtree on each child, since it allows for more parallelism.
-   */
-  void StyleNewChildren(dom::Element* aParent);
-
-  /**
-   * Eagerly styles the children of an element that has just had an XBL
-   * binding applied to it.  Some XBL consumers attach bindings to elements
-   * that have not been styled yet, and in such cases, this will do the
-   * equivalent of StyleNewSubtree instead.
-   */
-  void StyleNewlyBoundElement(dom::Element* aElement);
-
   /**
    * Helper for correctly calling UpdateStylist without paying the cost of an
    * extra function call in the common no-rebuild-needed case.

layout/style/ServoTypes.h

@@ -59,9 +59,6 @@ enum class ServoTraversalFlags : uint32_t {
   AnimationOnly = 1 << 0,
   // Traverses as normal mode but tries to update all CSS animations.
   ForCSSRuleChanges = 1 << 1,
-  // Styles unstyled elements, but does not handle invalidations on
-  // already-styled elements.
-  UnstyledOnly = 1 << 2,
   // A forgetful traversal ignores the previous state of the frame tree, and
   // thus does not compute damage or maintain other state describing the styles
   // pre-traversal. A forgetful traversal is usually the right thing if you