Bug 1310547 - Allow LinkedList to hold RefPtr elements (r=froydnj)

mfbt/LinkedList.h

@@ -68,17 +68,65 @@
 #include "mozilla/Attributes.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/Move.h"
+#include "mozilla/RefPtr.h"
 
 #ifdef __cplusplus
 
 namespace mozilla {
 
+template<typename T>
+class LinkedListElement;
+
+namespace detail {
+
+/**
+ * LinkedList supports refcounted elements using this adapter class. Clients
+ * using LinkedList<RefPtr<T>> will get a data structure that holds a strong
+ * reference to T as long as T is in the list.
+ */
+template<typename T>
+struct LinkedListElementTraits
+{
+  typedef T* RawType;
+  typedef const T* ConstRawType;
+  typedef T* ClientType;
+  typedef const T* ConstClientType;
+
+  // These static methods are called when an element is added to or removed from
+  // a linked list. It can be used to keep track ownership in lists that are
+  // supposed to own their elements. If elements are transferred from one list
+  // to another, no enter or exit calls happen since the elements still belong
+  // to a list.
+  static void enterList(LinkedListElement<T>* elt) {}
+  static void exitList(LinkedListElement<T>* elt) {}
+};
+
+template<typename T>
+struct LinkedListElementTraits<RefPtr<T>>
+{
+  typedef T* RawType;
+  typedef const T* ConstRawType;
+  typedef RefPtr<T> ClientType;
+  typedef RefPtr<const T> ConstClientType;
+
+  static void enterList(LinkedListElement<RefPtr<T>>* elt) { elt->asT()->AddRef(); }
+  static void exitList(LinkedListElement<RefPtr<T>>* elt) { elt->asT()->Release(); }
+};
+
+} /* namespace detail */
+
 template<typename T>
 class LinkedList;
 
 template<typename T>
 class LinkedListElement
 {
+  typedef typename detail::LinkedListElementTraits<T> Traits;
+  typedef typename Traits::RawType RawType;
+  typedef typename Traits::ConstRawType ConstRawType;
+  typedef typename Traits::ClientType ClientType;
+  typedef typename Traits::ConstClientType ConstClientType;
+
   /*
    * It's convenient that we return nullptr when getNext() or getPrevious()
    * hits the end of the list, but doing so costs an extra word of storage in
@@ -155,21 +203,21 @@ public:
    * Get the next element in the list, or nullptr if this is the last element
    * in the list.
    */
-  T* getNext()             { return mNext->asT(); }
-  const T* getNext() const { return mNext->asT(); }
+  RawType getNext()            { return mNext->asT(); }
+  ConstRawType getNext() const { return mNext->asT(); }
 
   /*
    * Get the previous element in the list, or nullptr if this is the first
    * element in the list.
    */
-  T* getPrevious()             { return mPrev->asT(); }
-  const T* getPrevious() const { return mPrev->asT(); }
+  RawType getPrevious()            { return mPrev->asT(); }
+  ConstRawType getPrevious() const { return mPrev->asT(); }
 
   /*
    * Insert aElem after this element in the list.  |this| must be part of a
    * linked list when you call setNext(); otherwise, this method will assert.
    */
-  void setNext(T* aElem)
+  void setNext(RawType aElem)
   {
     MOZ_ASSERT(isInList());
     setNextUnsafe(aElem);
@@ -180,7 +228,7 @@ public:
    * linked list when you call setPrevious(); otherwise, this method will
    * assert.
    */
-  void setPrevious(T* aElem)
+  void setPrevious(RawType aElem)
   {
     MOZ_ASSERT(isInList());
     setPreviousUnsafe(aElem);
@@ -198,6 +246,8 @@ public:
     mNext->mPrev = mPrev;
     mNext = this;
     mPrev = this;
+
+    Traits::exitList(this);
   }
 
   /*
@@ -221,6 +271,7 @@ public:
 
 private:
   friend class LinkedList<T>;
+  friend class detail::LinkedListElementTraits<T>;
 
   enum class NodeKind {
     Normal,
@@ -237,20 +288,20 @@ private:
    * Return |this| cast to T* if we're a normal node, or return nullptr if
    * we're a sentinel node.
    */
-  T* asT()
+  RawType asT()
   {
-    return mIsSentinel ? nullptr : static_cast<T*>(this);
+    return mIsSentinel ? nullptr : static_cast<RawType>(this);
   }
-  const T* asT() const
+  ConstRawType asT() const
   {
-    return mIsSentinel ? nullptr : static_cast<const T*>(this);
+    return mIsSentinel ? nullptr : static_cast<ConstRawType>(this);
   }
 
   /*
    * Insert aElem after this element, but don't check that this element is in
    * the list.  This is called by LinkedList::insertFront().
    */
-  void setNextUnsafe(T* aElem)
+  void setNextUnsafe(RawType aElem)
   {
     LinkedListElement *listElem = static_cast<LinkedListElement*>(aElem);
     MOZ_ASSERT(!listElem->isInList());
@@ -259,13 +310,15 @@ private:
     listElem->mPrev = this;
     this->mNext->mPrev = listElem;
     this->mNext = listElem;
+
+    Traits::enterList(aElem);
   }
 
   /*
    * Insert aElem before this element, but don't check that this element is in
    * the list.  This is called by LinkedList::insertBack().
    */
-  void setPreviousUnsafe(T* aElem)
+  void setPreviousUnsafe(RawType aElem)
   {
     LinkedListElement<T>* listElem = static_cast<LinkedListElement<T>*>(aElem);
     MOZ_ASSERT(!listElem->isInList());
@@ -274,6 +327,8 @@ private:
     listElem->mPrev = this->mPrev;
     this->mPrev->mNext = listElem;
     this->mPrev = listElem;
+
+    Traits::enterList(aElem);
   }
 
   /*
@@ -288,6 +343,10 @@ private:
       return;
     }
 
+    if (!mIsSentinel) {
+      Traits::enterList(this);
+    }
+
     MOZ_ASSERT(aOther.mNext->mPrev == &aOther);
     MOZ_ASSERT(aOther.mPrev->mNext == &aOther);
 
@@ -307,6 +366,10 @@ private:
      */
     aOther.mNext = &aOther;
     aOther.mPrev = &aOther;
+
+    if (!mIsSentinel) {
+      Traits::exitList(&aOther);
+    }
   }
 
   LinkedListElement& operator=(const LinkedListElement<T>& aOther) = delete;
@@ -317,16 +380,22 @@ template<typename T>
 class LinkedList
 {
 private:
+  typedef typename detail::LinkedListElementTraits<T> Traits;
+  typedef typename Traits::RawType RawType;
+  typedef typename Traits::ConstRawType ConstRawType;
+  typedef typename Traits::ClientType ClientType;
+  typedef typename Traits::ConstClientType ConstClientType;
+
   LinkedListElement<T> sentinel;
 
 public:
   class Iterator {
-    T* mCurrent;
+    RawType mCurrent;
 
   public:
-    explicit Iterator(T* aCurrent) : mCurrent(aCurrent) {}
+    explicit Iterator(RawType aCurrent) : mCurrent(aCurrent) {}
 
-    T* operator *() const {
+    RawType operator *() const {
       return mCurrent;
     }
 
@@ -363,7 +432,7 @@ public:
   /*
    * Add aElem to the front of the list.
    */
-  void insertFront(T* aElem)
+  void insertFront(RawType aElem)
   {
     /* Bypass setNext()'s this->isInList() assertion. */
     sentinel.setNextUnsafe(aElem);
@@ -372,7 +441,7 @@ public:
   /*
    * Add aElem to the back of the list.
    */
-  void insertBack(T* aElem)
+  void insertBack(RawType aElem)
   {
     sentinel.setPreviousUnsafe(aElem);
   }
@@ -380,24 +449,24 @@ public:
   /*
    * Get the first element of the list, or nullptr if the list is empty.
    */
-  T* getFirst()             { return sentinel.getNext(); }
-  const T* getFirst() const { return sentinel.getNext(); }
+  RawType getFirst()            { return sentinel.getNext(); }
+  ConstRawType getFirst() const { return sentinel.getNext(); }
 
   /*
    * Get the last element of the list, or nullptr if the list is empty.
    */
-  T* getLast()             { return sentinel.getPrevious(); }
-  const T* getLast() const { return sentinel.getPrevious(); }
+  RawType getLast()            { return sentinel.getPrevious(); }
+  ConstRawType getLast() const { return sentinel.getPrevious(); }
 
   /*
    * Get and remove the first element of the list.  If the list is empty,
    * return nullptr.
    */
-  T* popFirst()
+  ClientType popFirst()
   {
-    T* ret = sentinel.getNext();
+    ClientType ret = sentinel.getNext();
     if (ret) {
-      static_cast<LinkedListElement<T>*>(ret)->remove();
+      static_cast<LinkedListElement<T>*>(RawType(ret))->remove();
     }
     return ret;
   }
@@ -406,11 +475,11 @@ public:
    * Get and remove the last element of the list.  If the list is empty,
    * return nullptr.
    */
-  T* popLast()
+  ClientType popLast()
   {
-    T* ret = sentinel.getPrevious();
+    ClientType ret = sentinel.getPrevious();
     if (ret) {
-      static_cast<LinkedListElement<T>*>(ret)->remove();
+      static_cast<LinkedListElement<T>*>(RawType(ret))->remove();
     }
     return ret;
   }
@@ -531,10 +600,10 @@ public:
 private:
   friend class LinkedListElement<T>;
 
-  void assertContains(const T* aValue) const
+  void assertContains(const RawType aValue) const
   {
 #ifdef DEBUG
-    for (const T* elem = getFirst(); elem; elem = elem->getNext()) {
+    for (ConstRawType elem = getFirst(); elem; elem = elem->getNext()) {
       if (elem == aValue) {
         return;
       }

mfbt/tests/TestLinkedList.cpp

@@ -177,11 +177,73 @@ TestPrivate()
   MOZ_RELEASE_ASSERT(count == 2);
 }
 
+struct CountedClass : public LinkedListElement<RefPtr<CountedClass>>
+{
+  int mCount;
+  void AddRef() { mCount++; }
+  void Release() { mCount--; }
+
+  CountedClass() : mCount(0) {}
+  ~CountedClass() { MOZ_RELEASE_ASSERT(mCount == 0); }
+};
+
+static void
+TestRefPtrList()
+{
+  LinkedList<RefPtr<CountedClass>> list;
+  CountedClass* elt1 = new CountedClass;
+  CountedClass* elt2 = new CountedClass;
+
+  list.insertBack(elt1);
+  list.insertBack(elt2);
+
+  MOZ_RELEASE_ASSERT(elt1->mCount == 1);
+  MOZ_RELEASE_ASSERT(elt2->mCount == 1);
+
+  for (RefPtr<CountedClass> p : list) {
+    MOZ_RELEASE_ASSERT(p->mCount == 2);
+  }
+
+  RefPtr<CountedClass> ptr = list.getFirst();
+  while (ptr) {
+    MOZ_RELEASE_ASSERT(ptr->mCount == 2);
+    RefPtr<CountedClass> next = ptr->getNext();
+    ptr->remove();
+    ptr = Move(next);
+  }
+  ptr = nullptr;
+
+  MOZ_RELEASE_ASSERT(elt1->mCount == 0);
+  MOZ_RELEASE_ASSERT(elt2->mCount == 0);
+
+  list.insertBack(elt1);
+  elt1->setNext(elt2);
+
+  MOZ_RELEASE_ASSERT(elt1->mCount == 1);
+  MOZ_RELEASE_ASSERT(elt2->mCount == 1);
+
+  RefPtr<CountedClass> first = list.popFirst();
+
+  MOZ_RELEASE_ASSERT(elt1->mCount == 1);
+  MOZ_RELEASE_ASSERT(elt2->mCount == 1);
+
+  RefPtr<CountedClass> second = list.popFirst();
+
+  MOZ_RELEASE_ASSERT(elt1->mCount == 1);
+  MOZ_RELEASE_ASSERT(elt2->mCount == 1);
+
+  first = second = nullptr;
+
+  delete elt1;
+  delete elt2;
+}
+
 int
 main()
 {
   TestList();
   TestPrivate();
   TestMove();
+  TestRefPtrList();
   return 0;
 }