Bug 1596391 - Remove lazy computation of FlattenedChildIterator::mXBLInvolved. r=smaug
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 14 Nov 2019 12:52:00 +0000
changeset 501930 853ed67836c6e75b432f804a3e628f3040968ff6
parent 501929 1dc82629b31eca44d1f0d318ba358aa5fce6aa79
child 501931 f6794e4da7e8dc8c958338de8404716d559d4cba
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1596391
milestone72.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 1596391 - Remove lazy computation of FlattenedChildIterator::mXBLInvolved. r=smaug XBL is gone, and thus we don't need to check for the children element anymore, and computing it upfront becomes cheap. Depends on D52990 Differential Revision: https://phabricator.services.mozilla.com/D52991
dom/base/ChildIterator.cpp
dom/base/ChildIterator.h
--- a/dom/base/ChildIterator.cpp
+++ b/dom/base/ChildIterator.cpp
@@ -89,56 +89,34 @@ nsIContent* ExplicitChildIterator::GetNe
     }
   }
 
   return mChild;
 }
 
 void FlattenedChildIterator::Init(bool aIgnoreXBL) {
   if (aIgnoreXBL) {
-    mXBLInvolved = Some(false);
     return;
   }
 
   // TODO(emilio): I think it probably makes sense to only allow constructing
   // FlattenedChildIterators with Element.
   if (mParent->IsElement()) {
     if (ShadowRoot* shadow = mParent->AsElement()->GetShadowRoot()) {
       mParent = shadow;
-      mXBLInvolved = Some(true);
+      mXBLInvolved = true;
+      return;
+    }
+    if (mParentAsSlot) {
+      mXBLInvolved = true;
       return;
     }
   }
 }
 
-bool FlattenedChildIterator::ComputeWhetherXBLIsInvolved() const {
-  MOZ_ASSERT(mXBLInvolved.isNothing());
-  // We set mXBLInvolved to true if either the node we're iterating has a
-  // binding with content attached to it (in which case it is handled in Init),
-  // the node is generated XBL content and has an <xbl:children> child, or the
-  // node is a <slot> element.
-  if (!mParent->GetBindingParent()) {
-    return false;
-  }
-
-  if (mParentAsSlot) {
-    return true;
-  }
-
-  for (nsIContent* child = mParent->GetFirstChild(); child;
-       child = child->GetNextSibling()) {
-    if (child->NodeInfo()->Equals(nsGkAtoms::children, kNameSpaceID_XBL)) {
-      MOZ_ASSERT(child->GetBindingParent());
-      return true;
-    }
-  }
-
-  return false;
-}
-
 bool ExplicitChildIterator::Seek(const nsIContent* aChildToFind) {
   if (aChildToFind->GetParent() == mParent &&
       !aChildToFind->IsRootOfAnonymousSubtree()) {
     // Fast path: just point ourselves to aChildToFind, which is a
     // normal DOM child of ours.
     mChild = const_cast<nsIContent*>(aChildToFind);
     mIndexInInserted = 0;
     mDefaultChild = nullptr;
--- a/dom/base/ChildIterator.h
+++ b/dom/base/ChildIterator.h
@@ -139,27 +139,24 @@ class FlattenedChildIterator : public Ex
         mOriginalContent(aOther.mOriginalContent),
         mXBLInvolved(aOther.mXBLInvolved) {}
 
   FlattenedChildIterator(const FlattenedChildIterator& aOther)
       : ExplicitChildIterator(aOther),
         mOriginalContent(aOther.mOriginalContent),
         mXBLInvolved(aOther.mXBLInvolved) {}
 
+  // TODO(emilio): Rename to use shadow dom terminology.
   bool XBLInvolved() {
-    if (mXBLInvolved.isNothing()) {
-      mXBLInvolved = Some(ComputeWhetherXBLIsInvolved());
-    }
-    return *mXBLInvolved;
+    return mXBLInvolved;
   }
 
   const nsIContent* Parent() const { return mOriginalContent; }
 
  private:
-  bool ComputeWhetherXBLIsInvolved() const;
 
   void Init(bool aIgnoreXBL);
 
  protected:
   /**
    * This constructor is a hack to help AllChildrenIterator which sometimes
    * doesn't want to consider XBL.
    */
@@ -171,19 +168,17 @@ class FlattenedChildIterator : public Ex
     Init(ignoreXBL);
   }
 
   const nsIContent* mOriginalContent;
 
  private:
   // For certain optimizations, nsCSSFrameConstructor needs to know if the child
   // list of the element that we're iterating matches its .childNodes.
-  //
-  // This is lazily computed when asked for it.
-  Maybe<bool> mXBLInvolved;
+  bool mXBLInvolved = false;
 };
 
 /**
  * AllChildrenIterator traverses the children of an element including before /
  * after content and optionally XBL children.  The iterator can be initialized
  * to start at the end by providing false for aStartAtBeginning in order to
  * start iterating in reverse from the last child.
  *