Bug 1484360 - Remove UNSET_DISPLAY in nsCSSFrameConstructor. r=emilio
authorTing-Yu Lin <tlin@mozilla.com>
Sat, 18 Aug 2018 14:07:24 +0000
changeset 487388 eef8c5edb73ca16553921ebaa7b56da79c5ed921
parent 487387 6190326e1b38861e9b85d2a1b48a68d82e1b41a3
child 487391 2d1d32212c1e4107bef4d24dbbe2f12adca11837
child 487392 15cca5b18ec422dbc5b7f893f07a7410ec18300f
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1484360
milestone63.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 1484360 - Remove UNSET_DISPLAY in nsCSSFrameConstructor. r=emilio Differential Revision: https://phabricator.services.mozilla.com/D3686
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -6235,79 +6235,79 @@ nsCSSFrameConstructor::AppendFramesToPar
     }
     return;
   }
 
   // Insert the frames after our aPrevSibling
   InsertFrames(aParentFrame, kPrincipalList, aPrevSibling, aFrameList);
 }
 
-#define UNSET_DISPLAY static_cast<StyleDisplay>(255)
-
 // This gets called to see if the frames corresponding to aSibling and aContent
 // should be siblings in the frame tree. Although (1) rows and cols, (2) row
 // groups and col groups, (3) row groups and captions, (4) legends and content
 // inside fieldsets, (5) popups and other kids of the menu are siblings from a
 // content perspective, they are not considered siblings in the frame tree.
 bool
-nsCSSFrameConstructor::IsValidSibling(nsIFrame*              aSibling,
-                                      nsIContent*            aContent,
-                                      StyleDisplay&          aDisplay)
+nsCSSFrameConstructor::IsValidSibling(nsIFrame* aSibling,
+                                      nsIContent* aContent,
+                                      Maybe<StyleDisplay>& aDisplay)
 {
   nsIFrame* parentFrame = aSibling->GetParent();
   LayoutFrameType parentType = parentFrame->Type();
 
   StyleDisplay siblingDisplay = aSibling->GetDisplay();
   if (StyleDisplay::TableColumnGroup == siblingDisplay ||
       StyleDisplay::TableColumn      == siblingDisplay ||
       StyleDisplay::TableCaption     == siblingDisplay ||
       StyleDisplay::TableHeaderGroup == siblingDisplay ||
       StyleDisplay::TableRowGroup    == siblingDisplay ||
       StyleDisplay::TableFooterGroup == siblingDisplay ||
       LayoutFrameType::Menu == parentType) {
     // if we haven't already, resolve a style to find the display type of
     // aContent.
-    if (UNSET_DISPLAY == aDisplay) {
+    if (aDisplay.isNothing()) {
       if (aContent->IsComment() || aContent->IsProcessingInstruction()) {
         // Comments and processing instructions never have frames, so we should
         // not try to generate styles for them.
         return false;
       }
       // FIXME(emilio): This is buggy some times, see bug 1424656.
       RefPtr<ComputedStyle> computedStyle = ResolveComputedStyle(aContent);
       const nsStyleDisplay* display = computedStyle->StyleDisplay();
-      aDisplay = display->mDisplay;
-    }
+      aDisplay.emplace(display->mDisplay);
+    }
+
+    StyleDisplay display = aDisplay.value();
     if (LayoutFrameType::Menu == parentType) {
       return
-        (StyleDisplay::MozPopup == aDisplay) ==
+        (StyleDisplay::MozPopup == display) ==
         (StyleDisplay::MozPopup == siblingDisplay);
     }
     // To have decent performance we want to return false in cases in which
     // reordering the two siblings has no effect on display.  To ensure
     // correctness, we MUST return false in cases where the two siblings have
     // the same desired parent type and live on different display lists.
     // Specificaly, columns and column groups should only consider columns and
     // column groups as valid siblings.  Captions should only consider other
     // captions.  All other things should consider each other as valid
     // siblings.  The restriction in the |if| above on siblingDisplay is ok,
     // because for correctness the only part that really needs to happen is to
     // not consider captions, column groups, and row/header/footer groups
     // siblings of each other.  Treating a column or colgroup as a valid
     // sibling of a non-table-related frame will just mean we end up reframing.
     if ((siblingDisplay == StyleDisplay::TableCaption) !=
-        (aDisplay == StyleDisplay::TableCaption)) {
+        (display == StyleDisplay::TableCaption)) {
       // One's a caption and the other is not.  Not valid siblings.
       return false;
     }
 
     if ((siblingDisplay == StyleDisplay::TableColumnGroup ||
          siblingDisplay == StyleDisplay::TableColumn) !=
-        (aDisplay == StyleDisplay::TableColumnGroup ||
-         aDisplay == StyleDisplay::TableColumn)) {
+        (display == StyleDisplay::TableColumnGroup ||
+         display == StyleDisplay::TableColumn)) {
       // One's a column or column group and the other is not.  Not valid
       // siblings.
       return false;
     }
     // Fall through; it's possible that the display type was overridden and
     // a different sort of frame was constructed, so we may need to return false
     // below.
   }
@@ -6331,17 +6331,17 @@ nsCSSFrameConstructor::IsValidSibling(ns
 // FIXME(emilio): If we ever kill IsValidSibling() we can simplify this quite a
 // bit (no need to pass aTargetContent or aTargetContentDisplay, and the
 // adjust() calls can be responsibility of the caller).
 template<nsCSSFrameConstructor::SiblingDirection aDirection>
 nsIFrame*
 nsCSSFrameConstructor::FindSiblingInternal(
   FlattenedChildIterator& aIter,
   nsIContent* aTargetContent,
-  StyleDisplay& aTargetContentDisplay)
+  Maybe<StyleDisplay>& aTargetContentDisplay)
 {
   auto adjust = [&](nsIFrame* aPotentialSiblingFrame) -> nsIFrame* {
     return AdjustSiblingFrame(
       aPotentialSiblingFrame, aTargetContent, aTargetContentDisplay,
       aDirection);
   };
 
   auto nextDomSibling = [](FlattenedChildIterator& aIter) -> nsIContent* {
@@ -6392,17 +6392,17 @@ nsCSSFrameConstructor::FindSiblingIntern
 
   return adjust(getFarPseudo(aIter.Parent()));
 }
 
 nsIFrame*
 nsCSSFrameConstructor::AdjustSiblingFrame(
   nsIFrame* aSibling,
   nsIContent* aTargetContent,
-  mozilla::StyleDisplay& aTargetContentDisplay,
+  Maybe<StyleDisplay>& aTargetContentDisplay,
   SiblingDirection aDirection)
 {
   if (!aSibling) {
     return nullptr;
   }
 
   if (aSibling->GetStateBits() & NS_FRAME_OUT_OF_FLOW) {
     aSibling = aSibling->GetPlaceholderFrame();
@@ -6426,32 +6426,32 @@ nsCSSFrameConstructor::AdjustSiblingFram
     return nullptr;
   }
 
   return aSibling;
 }
 
 nsIFrame*
 nsCSSFrameConstructor::FindPreviousSibling(const FlattenedChildIterator& aIter,
-                                           StyleDisplay& aTargetContentDisplay)
+                                           Maybe<StyleDisplay>& aTargetContentDisplay)
 {
   return FindSibling<SiblingDirection::Backward>(aIter, aTargetContentDisplay);
 }
 
 nsIFrame*
 nsCSSFrameConstructor::FindNextSibling(const FlattenedChildIterator& aIter,
-                                       StyleDisplay& aTargetContentDisplay)
+                                       Maybe<StyleDisplay>& aTargetContentDisplay)
 {
   return FindSibling<SiblingDirection::Forward>(aIter, aTargetContentDisplay);
 }
 
 template<nsCSSFrameConstructor::SiblingDirection aDirection>
 nsIFrame*
 nsCSSFrameConstructor::FindSibling(const FlattenedChildIterator& aIter,
-                                   StyleDisplay& aTargetContentDisplay)
+                                   Maybe<StyleDisplay>& aTargetContentDisplay)
 {
   nsIContent* targetContent = aIter.Get();
   FlattenedChildIterator siblingIter = aIter;
   nsIFrame* sibling = FindSiblingInternal<aDirection>(
     siblingIter, targetContent, aTargetContentDisplay);
   if (sibling) {
     return sibling;
   }
@@ -6530,17 +6530,17 @@ nsCSSFrameConstructor::GetInsertionPrevS
     iter.GetNextChild();
     MOZ_ASSERT(aChild->GetProperty(nsGkAtoms::restylableAnonymousNode),
                "Someone passed native anonymous content directly into frame "
                "construction.  Stop doing that!");
   }
 
   // Note that FindPreviousSibling is passed the iterator by value, so that
   // the later usage of the iterator starts from the same place.
-  StyleDisplay childDisplay = UNSET_DISPLAY;
+  Maybe<StyleDisplay> childDisplay;
   nsIFrame* prevSibling = FindPreviousSibling(iter, childDisplay);
 
   // Now, find the geometric parent so that we can handle
   // continuations properly. Use the prev sibling if we have it;
   // otherwise use the next sibling.
   if (prevSibling) {
     aInsertion->mParentFrame = prevSibling->GetParent()->GetContentInsertionFrame();
   } else {
@@ -6561,17 +6561,17 @@ nsCSSFrameConstructor::GetInsertionPrevS
 
       // Deal with fieldsets.
       aInsertion->mParentFrame =
         ::GetAdjustedParentFrame(aInsertion->mParentFrame, aChild);
       prevSibling = ::FindAppendPrevSibling(aInsertion->mParentFrame, nullptr);
     }
   }
 
-  *aIsRangeInsertSafe = (childDisplay == UNSET_DISPLAY);
+  *aIsRangeInsertSafe = childDisplay.isNothing();
   return prevSibling;
 }
 
 nsContainerFrame*
 nsCSSFrameConstructor::GetContentInsertionFrameFor(nsIContent* aContent)
 {
   nsIFrame* frame;
   while (!(frame = aContent->GetPrimaryFrame())) {
@@ -6928,17 +6928,17 @@ nsCSSFrameConstructor::StyleNewChildRang
 
 nsIFrame*
 nsCSSFrameConstructor::FindNextSiblingForAppend(const InsertionPoint& aInsertion)
 {
   auto SlowPath = [&]() -> nsIFrame* {
     FlattenedChildIterator iter(aInsertion.mContainer,
                                 /* aStartAtBeginning = */ false);
     iter.GetPreviousChild(); // Prime the iterator.
-    StyleDisplay unused = UNSET_DISPLAY;
+    Maybe<StyleDisplay> unused;
     return FindNextSibling(iter, unused);
   };
 
   if (!IsDisplayContents(aInsertion.mContainer) &&
       !nsLayoutUtils::GetAfterFrame(aInsertion.mContainer)) {
     MOZ_ASSERT(!SlowPath());
     return nullptr;
   }
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -10,16 +10,17 @@
  */
 
 #ifndef nsCSSFrameConstructor_h___
 #define nsCSSFrameConstructor_h___
 
 #include "mozilla/ArenaAllocator.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/LinkedList.h"
+#include "mozilla/Maybe.h"
 #include "mozilla/RestyleManager.h"
 #include "mozilla/ScrollStyles.h"
 
 #include "nsCOMPtr.h"
 #include "nsILayoutHistoryState.h"
 #include "nsQuoteList.h"
 #include "nsCounterManager.h"
 #include "nsIAnonymousContentCreator.h"
@@ -2057,46 +2058,45 @@ private:
    * necessary.
    *
    * aIter is passed by const reference on purpose, so as not to modify the
    * caller's iterator.
    *
    * @param aIter should be positioned such that aIter.GetPreviousChild()
    *          is the first content to search for frames
    * @param aTargetContentDisplay the CSS display enum for the content aIter
-   *          points to if already known, UNSET_DISPLAY otherwise. It will be
-   *          filled in if needed.
+   *          points to if already known. It will be filled in if needed.
    */
   template<SiblingDirection>
   nsIFrame* FindSibling(const mozilla::dom::FlattenedChildIterator& aIter,
-                        mozilla::StyleDisplay& aTargetContentDisplay);
+                        mozilla::Maybe<mozilla::StyleDisplay>& aTargetContentDisplay);
 
   // Helper for the implementation of FindSibling.
   //
   // Beware that this function does mutate the iterator.
   template<SiblingDirection>
   nsIFrame* FindSiblingInternal(
     mozilla::dom::FlattenedChildIterator&,
     nsIContent* aTargetContent,
-    mozilla::StyleDisplay& aTargetContentDisplay);
+    mozilla::Maybe<mozilla::StyleDisplay>& aTargetContentDisplay);
 
   // An alias of FindSibling<SiblingDirection::Forward>.
   nsIFrame* FindNextSibling(const mozilla::dom::FlattenedChildIterator& aIter,
-                            mozilla::StyleDisplay& aTargetContentDisplay);
+                            mozilla::Maybe<mozilla::StyleDisplay>& aTargetContentDisplay);
   // An alias of FindSibling<SiblingDirection::Backwards>.
   nsIFrame* FindPreviousSibling(const mozilla::dom::FlattenedChildIterator& aIter,
-                                mozilla::StyleDisplay& aTargetContentDisplay);
+                                mozilla::Maybe<mozilla::StyleDisplay>& aTargetContentDisplay);
 
   // Given a potential first-continuation sibling frame for aTargetContent,
   // verify that it is an actual valid sibling for it, and return the
   // appropriate continuation the new frame for aTargetContent should be
   // inserted next to.
   nsIFrame* AdjustSiblingFrame(nsIFrame* aSibling,
                                nsIContent* aTargetContent,
-                               mozilla::StyleDisplay& aTargetContentDisplay,
+                               mozilla::Maybe<mozilla::StyleDisplay>& aTargetContentDisplay,
                                SiblingDirection aDirection);
 
   // Find the right previous sibling for an insertion.  This also updates the
   // parent frame to point to the correct continuation of the parent frame to
   // use, and returns whether this insertion is to be treated as an append.
   // aChild is the child being inserted.
   // aIsRangeInsertSafe returns whether it is safe to do a range insert with
   // aChild being the first child in the range. It is the callers'
@@ -2115,17 +2115,17 @@ private:
                                     nsIContent *aEndSkipChild = nullptr);
 
   // 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
   // may be constructed based on tag, not based on aDisplay!
   bool IsValidSibling(nsIFrame*              aSibling,
                       nsIContent*            aContent,
-                      mozilla::StyleDisplay& aDisplay);
+                      mozilla::Maybe<mozilla::StyleDisplay>& aDisplay);
 
   void QuotesDirty();
   void CountersDirty();
 
   // Create touch caret frame.
   void ConstructAnonymousContentForCanvas(nsFrameConstructorState& aState,
                                           nsIFrame* aFrame,
                                           nsIContent* aDocElement);