Bug 1427677: Get rid of nsContentUtils::HasDistributedChildren. r=bz

The whole function doesn't have much sense.

I killed its only DOM use in bug 1427511.

Now it only has two callers in nsCSSFrameConstructor, which basically only want
to know whether the children of the same node can have different flattened tree
parents.

So let's check that directly instead (checking whether the element has a binding
or a shadow root), and simplify a bit other surrounding code while at it.

Leave the XUL popup / menubar code doing the broken thing they were doing
beforehand, because it doesn't look to me like it's trivial to fix... They're
effectively assuming that the children of the menupopup end up in a single
insertion point, which is true, but doesn't need to be. Maybe they should walk
the DOM tree? Don't want to dig into that right now, since XUL insertion points
can be filtered and all that... Not fun.

Also, this removes the broken optimization that used to check
mParentFrame->GetContent()->HasChildren(), because it's pretty broken. It used
to be relevant before bug 653881, because <children> element used to not exist,
but now the insertion point at least needs to contain the <children> element all
the time.

There even used to be a XXX comment saying that the optimization didn't work,
which was removed in:

  https://hg.mozilla.org/mozilla-central/rev/2d8585ec74b3

We could still check for "no insertion points", and optimize that, but it
doesn't seem worth it.

MozReview-Commit-ID: L4lspkxKENr

dom/base/nsContentUtils.cpp

@@ -7478,30 +7478,6 @@ nsContentUtils::GetHTMLEditor(nsPresContext* aPresContext)
   return docShell->GetHTMLEditor();
 }
 
-// static
-bool
-nsContentUtils::HasDistributedChildren(nsIContent* aContent)
-{
-  if (!aContent || !nsDocument::IsWebComponentsEnabled(aContent)) {
-    return false;
-  }
-
-  if (aContent->GetShadowRoot()) {
-    // Children of a shadow root host are distributed
-    // to content insertion points in the shadow root.
-    return true;
-  }
-
-  HTMLSlotElement* slotEl = HTMLSlotElement::FromContent(aContent);
-  if (slotEl && slotEl->GetContainingShadow()) {
-    // Children of a slot are rendered if the slot does not have any assigned
-    // nodes (fallback content).
-    return slotEl->AssignedNodes().IsEmpty();
-  }
-
-  return false;
-}
-
 // static
 bool
 nsContentUtils::IsForbiddenRequestHeader(const nsACString& aHeader)

dom/base/nsContentUtils.h

@@ -2699,12 +2699,6 @@ public:
    */
   static mozilla::LogModule* DOMDumpLog();
 
-  /**
-   * Returns whether the children of the provided content are
-   * nodes that are distributed to Shadow DOM insertion points.
-   */
-  static bool HasDistributedChildren(nsIContent* aContent);
-
   /**
    * Returns whether a given header is forbidden for an XHR or fetch
    * request.

layout/base/nsCSSFrameConstructor.cpp

@@ -7412,34 +7412,63 @@ nsCSSFrameConstructor::IssueSingleInsertNofications(nsIContent* aContainer,
   }
 }
 
+bool
+nsCSSFrameConstructor::InsertionPoint::IsMultiple() const
+{
+  if (!mParentFrame) {
+    return false;
+  }
+
+  // Fieldset frames have multiple normal flow child frame lists so handle it
+  // the same as if it had multiple content insertion points.
+  if (mParentFrame->IsFieldSetFrame()) {
+    return true;
+  }
+
+  // A details frame moves the first summary frame to be its first child, so we
+  // treat it as if it has multiple content insertion points.
+  if (mParentFrame->IsDetailsFrame()) {
+    return true;
+  }
+
+  return false;
+}
+
 nsCSSFrameConstructor::InsertionPoint
 nsCSSFrameConstructor::GetRangeInsertionPoint(nsIContent* aContainer,
                                               nsIContent* aStartChild,
                                               nsIContent* aEndChild)
 {
-  // See if we have an XBL insertion point. If so, then that's our
-  // real parent frame; if not, then the frame hasn't been built yet
-  // and we just bail.
-  InsertionPoint insertionPoint = GetInsertionPoint(aContainer, nullptr);
-  if (!insertionPoint.mParentFrame && !insertionPoint.mMultiple) {
-    return insertionPoint; // Don't build the frames.
+  MOZ_ASSERT(aStartChild);
+
+  // If the children of the container may be distributed to different insertion
+  // points, insert them separately and bail out, letting ContentInserted handle
+  // the mess.
+  if (aContainer->GetShadowRoot() || aContainer->GetXBLBinding()) {
+    IssueSingleInsertNofications(aContainer, aStartChild, aEndChild);
+    return { };
   }
 
-  if (insertionPoint.mMultiple || aStartChild->GetXBLInsertionPoint()) {
-    // If we have multiple insertion points or if we have an insertion point
-    // and the operation is not a true append or if the insertion point already
-    // has explicit children, then we must fall back.
-    if (insertionPoint.mMultiple || aEndChild ||
-        insertionPoint.mParentFrame->GetContent()->HasChildren()) {
-      // Now comes the fun part.  For each inserted child, make a
-      // ContentInserted call as if it had just gotten inserted and
-      // let ContentInserted handle the mess.
-      IssueSingleInsertNofications(aContainer, aStartChild, aEndChild);
-      insertionPoint.mParentFrame = nullptr;
+#ifdef DEBUG
+  {
+    nsIContent* expectedParent = aStartChild->GetFlattenedTreeParent();
+    for (nsIContent* child = aStartChild->GetNextSibling(); child;
+         child = child->GetNextSibling()) {
+      MOZ_ASSERT(child->GetFlattenedTreeParent() == expectedParent);
     }
   }
+#endif
 
-  return insertionPoint;
+  // Now the flattened tree parent of all the siblings is the same, just use the
+  // same insertion point and take the fast path, unless it's a multiple
+  // insertion point.
+  InsertionPoint ip = GetInsertionPoint(aStartChild);
+  if (ip.IsMultiple()) {
+    IssueSingleInsertNofications(aContainer, aStartChild, aEndChild);
+    return { };
+  }
+
+  return ip;
 }
 
 bool
@@ -8080,7 +8109,7 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer,
     // See if we have an XBL insertion point. If so, then that's our
     // real parent frame; if not, then the frame hasn't been built yet
     // and we just bail.
-    insertion = GetInsertionPoint(aContainer, aStartChild);
+    insertion = GetInsertionPoint(aStartChild);
   } else {
     // Get our insertion point. If we need to issue single ContentInserted's
     // GetRangeInsertionPoint will take care of that for us.
@@ -9367,78 +9396,17 @@ nsCSSFrameConstructor::ReplicateFixedFrames(nsPageContentFrame* aParentFrame)
 }
 
 nsCSSFrameConstructor::InsertionPoint
-nsCSSFrameConstructor::GetInsertionPoint(nsIContent* aContainer,
-                                         nsIContent* aChild)
+nsCSSFrameConstructor::GetInsertionPoint(nsIContent* aChild)
 {
-  nsBindingManager* bindingManager = mDocument->BindingManager();
-
-  nsIContent* insertionElement;
-  if (aChild) {
-    // We've got an explicit insertion child. Check to see if it's
-    // anonymous.
-    if (aChild->GetBindingParent() == aContainer) {
-      // This child content is anonymous. Don't use the insertion
-      // point, since that's only for the explicit kids.
-      return InsertionPoint(GetContentInsertionFrameFor(aContainer), aContainer);
-    }
-
-    if (nsContentUtils::HasDistributedChildren(aContainer) ||
-        aContainer->IsShadowRoot()) {
-      // The container distributes nodes or is a shadow root, use the frame of
-      // the flattened tree parent.
-      //
-      // It may be the case that the node is distributed but not matched to any
-      // insertion points, so there is no flattened parent.
-      //
-      // FIXME(emilio): We should be able to use this path all the time.
-      nsIContent* flattenedParent = aChild->GetFlattenedTreeParent();
-      if (flattenedParent) {
-        return InsertionPoint(GetContentInsertionFrameFor(flattenedParent),
-                              flattenedParent);
-      }
-      return InsertionPoint();
-    }
-
-    insertionElement = bindingManager->FindNestedInsertionPoint(aContainer, aChild);
-  } else {
-    if (nsContentUtils::HasDistributedChildren(aContainer)) {
-      // The container distributes nodes to shadow DOM insertion points.
-      // Return with aMultiple set to true to induce callers to insert children
-      // individually into the node's flattened tree parent.
-      return InsertionPoint(nullptr, nullptr, true);
-    }
-
-    bool multiple;
-    insertionElement = bindingManager->FindNestedSingleInsertionPoint(aContainer, &multiple);
-    if (multiple) {
-      return InsertionPoint(nullptr, nullptr, true);
-    }
-  }
-
+  MOZ_ASSERT(aChild);
+  nsIContent* insertionElement = aChild->GetFlattenedTreeParent();
   if (!insertionElement) {
-    // The FindNested{,Single}InsertionPoint methods return null in the case
-    // that there is a binding with anonymous content but no insertion point.
-    // In that case the element doesn't belong in the flattened tree, and we
-    // don't want to render it.
-    return InsertionPoint();
+    // The element doesn't belong in the flattened tree, and thus we don't want
+    // to render it.
+    return { };
   }
 
-  InsertionPoint insertion(GetContentInsertionFrameFor(insertionElement),
-                           insertionElement);
-
-  // Fieldset frames have multiple normal flow child frame lists so handle it
-  // the same as if it had multiple content insertion points.
-  if (insertion.mParentFrame && insertion.mParentFrame->IsFieldSetFrame()) {
-    insertion.mMultiple = true;
-  }
-
-  // A details frame moves the first summary frame to be its first child, so we
-  // treat it as if it has multiple content insertion points.
-  if (insertion.mParentFrame && insertion.mParentFrame->IsDetailsFrame()) {
-    insertion.mMultiple = true;
-  }
-
-  return insertion;
+  return { GetContentInsertionFrameFor(insertionElement), insertionElement };
 }
 
 // Capture state for the frame tree rooted at the frame associated with the
@@ -11144,7 +11112,7 @@ nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState& aState,
       TreeMatchContext::AutoAncestorPusher ancestorPusher(aState.mTreeMatchContext);
       if (parent != aContent && parent->IsElement()) {
         insertion.mContainer = child->GetFlattenedTreeParent();
-        MOZ_ASSERT(insertion.mContainer == GetInsertionPoint(parent, child).mContainer);
+        MOZ_ASSERT(insertion.mContainer == GetInsertionPoint(child).mContainer);
         if (aState.HasAncestorFilter()) {
           ancestorPusher.PushAncestorAndStyleScope(parent->AsElement());
         } else {

layout/base/nsCSSFrameConstructor.h

@@ -141,11 +141,15 @@ private:
   struct InsertionPoint
   {
     InsertionPoint()
-      : mParentFrame(nullptr), mContainer(nullptr), mMultiple(false) {}
-    InsertionPoint(nsContainerFrame* aParentFrame, nsIContent* aContainer,
-                   bool aMultiple = false)
-      : mParentFrame(aParentFrame), mContainer(aContainer),
-        mMultiple(aMultiple) {}
+      : mParentFrame(nullptr)
+      , mContainer(nullptr)
+    {}
+
+    InsertionPoint(nsContainerFrame* aParentFrame, nsIContent* aContainer)
+      : mParentFrame(aParentFrame)
+      , mContainer(aContainer)
+    {}
+
     /**
      * The parent frame to use if the inserted children needs to create
      * frame(s).  May be null, which signals that  we shouldn't try to
@@ -160,12 +164,14 @@ private:
      * It's undefined if mParentFrame is null.
      */
     nsIContent* mContainer;
+
     /**
-     * If true then there are multiple insertion points, which means consumers
-     * should insert children individually into the node's flattened tree parent.
+     * Whether it is required to insert children one-by-one instead of as a
+     * range.
      */
-    bool mMultiple;
+    bool IsMultiple() const;
   };
+
   /**
    * Checks if the children of aContainer in the range [aStartChild, aEndChild)
    * can be inserted/appended to one insertion point together. If so, returns
@@ -357,9 +363,15 @@ public:
   nsresult ReplicateFixedFrames(nsPageContentFrame* aParentFrame);
 
   /**
-   * Get the XBL insertion point for aChild in aContainer.
+   * Get the insertion point for aChild.
    */
-  InsertionPoint GetInsertionPoint(nsIContent* aContainer, nsIContent* aChild);
+  InsertionPoint GetInsertionPoint(nsIContent* aChild);
+
+  /**
+   * Return the insertion frame of the primary frame of aContent, or its nearest
+   * ancestor that isn't display:contents.
+   */
+  nsContainerFrame* GetContentInsertionFrameFor(nsIContent* aContent);
 
   void CreateListBoxContent(nsContainerFrame* aParentFrame,
                             nsIFrame*         aPrevFrame,
@@ -2180,12 +2192,6 @@ private:
                                     nsIContent* aStartSkipChild = nullptr,
                                     nsIContent *aEndSkipChild = nullptr);
 
-  /**
-   * Return the insertion frame of the primary frame of aContent, or its nearest
-   * ancestor that isn't display:contents.
-   */
-  nsContainerFrame* GetContentInsertionFrameFor(nsIContent* aContent);
-
   // see if aContent and aSibling are legitimate siblings due to restrictions
   // imposed by table columns
   // XXXbz this code is generally wrong, since the frame for aContent

layout/xul/nsMenuBarFrame.cpp

@@ -162,11 +162,8 @@ nsMenuBarFrame::FindMenuWithShortcut(nsIDOMKeyEvent* aKeyEvent)
     return nullptr; // no character was pressed so just return
 
   // Enumerate over our list of frames.
-  auto insertion = PresShell()->FrameConstructor()->
-    GetInsertionPoint(GetContent(), nullptr);
-  nsContainerFrame* immediateParent = insertion.mParentFrame;
-  if (!immediateParent)
-    immediateParent = this;
+  nsContainerFrame* immediateParent =
+    nsXULPopupManager::ImmediateParentFrame(this);
 
   // Find a most preferred accesskey which should be returned.
   nsIFrame* foundMenu = nullptr;

layout/xul/nsMenuPopupFrame.cpp

@@ -2090,12 +2090,8 @@ nsMenuPopupFrame::FindMenuWithShortcut(nsIDOMKeyEvent* aKeyEvent, bool& doAction
   doAction = false;
 
   // Enumerate over our list of frames.
-  auto insertion = PresShell()->
-    FrameConstructor()->GetInsertionPoint(GetContent(), nullptr);
-  nsContainerFrame* immediateParent = insertion.mParentFrame;
-  if (!immediateParent)
-    immediateParent = this;
-
+  nsContainerFrame* immediateParent =
+    nsXULPopupManager::ImmediateParentFrame(this);
   uint32_t matchCount = 0, matchShortcutCount = 0;
   bool foundActive = false;
   nsMenuFrame* frameBefore = nullptr;

layout/xul/nsXULPopupManager.cpp

@@ -2433,19 +2433,32 @@ nsXULPopupManager::HandleKeyboardEventWithKeyCode(
   return true;
 }
 
+nsContainerFrame*
+nsXULPopupManager::ImmediateParentFrame(nsContainerFrame* aFrame)
+{
+  MOZ_ASSERT(aFrame && aFrame->GetContent());
+
+  bool multiple = false; // Unused
+  nsIContent* insertionPoint =
+    aFrame->GetContent()->OwnerDoc()->BindingManager()->
+      FindNestedSingleInsertionPoint(aFrame->GetContent(), &multiple);
+
+  nsCSSFrameConstructor* fc = aFrame->PresContext()->FrameConstructor();
+  nsContainerFrame* insertionFrame =
+    insertionPoint
+      ? fc->GetContentInsertionFrameFor(insertionPoint)
+      : nullptr;
+
+  return insertionFrame ? insertionFrame : aFrame;
+}
+
 nsMenuFrame*
 nsXULPopupManager::GetNextMenuItem(nsContainerFrame* aParent,
                                    nsMenuFrame* aStart,
                                    bool aIsPopup,
                                    bool aWrap)
 {
-  nsPresContext* presContext = aParent->PresContext();
-  auto insertion = presContext->PresShell()->
-    FrameConstructor()->GetInsertionPoint(aParent->GetContent(), nullptr);
-  nsContainerFrame* immediateParent = insertion.mParentFrame;
-  if (!immediateParent)
-    immediateParent = aParent;
-
+  nsContainerFrame* immediateParent = ImmediateParentFrame(aParent);
   nsIFrame* currFrame = nullptr;
   if (aStart) {
     if (aStart->GetNextSibling())
@@ -2505,13 +2518,7 @@ nsXULPopupManager::GetPreviousMenuItem(nsContainerFrame* aParent,
                                        bool aIsPopup,
                                        bool aWrap)
 {
-  nsPresContext* presContext = aParent->PresContext();
-  auto insertion = presContext->PresShell()->
-    FrameConstructor()->GetInsertionPoint(aParent->GetContent(), nullptr);
-  nsContainerFrame* immediateParent = insertion.mParentFrame;
-  if (!immediateParent)
-    immediateParent = aParent;
-
+  nsContainerFrame* immediateParent = ImmediateParentFrame(aParent);
   const nsFrameList& frames(immediateParent->PrincipalChildList());
 
   nsIFrame* currFrame = nullptr;

layout/xul/nsXULPopupManager.h

@@ -363,6 +363,13 @@ public:
   // if a popup manager could not be allocated
   static nsXULPopupManager* GetInstance();
 
+  // Returns the immediate parent frame of inserted children of aFrame's
+  // content.
+  //
+  // FIXME(emilio): Or something like that, because this is kind of broken in a
+  // variety of situations like multiple insertion points.
+  static nsContainerFrame* ImmediateParentFrame(nsContainerFrame* aFrame);
+
   // This should be called when a window is moved or resized to adjust the
   // popups accordingly.
   void AdjustPopupsOnWindowChange(nsPIDOMWindowOuter* aWindow);