From 89ca6e236f4c6d854bd101fa5a2c121ba800a414 Mon Sep 17 00:00:00 2001 From: Bill McCloskey Date: Sun, 16 Oct 2016 18:40:49 -0700 Subject: [PATCH] Bug 1310547 - Allow LinkedList to hold RefPtr elements (r=froydnj) --- mfbt/LinkedList.h | 127 ++++++++++++++++++++++++++-------- mfbt/tests/TestLinkedList.cpp | 62 +++++++++++++++++ 2 files changed, 160 insertions(+), 29 deletions(-) diff --git a/mfbt/LinkedList.h b/mfbt/LinkedList.h index 70641699fb8f..d8925577dba3 100644 --- a/mfbt/LinkedList.h +++ b/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 +class LinkedListElement; + +namespace detail { + +/** + * LinkedList supports refcounted elements using this adapter class. Clients + * using LinkedList> will get a data structure that holds a strong + * reference to T as long as T is in the list. + */ +template +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* elt) {} + static void exitList(LinkedListElement* elt) {} +}; + +template +struct LinkedListElementTraits> +{ + typedef T* RawType; + typedef const T* ConstRawType; + typedef RefPtr ClientType; + typedef RefPtr ConstClientType; + + static void enterList(LinkedListElement>* elt) { elt->asT()->AddRef(); } + static void exitList(LinkedListElement>* elt) { elt->asT()->Release(); } +}; + +} /* namespace detail */ + template class LinkedList; template class LinkedListElement { + typedef typename detail::LinkedListElementTraits 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; + friend class detail::LinkedListElementTraits; 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(this); + return mIsSentinel ? nullptr : static_cast(this); } - const T* asT() const + ConstRawType asT() const { - return mIsSentinel ? nullptr : static_cast(this); + return mIsSentinel ? nullptr : static_cast(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(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* listElem = static_cast*>(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& aOther) = delete; @@ -317,16 +380,22 @@ template class LinkedList { private: + typedef typename detail::LinkedListElementTraits Traits; + typedef typename Traits::RawType RawType; + typedef typename Traits::ConstRawType ConstRawType; + typedef typename Traits::ClientType ClientType; + typedef typename Traits::ConstClientType ConstClientType; + LinkedListElement 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*>(ret)->remove(); + static_cast*>(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*>(ret)->remove(); + static_cast*>(RawType(ret))->remove(); } return ret; } @@ -531,10 +600,10 @@ public: private: friend class LinkedListElement; - 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; } diff --git a/mfbt/tests/TestLinkedList.cpp b/mfbt/tests/TestLinkedList.cpp index 519b34c56636..aa373ed2f371 100644 --- a/mfbt/tests/TestLinkedList.cpp +++ b/mfbt/tests/TestLinkedList.cpp @@ -177,11 +177,73 @@ TestPrivate() MOZ_RELEASE_ASSERT(count == 2); } +struct CountedClass : public LinkedListElement> +{ + int mCount; + void AddRef() { mCount++; } + void Release() { mCount--; } + + CountedClass() : mCount(0) {} + ~CountedClass() { MOZ_RELEASE_ASSERT(mCount == 0); } +}; + +static void +TestRefPtrList() +{ + LinkedList> 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 p : list) { + MOZ_RELEASE_ASSERT(p->mCount == 2); + } + + RefPtr ptr = list.getFirst(); + while (ptr) { + MOZ_RELEASE_ASSERT(ptr->mCount == 2); + RefPtr 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 first = list.popFirst(); + + MOZ_RELEASE_ASSERT(elt1->mCount == 1); + MOZ_RELEASE_ASSERT(elt2->mCount == 1); + + RefPtr 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; }