Bug 1479528: Move all the data-finding related stuff into a single function. r=dholbert
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 31 Jul 2018 14:44:45 +0200
changeset 826241 5947eab0392411a7d710225b0d2b635dcf550f0e
parent 826240 935ad68fc5db5270bd95b3f5ee37ad086b0b2626
child 826242 c84a0cd61b682b11c2a3a2b2adc5b87011241aa2
push id118275
push userbmo:dharvey@mozilla.com
push dateFri, 03 Aug 2018 11:44:33 +0000
reviewersdholbert
bugs1479528
milestone63.0a1
Bug 1479528: Move all the data-finding related stuff into a single function. r=dholbert To make it hopefully a bit easier to follow. Differential Revision: https://phabricator.services.mozilla.com/D2564 MozReview-Commit-ID: 2LMf7IXM1kr
dom/base/Text.h
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
--- a/dom/base/Text.h
+++ b/dom/base/Text.h
@@ -11,16 +11,18 @@
 #include "mozilla/ErrorResult.h"
 
 namespace mozilla {
 namespace dom {
 
 class Text : public CharacterData
 {
 public:
+  NS_IMPL_FROMNODE_HELPER(Text, IsText())
+
   explicit Text(already_AddRefed<mozilla::dom::NodeInfo>& aNodeInfo)
     : CharacterData(aNodeInfo)
   {}
 
   explicit Text(already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo)
     : CharacterData(aNodeInfo)
   {}
 
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -3400,31 +3400,30 @@ FindAncestorWithGeneratedContentPseudo(n
 }
 
 #define SIMPLE_FCDATA(_func) FCDATA_DECL(0, _func)
 #define FULL_CTOR_FCDATA(_flags, _func)                             \
   { _flags | FCDATA_FUNC_IS_FULL_CTOR, { nullptr }, _func, nullptr }
 
 /* static */
 const nsCSSFrameConstructor::FrameConstructionData*
-nsCSSFrameConstructor::FindTextData(nsIFrame* aParentFrame,
-                                    nsIContent* aTextContent)
-{
-  MOZ_ASSERT(aTextContent, "How?");
+nsCSSFrameConstructor::FindTextData(const Text& aTextContent,
+                                    nsIFrame* aParentFrame)
+{
   if (aParentFrame && IsFrameForSVG(aParentFrame)) {
     nsIFrame* ancestorFrame =
       nsSVGUtils::GetFirstNonAAncestorFrame(aParentFrame);
     if (!ancestorFrame || !nsSVGUtils::IsInSVGTextSubtree(ancestorFrame)) {
       return nullptr;
     }
 
     // Don't render stuff in display: contents / Shadow DOM subtrees, because
     // TextCorrespondenceRecorder in the SVG text code doesn't really know how
     // to deal with it. This kinda sucks. :(
-    if (aParentFrame->GetContent() != aTextContent->GetParent()) {
+    if (aParentFrame->GetContent() != aTextContent.GetParent()) {
       return nullptr;
     }
 
     static const FrameConstructionData sSVGTextData =
       FCDATA_DECL(FCDATA_IS_LINE_PARTICIPANT | FCDATA_IS_SVG_TEXT,
                   NS_NewTextFrame);
     return &sSVGTextData;
   }
@@ -3548,22 +3547,17 @@ IsFrameForFieldSet(nsIFrame* aFrame)
 }
 
 /* static */
 const nsCSSFrameConstructor::FrameConstructionData*
 nsCSSFrameConstructor::FindHTMLData(const Element& aElement,
                                     nsIFrame* aParentFrame,
                                     ComputedStyle& aStyle)
 {
-  // Ignore the tag if it's not HTML content and if it doesn't extend (via XBL)
-  // a valid HTML namespace.  This check must match the one in
-  // ShouldHaveFirstLineStyle.
-  if (!aElement.IsHTMLElement()) {
-    return nullptr;
-  }
+  MOZ_ASSERT(aElement.IsHTMLElement());
 
   nsAtom* tag = aElement.NodeInfo()->NameAtom();
   NS_ASSERTION(!aParentFrame ||
                aParentFrame->Style()->GetPseudo() !=
                  nsCSSAnonBoxes::fieldsetContent ||
                aParentFrame->GetParent()->IsFieldSetFrame(),
                "Unexpected parent for fieldset content anon box");
   if (tag == nsGkAtoms::legend &&
@@ -4190,22 +4184,19 @@ nsIFrame* NS_NewGridBoxFrame(nsIPresShel
   NS_NewGridLayout2(aPresShell, getter_AddRefs(layout));
   return NS_NewBoxFrame(aPresShell, aComputedStyle, false, layout);
 }
 
 /* static */
 const nsCSSFrameConstructor::FrameConstructionData*
 nsCSSFrameConstructor::FindXULTagData(const Element& aElement,
                                       nsAtom* aTag,
-                                      int32_t aNameSpaceID,
                                       ComputedStyle& aStyle)
 {
-  if (aNameSpaceID != kNameSpaceID_XUL) {
-    return nullptr;
-  }
+  MOZ_ASSERT(aElement.IsXULElement());
 
   static const FrameConstructionDataByTag sXULTagData[] = {
 #ifdef MOZ_XUL
     SCROLLABLE_XUL_CREATE(button, NS_NewButtonBoxFrame),
     SCROLLABLE_XUL_CREATE(thumb, NS_NewButtonBoxFrame),
     SCROLLABLE_XUL_CREATE(checkbox, NS_NewButtonBoxFrame),
     SCROLLABLE_XUL_CREATE(radio, NS_NewButtonBoxFrame),
     SCROLLABLE_XUL_CREATE(titlebar, NS_NewTitleBarFrame),
@@ -4923,20 +4914,17 @@ nsCSSFrameConstructor::FlushAccumulatedB
                   FCDATA_FORCE_NULL_ABSPOS_CONTAINER |                  \
                   FCDATA_WRAP_KIDS_IN_BLOCKS, _func) }
 
 /* static */
 const nsCSSFrameConstructor::FrameConstructionData*
 nsCSSFrameConstructor::FindMathMLData(const Element& aElement,
                                       ComputedStyle& aStyle)
 {
-  // Make sure that we remain confined in the MathML world
-  if (!aElement.IsMathMLElement()) {
-    return nullptr;
-  }
+  MOZ_ASSERT(aElement.IsMathMLElement());
 
   nsAtom* tag = aElement.NodeInfo()->NameAtom();
 
   // Handle <math> specially, because it sometimes produces inlines
   if (tag == nsGkAtoms::math) {
     // This needs to match the test in EnsureBlockDisplay in
     // nsRuleNode.cpp.  Though the behavior here for the display:table
     // case is pretty weird...
@@ -5111,19 +5099,17 @@ IsFilterPrimitiveChildTag(const nsAtom* 
 /* static */
 const nsCSSFrameConstructor::FrameConstructionData*
 nsCSSFrameConstructor::FindSVGData(const Element& aElement,
                                    nsIFrame* aParentFrame,
                                    bool aIsWithinSVGText,
                                    bool aAllowsTextPathChild,
                                    ComputedStyle& aStyle)
 {
-  if (!aElement.IsSVGElement()) {
-    return nullptr;
-  }
+  MOZ_ASSERT(aElement.IsSVGElement());
 
   static const FrameConstructionData sSuppressData = SUPPRESS_FCDATA();
   static const FrameConstructionData sContainerData =
     SIMPLE_SVG_FCDATA(NS_NewSVGContainerFrame);
 
   bool parentIsSVG = aIsWithinSVGText;
   nsIContent* parentContent =
     aParentFrame ? aParentFrame->GetContent() : nullptr;
@@ -5488,16 +5474,101 @@ ShouldSuppressFrameInNonOpenDetails(cons
       !aChild.IsGeneratedContentContainerForBefore() &&
       !aChild.IsGeneratedContentContainerForAfter()) {
     return false;
   }
 
   return true;
 }
 
+const nsCSSFrameConstructor::FrameConstructionData*
+nsCSSFrameConstructor::FindDataForContent(nsIContent& aContent,
+                                          ComputedStyle& aStyle,
+                                          nsIFrame* aParentFrame,
+                                          nsAtom* aTag,
+                                          uint32_t aFlags)
+{
+  MOZ_ASSERT(aStyle.StyleDisplay()->mDisplay != StyleDisplay::None &&
+             aStyle.StyleDisplay()->mDisplay != StyleDisplay::Contents,
+             "These two special display values should be handled earlier");
+
+  if (auto* text = Text::FromNode(aContent)) {
+    return FindTextData(*text, aParentFrame);
+  }
+
+  return FindElementData(*aContent.AsElement(),
+                         aStyle,
+                         aParentFrame,
+                         aTag,
+                         aFlags);
+}
+
+const nsCSSFrameConstructor::FrameConstructionData*
+nsCSSFrameConstructor::FindElementData(const Element& aElement,
+                                       ComputedStyle& aStyle,
+                                       nsIFrame* aParentFrame,
+                                       nsAtom* aTag,
+                                       uint32_t aFlags)
+{
+  // Don't create frames for non-SVG element children of SVG elements.
+  if (!aElement.IsSVGElement()) {
+    if (aParentFrame && IsFrameForSVG(aParentFrame) &&
+        !aParentFrame->IsFrameOfType(nsIFrame::eSVGForeignObject)) {
+      return nullptr;
+    }
+    if (aFlags & ITEM_IS_WITHIN_SVG_TEXT) {
+      return nullptr;
+    }
+  }
+
+  if (auto* data = FindElementTagData(aElement, aStyle, aParentFrame, aTag, aFlags)) {
+    return data;
+  }
+
+  // Check for 'content: <image-url>' on the element (which makes us ignore
+  // 'display' values other than 'none' or 'contents').
+  if (ShouldCreateImageFrameForContent(aElement, aStyle)) {
+    static const FrameConstructionData sImgData =
+      SIMPLE_FCDATA(NS_NewImageFrameForContentProperty);
+    return &sImgData;
+  }
+
+  const auto& display = *aStyle.StyleDisplay();
+  if (auto* data = FindXULDisplayData(display, aElement)) {
+    return data;
+  }
+
+  return FindDisplayData(display, aElement);
+}
+
+const nsCSSFrameConstructor::FrameConstructionData*
+nsCSSFrameConstructor::FindElementTagData(const Element& aElement,
+                                          ComputedStyle& aStyle,
+                                          nsIFrame* aParentFrame,
+                                          nsAtom* aTag,
+                                          uint32_t aFlags)
+{
+  switch (aElement.GetNameSpaceID()) {
+    case kNameSpaceID_XHTML:
+      return FindHTMLData(aElement, aParentFrame, aStyle);
+    case kNameSpaceID_MathML:
+      return FindMathMLData(aElement, aStyle);
+    case kNameSpaceID_SVG:
+      return FindSVGData(aElement,
+                         aParentFrame,
+                         aFlags & ITEM_IS_WITHIN_SVG_TEXT,
+                         aFlags & ITEM_ALLOWS_TEXT_PATH_CHILD,
+                         aStyle);
+    case kNameSpaceID_XUL:
+      return FindXULTagData(aElement, aTag, aStyle);
+    default:
+      return nullptr;
+  }
+}
+
 nsCSSFrameConstructor::XBLBindingLoadInfo::XBLBindingLoadInfo(
   already_AddRefed<ComputedStyle> aStyle,
   mozilla::UniquePtr<PendingBinding> aPendingBinding,
   nsAtom* aTag)
   : mStyle(aStyle)
   , mPendingBinding(std::move(aPendingBinding))
   , mTag(aTag)
 {
@@ -5656,90 +5727,36 @@ nsCSSFrameConstructor::AddFrameConstruct
   // to things that are not roots of native anonymous subtrees (except for
   // ::before and ::after); we always want to create "internal" anonymous
   // content.
   auto* details = HTMLDetailsElement::FromNodeOrNull(parent);
   if (ShouldSuppressFrameInNonOpenDetails(details, *aContent)) {
     return;
   }
 
+  const FrameConstructionData* data =
+    FindDataForContent(*aContent, *style, aParentFrame, tag, aFlags);
+  if (!data || data->mBits & FCDATA_SUPPRESS_FRAME) {
+    return;
+  }
+
   bool isPopup = false;
-  const bool isText = !aContent->IsElement();
-  // Try to find frame construction data for this content
-  const FrameConstructionData* data;
-  if (isText) {
-    data = FindTextData(aParentFrame, aContent);
-    if (!data) {
-      // Nothing to do here; suppressed text inside SVG
-      return;
-    }
-  } else {
-    Element& element = *aContent->AsElement();
-
-    // Don't create frames for non-SVG element children of SVG elements.
-    if (namespaceId != kNameSpaceID_SVG &&
-        ((aParentFrame &&
-          IsFrameForSVG(aParentFrame) &&
-          !aParentFrame->IsFrameOfType(nsIFrame::eSVGForeignObject)) ||
-         (aFlags & ITEM_IS_WITHIN_SVG_TEXT))) {
-      return;
-    }
-
-    data = FindHTMLData(element, aParentFrame, *style);
-    if (!data) {
-      data = FindXULTagData(element, tag, namespaceId, *style);
-    }
-    if (!data) {
-      data = FindMathMLData(element, *style);
-    }
-    if (!data) {
-      data = FindSVGData(element,
-                         aParentFrame,
-                         aFlags & ITEM_IS_WITHIN_SVG_TEXT,
-                         aFlags & ITEM_ALLOWS_TEXT_PATH_CHILD,
-                         *style);
-    }
-
-    // Check for 'content: <image-url>' on the element (which makes us ignore
-    // 'display' values other than 'none' or 'contents').
-    if (!data && ShouldCreateImageFrameForContent(element, *style)) {
-      static const FrameConstructionData sImgData =
-        SIMPLE_FCDATA(NS_NewImageFrameForContentProperty);
-      data = &sImgData;
-    }
-
-    // Now check for XUL display types
-    if (!data) {
-      data = FindXULDisplayData(display, element);
-    }
-
-    // And general display types
-    if (!data) {
-      data = FindDisplayData(display, element);
-    }
-
-    MOZ_ASSERT(data, "Should have frame construction data now");
-
-    if (data->mBits & FCDATA_SUPPRESS_FRAME) {
-      return;
-    }
 
 #ifdef MOZ_XUL
-    if ((data->mBits & FCDATA_IS_POPUP) &&
-        (!aParentFrame || // Parent is inline
-         !aParentFrame->IsMenuFrame())) {
-      if (!aState.mPopupItems.containingBlock &&
-          !aState.mHavePendingPopupgroup) {
-        return;
-      }
-
-      isPopup = true;
-    }
+  if ((data->mBits & FCDATA_IS_POPUP) &&
+      (!aParentFrame || // Parent is inline
+       !aParentFrame->IsMenuFrame())) {
+    if (!aState.mPopupItems.containingBlock &&
+        !aState.mHavePendingPopupgroup) {
+      return;
+    }
+
+    isPopup = true;
+  }
 #endif /* MOZ_XUL */
-  }
 
   uint32_t bits = data->mBits;
 
   // Inside colgroups, suppress everything except columns.
   if (aParentFrame && aParentFrame->IsTableColGroupFrame() &&
       (!(bits & FCDATA_IS_TABLE_PART) ||
        display.mDisplay != StyleDisplay::TableColumn)) {
     return;
@@ -5766,34 +5783,34 @@ nsCSSFrameConstructor::AddFrameConstruct
     }
   }
 
   if (!item) {
     item = aItems.AppendItem(this, data, aContent, pendingBinding,
                              style.forget(),
                              aSuppressWhiteSpaceOptimizations);
   }
-  item->mIsText = isText;
+  item->mIsText = !aContent->IsElement();
   item->mIsGeneratedContent = isGeneratedContent;
   item->mIsAnonymousContentCreatorContent =
     aFlags & ITEM_IS_ANONYMOUSCONTENTCREATOR_CONTENT;
   if (isGeneratedContent) {
     // We need to keep this alive until the frame takes ownership.
     // This corresponds to the Release in ConstructFramesFromItem.
     item->mContent->AddRef();
   }
   item->mIsRootPopupgroup =
-    namespaceId == kNameSpaceID_XUL && tag == nsGkAtoms::popupgroup &&
-    aContent->IsRootOfNativeAnonymousSubtree();
+    aContent->IsRootOfNativeAnonymousSubtree() &&
+    aContent->IsXULElement() &&
+    tag == nsGkAtoms::popupgroup;
   if (item->mIsRootPopupgroup) {
     aState.mHavePendingPopupgroup = true;
   }
   item->mIsPopup = isPopup;
-  item->mIsForSVGAElement = namespaceId == kNameSpaceID_SVG &&
-                            tag == nsGkAtoms::a;
+  item->mIsForSVGAElement = aContent->IsSVGElement(nsGkAtoms::a);
 
   if (canHavePageBreak && display.mBreakAfter) {
     AddPageBreakItem(aContent, aItems);
   }
 
   if (bits & FCDATA_IS_INLINE) {
     // To correctly set item->mIsAllInline we need to build up our child items
     // right now.
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -41,27 +41,29 @@ class nsFrameConstructorState;
 
 namespace mozilla {
 
 class ComputedStyle;
 
 namespace dom {
 
 class CharacterData;
+class Text;
 class FlattenedChildIterator;
 
 } // namespace dom
 } // namespace mozilla
 
 class nsCSSFrameConstructor final : public nsFrameManager
 {
 public:
   typedef mozilla::ComputedStyle ComputedStyle;
   typedef mozilla::CSSPseudoElementType CSSPseudoElementType;
   typedef mozilla::dom::Element Element;
+  typedef mozilla::dom::Text Text;
 
   // FIXME(emilio): Is this really needed?
   friend class mozilla::RestyleManager;
 
   nsCSSFrameConstructor(nsIDocument* aDocument, nsIPresShell* aPresShell);
   ~nsCSSFrameConstructor() {
     MOZ_ASSERT(mFCItemsInUse == 0);
   }
@@ -803,16 +805,36 @@ private:
     XBLBindingLoadInfo();
   };
 
   // Returns null mStyle / mTag members to signal an error.
   XBLBindingLoadInfo LoadXBLBindingIfNeeded(nsIContent&,
                                             ComputedStyle&,
                                             uint32_t aFlags);
 
+  const FrameConstructionData* FindDataForContent(nsIContent&,
+                                                  ComputedStyle&,
+                                                  nsIFrame* aParentFrame,
+                                                  nsAtom* aTag,
+                                                  uint32_t aFlags);
+
+  // aParentFrame might be null.  If it is, that means it was an inline frame.
+  static const FrameConstructionData* FindTextData(const Text&,
+                                                   nsIFrame* aParentFrame);
+  const FrameConstructionData* FindElementData(const Element&,
+                                               ComputedStyle&,
+                                               nsIFrame* aParentFrame,
+                                               nsAtom* aTag,
+                                               uint32_t aFlags);
+  const FrameConstructionData* FindElementTagData(const Element&,
+                                                  ComputedStyle&,
+                                                  nsIFrame* aParentFrame,
+                                                  nsAtom* aTag,
+                                                  uint32_t aFlags);
+
   /* A function that takes an integer, content, style, and array of
      FrameConstructionDataByInts and finds the appropriate frame construction
      data to use and returns it.  This can return null if none of the integers
      match or if the matching integer has a FrameConstructionDataGetter that
      returns null. */
   static const FrameConstructionData*
     FindDataByInt(int32_t aInt,
                   const Element&,
@@ -1408,21 +1430,16 @@ private:
   // ConstructDetailsFrame puts the new frame in aFrameItems and
   // handles the kids of the details.
   nsIFrame* ConstructDetailsFrame(nsFrameConstructorState& aState,
                                   FrameConstructionItem& aItem,
                                   nsContainerFrame* aParentFrame,
                                   const nsStyleDisplay* aStyleDisplay,
                                   nsFrameItems& aFrameItems);
 
-  // aParentFrame might be null.  If it is, that means it was an
-  // inline frame.
-  static const FrameConstructionData* FindTextData(nsIFrame* aParentFrame,
-                                                   nsIContent* aTextContent);
-
   void ConstructTextFrame(const FrameConstructionData* aData,
                           nsFrameConstructorState& aState,
                           nsIContent*              aContent,
                           nsContainerFrame*        aParentFrame,
                           ComputedStyle*          aComputedStyle,
                           nsFrameItems&            aFrameItems);
 
   // If aPossibleTextContent is a text node and doesn't have a frame, append a
@@ -1532,17 +1549,16 @@ private:
 
   // Function to find FrameConstructionData for an element.  Will return
   // null if the element is not XUL.
   //
   // NOTE(emilio): This gets the overloaded tag and namespace id since they can
   // be overriden by extends="" in XBL.
   static const FrameConstructionData* FindXULTagData(const Element&,
                                                      nsAtom* aTag,
-                                                     int32_t aNameSpaceID,
                                                      ComputedStyle&);
   // XUL data-finding helper functions and structures
 #ifdef MOZ_XUL
   static const FrameConstructionData* FindPopupGroupData(const Element&, ComputedStyle&);
   // sXULTextBoxData used for both labels and descriptions
   static const FrameConstructionData sXULTextBoxData;
   static const FrameConstructionData* FindXULLabelData(const Element&, ComputedStyle&);
   static const FrameConstructionData* FindXULDescriptionData(const Element&, ComputedStyle&);