Bug 1309445 - Convert FrameConstructionItemList::mItems to use mozilla::LinkedList. draft
authorTing-Yu Lin <tlin@mozilla.com>
Tue, 18 Oct 2016 15:50:27 +0800
changeset 427316 52f527bd1524363e43e47e65cf2c9613b6c23ede
parent 426483 01ab78dd98805e150b0311cce2351d5b408f3001
child 427422 201a5a770addea749b4dfab105676640e7e81496
push id32977
push userbmo:tlin@mozilla.com
push dateThu, 20 Oct 2016 05:03:44 +0000
bugs1309445
milestone52.0a1
Bug 1309445 - Convert FrameConstructionItemList::mItems to use mozilla::LinkedList. The major change to the Iterator is due to the fact that the end of a LinkedList is represented by nullptr. Also delete the type conversion functions which are no longer needed. MozReview-Commit-ID: 2lYtFW9pSon
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -12780,47 +12780,49 @@ Iterator::SkipWhitespace(nsFrameConstruc
 
 void
 nsCSSFrameConstructor::FrameConstructionItemList::
 Iterator::AppendItemToList(FrameConstructionItemList& aTargetList)
 {
   NS_ASSERTION(&aTargetList != &mList, "Unexpected call");
   NS_PRECONDITION(!IsDone(), "should not be done");
 
-  FrameConstructionItem* item = ToItem(mCurrent);
+  FrameConstructionItem* item = mCurrent;
   Next();
-  PR_REMOVE_LINK(item);
-  PR_APPEND_LINK(item, &aTargetList.mItems);
+  item->remove();
+  aTargetList.mItems.insertBack(item);
 
   mList.AdjustCountsForItem(item, -1);
   aTargetList.AdjustCountsForItem(item, 1);
 }
 
 void
 nsCSSFrameConstructor::FrameConstructionItemList::
 Iterator::AppendItemsToList(const Iterator& aEnd,
                             FrameConstructionItemList& aTargetList)
 {
   NS_ASSERTION(&aTargetList != &mList, "Unexpected call");
-  NS_PRECONDITION(mEnd == aEnd.mEnd, "end iterator for some other list?");
+  NS_PRECONDITION(&mList == &aEnd.mList, "End iterator for some other list?");
 
   // We can't just move our guts to the other list if it already has
   // some information or if we're not moving our entire list.
   if (!AtStart() || !aEnd.IsDone() || !aTargetList.IsEmpty() ||
       !aTargetList.mUndisplayedItems.IsEmpty()) {
     do {
       AppendItemToList(aTargetList);
     } while (*this != aEnd);
     return;
   }
 
-  // move over the list of items
-  PR_INSERT_AFTER(&aTargetList.mItems, &mList.mItems);
-  // Need to init when we remove to makd ~FrameConstructionItemList work right.
-  PR_REMOVE_AND_INIT_LINK(&mList.mItems);
+  // Move our entire list of items into the empty target list.
+  // XXX: If LinkedList supports move assignment, we could use
+  // aTargetList.mItems = Move(mList.mItems);
+  aTargetList.mItems.~LinkedList<FrameConstructionItem>();
+  new (&aTargetList.mItems) LinkedList<FrameConstructionItem>(
+    Move(mList.mItems));
 
   // Copy over the various counters
   aTargetList.mInlineCount = mList.mInlineCount;
   aTargetList.mBlockCount = mList.mBlockCount;
   aTargetList.mLineParticipantCount = mList.mLineParticipantCount;
   aTargetList.mItemCount = mList.mItemCount;
   memcpy(aTargetList.mDesiredParentCounts, mList.mDesiredParentCounts,
          sizeof(aTargetList.mDesiredParentCounts));
@@ -12828,39 +12830,43 @@ Iterator::AppendItemsToList(const Iterat
   // Swap out undisplayed item arrays, before we nuke the array on our end
   aTargetList.mUndisplayedItems.SwapElements(mList.mUndisplayedItems);
 
   // reset mList
   mList.~FrameConstructionItemList();
   new (&mList) FrameConstructionItemList();
 
   // Point ourselves to aEnd, as advertised
-  mCurrent = mEnd = &mList.mItems;
+  SetToEnd();
   NS_POSTCONDITION(*this == aEnd, "How did that happen?");
 }
 
 void
 nsCSSFrameConstructor::FrameConstructionItemList::
 Iterator::InsertItem(FrameConstructionItem* aItem)
 {
-  // Just insert the item before us.  There's no magic here.
-  PR_INSERT_BEFORE(aItem, mCurrent);
+  if (IsDone()) {
+    mList.mItems.insertBack(aItem);
+  } else {
+    // Just insert the item before us.  There's no magic here.
+    mCurrent->setPrevious(aItem);
+  }
   mList.AdjustCountsForItem(aItem, 1);
 
-  NS_POSTCONDITION(PR_NEXT_LINK(aItem) == mCurrent, "How did that happen?");
+  NS_POSTCONDITION(aItem->getNext() == mCurrent, "How did that happen?");
 }
 
 void
 nsCSSFrameConstructor::FrameConstructionItemList::
 Iterator::DeleteItemsTo(const Iterator& aEnd)
 {
-  NS_PRECONDITION(mEnd == aEnd.mEnd, "end iterator for some other list?");
+  NS_PRECONDITION(&mList == &aEnd.mList, "End iterator for some other list?");
   NS_PRECONDITION(*this != aEnd, "Shouldn't be at aEnd yet");
 
   do {
     NS_ASSERTION(!IsDone(), "Ran off end of list?");
-    FrameConstructionItem* item = ToItem(mCurrent);
+    FrameConstructionItem* item = mCurrent;
     Next();
-    PR_REMOVE_LINK(item);
+    item->remove();
     mList.AdjustCountsForItem(item, -1);
     delete item;
   } while (*this != aEnd);
 }
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -7,16 +7,17 @@
  * construction of a frame tree that is nearly isomorphic to the content
  * tree and updating of that tree in response to dynamic changes
  */
 
 #ifndef nsCSSFrameConstructor_h___
 #define nsCSSFrameConstructor_h___
 
 #include "mozilla/Attributes.h"
+#include "mozilla/LinkedList.h"
 #include "mozilla/RestyleManagerBase.h"
 #include "mozilla/RestyleManagerHandle.h"
 
 #include "nsCOMPtr.h"
 #include "nsILayoutHistoryState.h"
 #include "nsQuoteList.h"
 #include "nsCounterManager.h"
 #include "nsIAnonymousContentCreator.h"
@@ -780,43 +781,36 @@ private:
      returns null. */
   static const FrameConstructionData*
     FindDataByTag(nsIAtom* aTag, Element* aElement,
                   nsStyleContext* aStyleContext,
                   const FrameConstructionDataByTag* aDataPtr,
                   uint32_t aDataLength);
 
   /* A class representing a list of FrameConstructionItems */
-  class FrameConstructionItemList {
+  class FrameConstructionItemList final {
   public:
     FrameConstructionItemList() :
       mInlineCount(0),
       mBlockCount(0),
       mLineParticipantCount(0),
       mItemCount(0),
       mLineBoundaryAtStart(false),
       mLineBoundaryAtEnd(false),
       mParentHasNoXBLChildren(false),
       mTriedConstructingFrames(false)
     {
-      PR_INIT_CLIST(&mItems);
       memset(mDesiredParentCounts, 0, sizeof(mDesiredParentCounts));
     }
 
     ~FrameConstructionItemList() {
-      PRCList* cur = PR_NEXT_LINK(&mItems);
-      while (cur != &mItems) {
-        PRCList* next = PR_NEXT_LINK(cur);
-        delete ToItem(cur);
-        cur = next;
+      while (FrameConstructionItem* item = mItems.popFirst()) {
+        delete item;
       }
 
-      // Leaves our mItems pointing to deleted memory in both directions,
-      // but that's OK at this point.
-
       // Create the undisplayed entries for our mUndisplayedItems, if any, but
       // only if we have tried constructing frames for this item list.  If we
       // haven't, then we're just throwing it away and will probably try again.
       if (!mUndisplayedItems.IsEmpty() && mTriedConstructingFrames) {
         // We could store the frame manager in a member, but just
         // getting it off the style context is not too bad.
         nsFrameManager *mgr =
           mUndisplayedItems[0].mStyleContext->PresContext()->FrameManager();
@@ -831,17 +825,17 @@ private:
     void SetLineBoundaryAtEnd(bool aBoundary) { mLineBoundaryAtEnd = aBoundary; }
     void SetParentHasNoXBLChildren(bool aHasNoXBLChildren) {
       mParentHasNoXBLChildren = aHasNoXBLChildren;
     }
     void SetTriedConstructingFrames() { mTriedConstructingFrames = true; }
     bool HasLineBoundaryAtStart() { return mLineBoundaryAtStart; }
     bool HasLineBoundaryAtEnd() { return mLineBoundaryAtEnd; }
     bool ParentHasNoXBLChildren() { return mParentHasNoXBLChildren; }
-    bool IsEmpty() const { return PR_CLIST_IS_EMPTY(&mItems); }
+    bool IsEmpty() const { return mItems.isEmpty(); }
     bool AnyItemsNeedBlockParent() const { return mLineParticipantCount != 0; }
     bool AreAllItemsInline() const { return mInlineCount == mItemCount; }
     bool AreAllItemsBlock() const { return mBlockCount == mItemCount; }
     bool AllWantParentType(ParentType aDesiredParentType) const {
       return mDesiredParentCounts[aDesiredParentType] == mItemCount;
     }
 
     // aSuppressWhiteSpaceOptimizations is true if optimizations that
@@ -857,17 +851,17 @@ private:
                                       bool aSuppressWhiteSpaceOptimizations,
                                       nsTArray<nsIAnonymousContentCreator::ContentInfo>* aAnonChildren)
     {
       FrameConstructionItem* item =
         new FrameConstructionItem(aFCData, aContent, aTag, aNameSpaceID,
                                   aPendingBinding, aStyleContext,
                                   aSuppressWhiteSpaceOptimizations,
                                   aAnonChildren);
-      PR_APPEND_LINK(item, &mItems);
+      mItems.insertBack(item);
       ++mItemCount;
       ++mDesiredParentCounts[item->DesiredParentType()];
       return item;
     }
 
     // Arguments are the same as AppendItem().
     FrameConstructionItem* PrependItem(const FrameConstructionData* aFCData,
                                        nsIContent* aContent,
@@ -878,89 +872,80 @@ private:
                                        bool aSuppressWhiteSpaceOptimizations,
                                        nsTArray<nsIAnonymousContentCreator::ContentInfo>* aAnonChildren)
     {
       FrameConstructionItem* item =
         new FrameConstructionItem(aFCData, aContent, aTag, aNameSpaceID,
                                   aPendingBinding, aStyleContext,
                                   aSuppressWhiteSpaceOptimizations,
                                   aAnonChildren);
-      PR_INSERT_LINK(item, &mItems);
+      mItems.insertFront(item);
       ++mItemCount;
       ++mDesiredParentCounts[item->DesiredParentType()];
       return item;
     }
 
     void AppendUndisplayedItem(nsIContent* aContent,
                                nsStyleContext* aStyleContext) {
       mUndisplayedItems.AppendElement(UndisplayedItem(aContent, aStyleContext));
     }
 
     void InlineItemAdded() { ++mInlineCount; }
     void BlockItemAdded() { ++mBlockCount; }
     void LineParticipantItemAdded() { ++mLineParticipantCount; }
 
-    class Iterator;
-    friend class Iterator;
-
     class Iterator {
     public:
-      explicit Iterator(FrameConstructionItemList& list) :
-        mCurrent(PR_NEXT_LINK(&list.mItems)),
-        mEnd(&list.mItems),
-        mList(list)
+      explicit Iterator(FrameConstructionItemList& aList)
+        : mCurrent(aList.mItems.getFirst())
+        , mList(aList)
       {}
       Iterator(const Iterator& aOther) :
         mCurrent(aOther.mCurrent),
-        mEnd(aOther.mEnd),
         mList(aOther.mList)
       {}
 
       bool operator==(const Iterator& aOther) const {
-        NS_ASSERTION(mEnd == aOther.mEnd, "Iterators for different lists?");
+        MOZ_ASSERT(&mList == &aOther.mList, "Iterators for different lists?");
         return mCurrent == aOther.mCurrent;
       }
       bool operator!=(const Iterator& aOther) const {
         return !(*this == aOther);
       }
       Iterator& operator=(const Iterator& aOther) {
-        NS_ASSERTION(mEnd == aOther.mEnd, "Iterators for different lists?");
+        MOZ_ASSERT(&mList == &aOther.mList, "Iterators for different lists?");
         mCurrent = aOther.mCurrent;
         return *this;
       }
 
       FrameConstructionItemList* List() {
         return &mList;
       }
 
-      operator FrameConstructionItem& () {
-        return item();
-      }
-
       FrameConstructionItem& item() {
         MOZ_ASSERT(!IsDone(), "Should have checked IsDone()!");
-        return *FrameConstructionItemList::ToItem(mCurrent);
+        return *mCurrent;
       }
 
       const FrameConstructionItem& item() const {
         MOZ_ASSERT(!IsDone(), "Should have checked IsDone()!");
-        return *FrameConstructionItemList::ToItem(mCurrent);
+        return *mCurrent;
       }
 
-      bool IsDone() const { return mCurrent == mEnd; }
-      bool AtStart() const { return mCurrent == PR_NEXT_LINK(mEnd); }
+      bool IsDone() const { return mCurrent == nullptr; }
+      bool AtStart() const { return mCurrent == mList.mItems.getFirst(); }
       void Next() {
         NS_ASSERTION(!IsDone(), "Should have checked IsDone()!");
-        mCurrent = PR_NEXT_LINK(mCurrent);
+        mCurrent = mCurrent->getNext();
       }
       void Prev() {
         NS_ASSERTION(!AtStart(), "Should have checked AtStart()!");
-        mCurrent = PR_PREV_LINK(mCurrent);
+        mCurrent = mCurrent ? mCurrent->getPrevious() : mList.mItems.getLast();
       }
-      void SetToEnd() { mCurrent = mEnd; }
+      void SetToEnd() { mCurrent = nullptr; }
 
       // Skip over all items that want the given parent type. Return whether
       // the iterator is done after doing that.  The iterator must not be done
       // when this is called.
       inline bool SkipItemsWantingParentType(ParentType aParentType);
 
       // Skip over all items that want a parent type different from the given
       // one.  Return whether the iterator is done after doing that.  The
@@ -988,17 +973,17 @@ private:
 
       // Skip over whitespace.  Return whether the iterator is done after doing
       // that.  The iterator must not be done, and must be pointing to a
       // whitespace item when this is called.
       inline bool SkipWhitespace(nsFrameConstructorState& aState);
 
       // Remove the item pointed to by this iterator from its current list and
       // Append it to aTargetList.  This iterator is advanced to point to the
-      // next item in its list.  aIter must not be done.  aOther must not be
+      // next item in its list.  aIter must not be done.  aTargetList must not be
       // the list this iterator is iterating over..
       void AppendItemToList(FrameConstructionItemList& aTargetList);
 
       // As above, but moves all items starting with this iterator until we
       // get to aEnd; the item pointed to by aEnd is not stolen.  This method
       // might have optimizations over just looping and doing StealItem for
       // some special cases.  After this method returns, this iterator will
       // point to the item aEnd points to now; aEnd is not modified.
@@ -1016,40 +1001,35 @@ private:
       // Delete the items between this iterator and aEnd, including the item
       // this iterator currently points to but not including the item pointed
       // to by aEnd.  When this returns, this iterator will point to the same
       // item as aEnd.  This iterator must not equal aEnd when this method is
       // called.
       void DeleteItemsTo(const Iterator& aEnd);
 
     private:
-      PRCList* mCurrent;
-      PRCList* mEnd;
+      FrameConstructionItem* mCurrent;
       FrameConstructionItemList& mList;
     };
 
   private:
-    static FrameConstructionItem* ToItem(PRCList* item) {
-      return static_cast<FrameConstructionItem*>(item);
-    }
-
     struct UndisplayedItem {
       UndisplayedItem(nsIContent* aContent, nsStyleContext* aStyleContext) :
         mContent(aContent), mStyleContext(aStyleContext)
       {}
 
       nsIContent * const mContent;
       RefPtr<nsStyleContext> mStyleContext;
     };
 
     // Adjust our various counts for aItem being added or removed.  aDelta
     // should be either +1 or -1 depending on which is happening.
     void AdjustCountsForItem(FrameConstructionItem* aItem, int32_t aDelta);
 
-    PRCList mItems;
+    mozilla::LinkedList<FrameConstructionItem> mItems;
     uint32_t mInlineCount;
     uint32_t mBlockCount;
     uint32_t mLineParticipantCount;
     uint32_t mItemCount;
     uint32_t mDesiredParentCounts[eParentTypeCount];
     // True if there is guaranteed to be a line boundary before the
     // frames created by these items
     bool mLineBoundaryAtStart;
@@ -1065,19 +1045,18 @@ private:
   };
 
   typedef FrameConstructionItemList::Iterator FCItemIterator;
 
   /* A struct representing an item for which frames might need to be
    * constructed.  This contains all the information needed to construct the
    * frame other than the parent frame and whatever would be stored in the
    * frame constructor state. */
-  struct FrameConstructionItem : public PRCList {
-    // No need to PR_INIT_CLIST in the constructor because the only
-    // place that creates us immediately appends us.
+  struct FrameConstructionItem final
+    : public mozilla::LinkedListElement<FrameConstructionItem> {
     FrameConstructionItem(const FrameConstructionData* aFCData,
                           nsIContent* aContent,
                           nsIAtom* aTag,
                           int32_t aNameSpaceID,
                           PendingBinding* aPendingBinding,
                           already_AddRefed<nsStyleContext>& aStyleContext,
                           bool aSuppressWhiteSpaceOptimizations,
                           nsTArray<nsIAnonymousContentCreator::ContentInfo>* aAnonChildren) :