Bug 812687 part 4: Add an optional parameter which can make CSSOrderAwareFrameIterator use the legacy "box-ordinal-group" property. r=mats
authorDaniel Holbert <dholbert@cs.stanford.edu>
Wed, 05 Apr 2017 19:31:47 -0700
changeset 351531 f6cc0dd3e7b8d90d0b1d29d5835a1bc4075211e4
parent 351530 947d5e737c2d05f038f01ddc2f02a01289db7ba8
child 351532 173a4f49dfe3db98a490a037db937cb06da5170d
push id31611
push usercbook@mozilla.com
push dateThu, 06 Apr 2017 10:51:05 +0000
treeherdermozilla-central@950612071c4e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs812687
milestone55.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 812687 part 4: Add an optional parameter which can make CSSOrderAwareFrameIterator use the legacy "box-ordinal-group" property. r=mats This patch just adds an optional codepath that isn't taken yet, so it shouldn't affect our behavior. (The next patch in the series will make use of this new codepath.) Note: the large code-comment that this patch adds is taken mostly-verbatim from some nsFlexContainerFrame.cpp code. (The original copy will be removed by the next patch in this series, when we switch to take advantage of this new mechanism.) MozReview-Commit-ID: 9pkJ346rrXg
layout/generic/CSSOrderAwareFrameIterator.cpp
layout/generic/CSSOrderAwareFrameIterator.h
--- a/layout/generic/CSSOrderAwareFrameIterator.cpp
+++ b/layout/generic/CSSOrderAwareFrameIterator.cpp
@@ -13,16 +13,22 @@ namespace mozilla {
 template<>
 bool
 CSSOrderAwareFrameIterator::CSSOrderComparator(nsIFrame* const& a,
                                                nsIFrame* const& b)
 { return a->StylePosition()->mOrder < b->StylePosition()->mOrder; }
 
 template<>
 bool
+CSSOrderAwareFrameIterator::CSSBoxOrdinalGroupComparator(nsIFrame* const& a,
+                                                         nsIFrame* const& b)
+{ return a->StyleXUL()->mBoxOrdinal < b->StyleXUL()->mBoxOrdinal; }
+
+template<>
+bool
 CSSOrderAwareFrameIterator::IsForward() const { return true; }
 
 template<>
 nsFrameList::iterator
 CSSOrderAwareFrameIterator::begin(const nsFrameList& aList)
 { return aList.begin(); }
 
 template<>
@@ -32,16 +38,22 @@ nsFrameList::iterator CSSOrderAwareFrame
 template<>
 bool
 ReverseCSSOrderAwareFrameIterator::CSSOrderComparator(nsIFrame* const& a,
                                                       nsIFrame* const& b)
 { return a->StylePosition()->mOrder > b->StylePosition()->mOrder; }
 
 template<>
 bool
+ReverseCSSOrderAwareFrameIterator::CSSBoxOrdinalGroupComparator(nsIFrame* const& a,
+                                                                nsIFrame* const& b)
+{ return a->StyleXUL()->mBoxOrdinal > b->StyleXUL()->mBoxOrdinal; }
+
+template<>
+bool
 ReverseCSSOrderAwareFrameIterator::IsForward() const
 { return false; }
 
 template<>
 nsFrameList::reverse_iterator
 ReverseCSSOrderAwareFrameIterator::begin(const nsFrameList& aList)
 { return aList.rbegin(); }
 
--- a/layout/generic/CSSOrderAwareFrameIterator.h
+++ b/layout/generic/CSSOrderAwareFrameIterator.h
@@ -22,54 +22,79 @@
 namespace mozilla {
 
 template<typename Iterator>
 class CSSOrderAwareFrameIteratorT
 {
 public:
   enum OrderState { eUnknownOrder, eKnownOrdered, eKnownUnordered };
   enum ChildFilter { eSkipPlaceholders, eIncludeAll };
+  enum OrderingProperty {
+    eUseOrder,          // Default behavior: use "order".
+    eUseBoxOrdinalGroup // Legacy behavior: use prefixed "box-ordinal-group".
+  };
   CSSOrderAwareFrameIteratorT(nsIFrame* aContainer,
                               nsIFrame::ChildListID aListID,
                               ChildFilter aFilter = eSkipPlaceholders,
-                              OrderState aState = eUnknownOrder)
+                              OrderState aState = eUnknownOrder,
+                              OrderingProperty aOrderProp = eUseOrder)
     : mChildren(aContainer->GetChildList(aListID))
     , mArrayIndex(0)
     , mItemIndex(0)
     , mSkipPlaceholders(aFilter == eSkipPlaceholders)
 #ifdef DEBUG
     , mContainer(aContainer)
     , mListID(aListID)
 #endif
   {
     size_t count = 0;
     bool isOrdered = aState != eKnownUnordered;
     if (aState == eUnknownOrder) {
       auto maxOrder = std::numeric_limits<int32_t>::min();
       for (auto child : mChildren) {
         ++count;
-        int32_t order = child->StylePosition()->mOrder;
+
+        int32_t order;
+        if (aOrderProp == eUseBoxOrdinalGroup) {
+          // We'll be using mBoxOrdinal, which has type uint32_t. However, the
+          // modern 'order' property (whose functionality we're co-opting) has
+          // type int32_t.  So: if we happen to have a uint32_t value that's
+          // greater than INT32_MAX, we clamp it rather than letting it
+          // overflow. Chances are, this is just an author using BIG_VALUE
+          // anyway, so the clamped value should be fine.
+          uint32_t clampedBoxOrdinal =
+            std::min(child->StyleXUL()->mBoxOrdinal,
+                     static_cast<uint32_t>(INT32_MAX));
+          order = static_cast<int32_t>(clampedBoxOrdinal);
+        } else {
+          order = child->StylePosition()->mOrder;
+        }
+
         if (order < maxOrder) {
           isOrdered = false;
           break;
         }
         maxOrder = order;
       }
     }
     if (isOrdered) {
       mIter.emplace(begin(mChildren));
       mIterEnd.emplace(end(mChildren));
     } else {
       count *= 2; // XXX somewhat arbitrary estimate for now...
       mArray.emplace(count);
       for (Iterator i(begin(mChildren)), iEnd(end(mChildren)); i != iEnd; ++i) {
         mArray->AppendElement(*i);
       }
+      auto comparator = (aOrderProp == eUseBoxOrdinalGroup)
+        ? CSSBoxOrdinalGroupComparator
+        : CSSOrderComparator;
+
       // XXX replace this with nsTArray::StableSort when bug 1147091 is fixed.
-      std::stable_sort(mArray->begin(), mArray->end(), CSSOrderComparator);
+      std::stable_sort(mArray->begin(), mArray->end(), comparator);
     }
 
     if (mSkipPlaceholders) {
       SkipPlaceholders();
     }
   }
   ~CSSOrderAwareFrameIteratorT()
   {
@@ -193,16 +218,17 @@ public:
     mIter.reset();
     mArray.reset();
     mozWritePoison(&mChildren, sizeof(mChildren));
   }
 
   bool ItemsAreAlreadyInOrder() const { return mIter.isSome(); }
 
   static bool CSSOrderComparator(nsIFrame* const& a, nsIFrame* const& b);
+  static bool CSSBoxOrdinalGroupComparator(nsIFrame* const& a, nsIFrame* const& b);
 private:
   nsFrameList mChildren;
   // Used if child list is already in ascending 'order'.
   Maybe<Iterator> mIter;
   Maybe<Iterator> mIterEnd;
   // Used if child list is *not* in ascending 'order'.
   // This array is pre-sorted in reverse order for a reverse iterator.
   Maybe<nsTArray<nsIFrame*>> mArray;