Backed out changeset c41a71b1c24e (bug 1310547) for failing splitText-normalize.html in reftests on Windows 7 VM debug. r=backout on a CLOSED TREE
authorSebastian Hengst <archaeopteryx@coole-files.de>
Sat, 29 Oct 2016 13:48:56 +0200
changeset 365142 b38d9a1cd7995c1cf79d7fc9e5351839a1a2d95c
parent 365141 0e4c28cbee9c44b1e1a97846212f03f95f4b11fb
child 365143 2ac285d70ab25253dd2f1822f23163b42e21e4a2
push id1369
push userjlorenzo@mozilla.com
push dateMon, 27 Feb 2017 14:59:41 +0000
treeherdermozilla-release@d75a1dba431f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1310547
milestone52.0a1
backs outc41a71b1c24eafe98514316d486d2a57476eb57c
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
Backed out changeset c41a71b1c24e (bug 1310547) for failing splitText-normalize.html in reftests on Windows 7 VM debug. r=backout on a CLOSED TREE
mfbt/LinkedList.h
mfbt/tests/TestLinkedList.cpp
--- a/mfbt/LinkedList.h
+++ b/mfbt/LinkedList.h
@@ -63,75 +63,27 @@
 
 #ifndef mozilla_LinkedList_h
 #define mozilla_LinkedList_h
 
 #include "mozilla/Assertions.h"
 #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
    * each linked list node (to keep track of whether |this| is the sentinel
    * node) and a branch on this value in getNext/getPrevious.
    *
    * We could get rid of the extra word of storage by shoving the "is
    * sentinel" bit into one of the pointers, although this would, of course,
@@ -198,42 +150,42 @@ public:
       remove();
     }
   }
 
   /*
    * Get the next element in the list, or nullptr if this is the last element
    * in the list.
    */
-  RawType getNext()            { return mNext->asT(); }
-  ConstRawType getNext() const { return mNext->asT(); }
+  T* getNext()             { return mNext->asT(); }
+  const T* getNext() const { return mNext->asT(); }
 
   /*
    * Get the previous element in the list, or nullptr if this is the first
    * element in the list.
    */
-  RawType getPrevious()            { return mPrev->asT(); }
-  ConstRawType getPrevious() const { return mPrev->asT(); }
+  T* getPrevious()             { return mPrev->asT(); }
+  const T* 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(RawType aElem)
+  void setNext(T* aElem)
   {
     MOZ_ASSERT(isInList());
     setNextUnsafe(aElem);
   }
 
   /*
    * Insert aElem before this element in the list.  |this| must be part of a
    * linked list when you call setPrevious(); otherwise, this method will
    * assert.
    */
-  void setPrevious(RawType aElem)
+  void setPrevious(T* aElem)
   {
     MOZ_ASSERT(isInList());
     setPreviousUnsafe(aElem);
   }
 
   /*
    * Remove this element from the list which contains it.  If this element is
    * not currently part of a linked list, this method asserts.
@@ -241,18 +193,16 @@ public:
   void remove()
   {
     MOZ_ASSERT(isInList());
 
     mPrev->mNext = mNext;
     mNext->mPrev = mPrev;
     mNext = this;
     mPrev = this;
-
-    Traits::exitList(this);
   }
 
   /*
    * Identical to remove(), but also asserts in debug builds that this element
    * is in aList.
    */
   void removeFrom(const LinkedList<T>& aList)
   {
@@ -266,92 +216,83 @@ public:
   bool isInList() const
   {
     MOZ_ASSERT((mNext == this) == (mPrev == this));
     return mNext != this;
   }
 
 private:
   friend class LinkedList<T>;
-  friend class detail::LinkedListElementTraits<T>;
 
   enum class NodeKind {
     Normal,
     Sentinel
   };
 
   explicit LinkedListElement(NodeKind nodeKind)
     : mNext(this),
       mPrev(this),
       mIsSentinel(nodeKind == NodeKind::Sentinel)
   { }
 
   /*
    * Return |this| cast to T* if we're a normal node, or return nullptr if
    * we're a sentinel node.
    */
-  RawType asT()
+  T* asT()
   {
-    return mIsSentinel ? nullptr : static_cast<RawType>(this);
+    return mIsSentinel ? nullptr : static_cast<T*>(this);
   }
-  ConstRawType asT() const
+  const T* asT() const
   {
-    return mIsSentinel ? nullptr : static_cast<ConstRawType>(this);
+    return mIsSentinel ? nullptr : static_cast<const T*>(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(RawType aElem)
+  void setNextUnsafe(T* aElem)
   {
     LinkedListElement *listElem = static_cast<LinkedListElement*>(aElem);
     MOZ_ASSERT(!listElem->isInList());
 
     listElem->mNext = this->mNext;
     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(RawType aElem)
+  void setPreviousUnsafe(T* aElem)
   {
     LinkedListElement<T>* listElem = static_cast<LinkedListElement<T>*>(aElem);
     MOZ_ASSERT(!listElem->isInList());
 
     listElem->mNext = this;
     listElem->mPrev = this->mPrev;
     this->mPrev->mNext = listElem;
     this->mPrev = listElem;
-
-    Traits::enterList(aElem);
   }
 
   /*
    * Adjust mNext and mPrev for implementing move constructor and move
    * assignment.
    */
   void adjustLinkForMove(LinkedListElement<T>&& aOther)
   {
     if (!aOther.isInList()) {
       mNext = this;
       mPrev = this;
       return;
     }
 
-    if (!mIsSentinel) {
-      Traits::enterList(this);
-    }
-
     MOZ_ASSERT(aOther.mNext->mPrev == &aOther);
     MOZ_ASSERT(aOther.mPrev->mNext == &aOther);
 
     /*
      * Initialize |this| with |aOther|'s mPrev/mNext pointers, and adjust those
      * element to point to this one.
      */
     mNext = aOther.mNext;
@@ -361,46 +302,36 @@ private:
     mPrev->mNext = this;
 
     /*
      * Adjust |aOther| so it doesn't think it's in a list.  This makes it
      * safely destructable.
      */
     aOther.mNext = &aOther;
     aOther.mPrev = &aOther;
-
-    if (!mIsSentinel) {
-      Traits::exitList(&aOther);
-    }
   }
 
   LinkedListElement& operator=(const LinkedListElement<T>& aOther) = delete;
   LinkedListElement(const LinkedListElement<T>& aOther) = delete;
 };
 
 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 {
-    RawType mCurrent;
+    T* mCurrent;
 
   public:
-    explicit Iterator(RawType aCurrent) : mCurrent(aCurrent) {}
+    explicit Iterator(T* aCurrent) : mCurrent(aCurrent) {}
 
-    RawType operator *() const {
+    T* operator *() const {
       return mCurrent;
     }
 
     const Iterator& operator++() {
       mCurrent = mCurrent->getNext();
       return *this;
     }
 
@@ -427,64 +358,64 @@ public:
                "failing this assertion means this LinkedList's creator is "
                "buggy: it should have removed all this list's elements before "
                "the list's destruction");
   }
 
   /*
    * Add aElem to the front of the list.
    */
-  void insertFront(RawType aElem)
+  void insertFront(T* aElem)
   {
     /* Bypass setNext()'s this->isInList() assertion. */
     sentinel.setNextUnsafe(aElem);
   }
 
   /*
    * Add aElem to the back of the list.
    */
-  void insertBack(RawType aElem)
+  void insertBack(T* aElem)
   {
     sentinel.setPreviousUnsafe(aElem);
   }
 
   /*
    * Get the first element of the list, or nullptr if the list is empty.
    */
-  RawType getFirst()            { return sentinel.getNext(); }
-  ConstRawType getFirst() const { return sentinel.getNext(); }
+  T* getFirst()             { return sentinel.getNext(); }
+  const T* getFirst() const { return sentinel.getNext(); }
 
   /*
    * Get the last element of the list, or nullptr if the list is empty.
    */
-  RawType getLast()            { return sentinel.getPrevious(); }
-  ConstRawType getLast() const { return sentinel.getPrevious(); }
+  T* getLast()             { return sentinel.getPrevious(); }
+  const T* getLast() const { return sentinel.getPrevious(); }
 
   /*
    * Get and remove the first element of the list.  If the list is empty,
    * return nullptr.
    */
-  ClientType popFirst()
+  T* popFirst()
   {
-    ClientType ret = sentinel.getNext();
+    T* ret = sentinel.getNext();
     if (ret) {
-      static_cast<LinkedListElement<T>*>(RawType(ret))->remove();
+      static_cast<LinkedListElement<T>*>(ret)->remove();
     }
     return ret;
   }
 
   /*
    * Get and remove the last element of the list.  If the list is empty,
    * return nullptr.
    */
-  ClientType popLast()
+  T* popLast()
   {
-    ClientType ret = sentinel.getPrevious();
+    T* ret = sentinel.getPrevious();
     if (ret) {
-      static_cast<LinkedListElement<T>*>(RawType(ret))->remove();
+      static_cast<LinkedListElement<T>*>(ret)->remove();
     }
     return ret;
   }
 
   /*
    * Return true if the list is empty, or false otherwise.
    */
   bool isEmpty() const
@@ -595,20 +526,20 @@ public:
         cur = cur->mNext;
     } while (cur != &sentinel);
 #endif /* ifdef DEBUG */
   }
 
 private:
   friend class LinkedListElement<T>;
 
-  void assertContains(const RawType aValue) const
+  void assertContains(const T* aValue) const
   {
 #ifdef DEBUG
-    for (ConstRawType elem = getFirst(); elem; elem = elem->getNext()) {
+    for (const T* elem = getFirst(); elem; elem = elem->getNext()) {
       if (elem == aValue) {
         return;
       }
     }
     MOZ_CRASH("element wasn't found in this list!");
 #endif
   }
 
--- a/mfbt/tests/TestLinkedList.cpp
+++ b/mfbt/tests/TestLinkedList.cpp
@@ -172,78 +172,16 @@ TestPrivate()
   size_t count = 0;
   for (PrivateClass* p : list) {
     MOZ_RELEASE_ASSERT(p, "cannot have null elements in list");
     count++;
   }
   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;
 }