Backed out 2 changesets (bug 1396723) for !ElementAccess::Get(mHead).mPrev assertion failures
authorPhil Ringnalda <philringnalda@gmail.com>
Wed, 06 Sep 2017 21:39:20 -0700
changeset 431237 c02d7df64e12391b7464814e370789402cff7b46
parent 431236 ab26002b1029ce3340d2c43c3c0507043a37aa8e
child 431238 20551d68f602f8815c11bcc349e275a195babbc0
push id1567
push userjlorenzo@mozilla.com
push dateThu, 02 Nov 2017 12:36:05 +0000
treeherdermozilla-release@e512c14a0406 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1396723
milestone57.0a1
backs out4f17343164b66db561be6371496c544f0c202a2d
b744eba9ca787a5f28972463ba9cfeb36f5b49a2
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 2 changesets (bug 1396723) for !ElementAccess::Get(mHead).mPrev assertion failures Backed out changeset 4f17343164b6 (bug 1396723) Backed out changeset b744eba9ca78 (bug 1396723) MozReview-Commit-ID: JbvM6oMxEFl
js/src/vm/Debugger.h
js/src/vm/Runtime.h
memory/mozjemalloc/linkedlist.h
memory/mozjemalloc/mozjemalloc.cpp
mfbt/DoublyLinkedList.h
mfbt/tests/TestDoublyLinkedList.cpp
--- a/js/src/vm/Debugger.h
+++ b/js/src/vm/Debugger.h
@@ -254,17 +254,17 @@ typedef mozilla::Variant<ScriptSourceObj
 // Either a AbstractFramePtr, for ordinary JS, or a wasm::DebugFrame,
 // for synthesized frame of a wasm code.
 typedef mozilla::Variant<AbstractFramePtr, wasm::DebugFrame*> DebuggerFrameReferent;
 
 class Debugger : private mozilla::LinkedListElement<Debugger>
 {
     friend class Breakpoint;
     friend class DebuggerMemory;
-    friend struct JSRuntime::GlobalObjectWatchersLinkAccess<Debugger>;
+    friend struct JSRuntime::GlobalObjectWatchersSiblingAccess<Debugger>;
     friend class SavedStacks;
     friend class ScriptedOnStepHandler;
     friend class ScriptedOnPopHandler;
     friend class mozilla::LinkedListElement<Debugger>;
     friend class mozilla::LinkedList<Debugger>;
     friend bool (::JS_DefineDebuggerObject)(JSContext* cx, JS::HandleObject obj);
     friend bool (::JS::dbg::IsDebugger)(JSObject&);
     friend bool (::JS::dbg::GetDebuggeeGlobals)(JSContext*, JSObject&, AutoObjectVector&);
@@ -384,27 +384,36 @@ class Debugger : private mozilla::Linked
     js::GCPtrObject uncaughtExceptionHook; /* Strong reference. */
     bool enabled;
     bool allowUnobservedAsmJS;
     bool allowWasmBinarySource;
 
     // Whether to enable code coverage on the Debuggee.
     bool collectCoverageInfo;
 
-    template <typename T>
-    struct DebuggerLinkAccess {
-      static mozilla::DoublyLinkedListElement<T>& Get(T* aThis) {
-        return aThis->debuggerLink;
+    template<typename T>
+    struct DebuggerSiblingAccess {
+      static T* GetNext(T* elm) {
+        return elm->debuggerLink.mNext;
+      }
+      static void SetNext(T* elm, T* next) {
+        elm->debuggerLink.mNext = next;
+      }
+      static T* GetPrev(T* elm) {
+        return elm->debuggerLink.mPrev;
+      }
+      static void SetPrev(T* elm, T* prev) {
+        elm->debuggerLink.mPrev = prev;
       }
     };
 
     // List of all js::Breakpoints in this debugger.
     using BreakpointList =
         mozilla::DoublyLinkedList<js::Breakpoint,
-                                  DebuggerLinkAccess<js::Breakpoint>>;
+                                  DebuggerSiblingAccess<js::Breakpoint>>;
     BreakpointList breakpoints;
 
     // The set of GC numbers for which one or more of this Debugger's observed
     // debuggees participated in.
     using GCNumberSet = HashSet<uint64_t, DefaultHasher<uint64_t>, RuntimeAllocPolicy>;
     GCNumberSet observedGCs;
 
     using AllocationsLog = js::TraceableFifo<AllocationsLogEntry>;
@@ -1581,27 +1590,36 @@ class BreakpointSite {
     friend class Debugger;
 
   public:
     enum class Type { JS, Wasm };
 
   private:
     Type type_;
 
-    template <typename T>
-    struct SiteLinkAccess {
-      static mozilla::DoublyLinkedListElement<T>& Get(T* aThis) {
-        return aThis->siteLink;
+    template<typename T>
+    struct SiteSiblingAccess {
+      static T* GetNext(T* elm) {
+        return elm->siteLink.mNext;
+      }
+      static void SetNext(T* elm, T* next) {
+        elm->siteLink.mNext = next;
+      }
+      static T* GetPrev(T* elm) {
+        return elm->siteLink.mPrev;
+      }
+      static void SetPrev(T* elm, T* prev) {
+        elm->siteLink.mPrev = prev;
       }
     };
 
     // List of all js::Breakpoints at this instruction.
     using BreakpointList =
         mozilla::DoublyLinkedList<js::Breakpoint,
-                                  SiteLinkAccess<js::Breakpoint>>;
+                                  SiteSiblingAccess<js::Breakpoint>>;
     BreakpointList breakpoints;
     size_t enabledCount;  /* number of breakpoints in the list that are enabled */
 
   protected:
     virtual void recompile(FreeOp* fop) = 0;
     bool isEmpty() const;
     inline bool isEnabled() const { return enabledCount > 0; }
 
--- a/js/src/vm/Runtime.h
+++ b/js/src/vm/Runtime.h
@@ -577,25 +577,34 @@ struct JSRuntime : public js::MallocProv
     js::ActiveThreadData<mozilla::LinkedList<JS::detail::WeakCacheBase>> weakCaches_;
   public:
     mozilla::LinkedList<JS::detail::WeakCacheBase>& weakCaches() { return weakCaches_.ref(); }
     void registerWeakCache(JS::detail::WeakCacheBase* cachep) {
         weakCaches().insertBack(cachep);
     }
 
     template <typename T>
-    struct GlobalObjectWatchersLinkAccess {
-      static mozilla::DoublyLinkedListElement<T>& Get(T* aThis) {
-        return aThis->onNewGlobalObjectWatchersLink;
+    struct GlobalObjectWatchersSiblingAccess {
+      static T* GetNext(T* elm) {
+        return elm->onNewGlobalObjectWatchersLink.mNext;
+      }
+      static void SetNext(T* elm, T* next) {
+        elm->onNewGlobalObjectWatchersLink.mNext = next;
+      }
+      static T* GetPrev(T* elm) {
+        return elm->onNewGlobalObjectWatchersLink.mPrev;
+      }
+      static void SetPrev(T* elm, T* prev) {
+        elm->onNewGlobalObjectWatchersLink.mPrev = prev;
       }
     };
 
     using WatchersList =
         mozilla::DoublyLinkedList<js::Debugger,
-                                  GlobalObjectWatchersLinkAccess<js::Debugger>>;
+                                  GlobalObjectWatchersSiblingAccess<js::Debugger>>;
   private:
     /*
      * List of all enabled Debuggers that have onNewGlobalObject handler
      * methods established.
      */
     js::ActiveThreadData<WatchersList> onNewGlobalObjectWatchers_;
 
   public:
new file mode 100644
--- /dev/null
+++ b/memory/mozjemalloc/linkedlist.h
@@ -0,0 +1,75 @@
+/* -*- Mode: C; tab-width: 8; c-basic-offset: 8; indent-tabs-mode: t -*- */
+/* vim:set softtabstop=8 shiftwidth=8 noet: */
+/*-
+ * Copyright (C) the Mozilla Foundation.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice(s), this list of conditions and the following disclaimer as
+ *    the first lines of this file unmodified other than the possible
+ *    addition of one or more copyright notices.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice(s), this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDER(S) ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE
+ * OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
+ * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ *******************************************************************************/
+
+#ifndef linkedlist_h__
+#define linkedlist_h__
+
+#include <stddef.h>
+
+struct LinkedList {
+	LinkedList *next;
+	LinkedList *prev;
+};
+
+/* Convert from LinkedList* to foo*. */
+#define LinkedList_Get(e, type, prop) \
+  (type*)((char*)(e) - offsetof(type, prop))
+
+/* Insert |e| at the beginning of |l|.  */
+void LinkedList_InsertHead(LinkedList *l, LinkedList *e)
+{
+	e->next = l;
+	e->prev = l->prev;
+	e->next->prev = e;
+	e->prev->next = e;
+}
+
+void LinkedList_Remove(LinkedList *e)
+{
+	e->prev->next = e->next;
+	e->next->prev = e->prev;
+	e->next = e;
+	e->prev = e;
+}
+
+bool LinkedList_IsEmpty(LinkedList *e)
+{
+	return e->next == e;
+}
+
+void LinkedList_Init(LinkedList *e)
+{
+	e->next = e;
+	e->prev = e;
+}
+
+#endif
--- a/memory/mozjemalloc/mozjemalloc.cpp
+++ b/memory/mozjemalloc/mozjemalloc.cpp
@@ -107,17 +107,16 @@
  *******************************************************************************
  */
 
 #include "mozmemory_wrap.h"
 #include "mozjemalloc.h"
 #include "mozilla/Sprintf.h"
 #include "mozilla/Likely.h"
 #include "mozilla/MacroArgs.h"
-#include "mozilla/DoublyLinkedList.h"
 
 #ifdef ANDROID
 #define NO_TLS
 #endif
 
 /*
  * On Linux, we use madvise(MADV_DONTNEED) to release memory back to the
  * operating system.  If we release 1MB of live pages with MADV_DONTNEED, our
@@ -248,16 +247,17 @@ typedef long ssize_t;
 #include <mach/mach_init.h>
 #include <mach/vm_map.h>
 #include <malloc/malloc.h>
 #endif
 
 #endif
 
 #include "mozjemalloc_types.h"
+#include "linkedlist.h"
 
 /* Some tools, such as /dev/dsp wrappers, LD_PRELOAD libraries that
  * happen to override mmap() and call dlsym() from their overridden
  * mmap(). The problem is that dlsym() calls malloc(), and this ends
  * up in a dead lock in jemalloc.
  * On these systems, we prefer to directly use the system call.
  * We do that for Linux systems and kfreebsd with GNU userland.
  * Note sanity checks are not done (alignment of offset, ...) because
@@ -622,38 +622,27 @@ struct arena_chunk_t {
 
 #ifdef MALLOC_DOUBLE_PURGE
 	/* If we're double-purging, we maintain a linked list of chunks which
 	 * have pages which have been madvise(MADV_FREE)'d but not explicitly
 	 * purged.
 	 *
 	 * We're currently lazy and don't remove a chunk from this list when
 	 * all its madvised pages are recommitted. */
-	mozilla::DoublyLinkedListElement<arena_chunk_t> chunks_madvised_elem;
+	LinkedList	chunks_madvised_elem;
 #endif
 
 	/* Number of dirty pages. */
 	size_t		ndirty;
 
 	/* Map of pages within chunk that keeps track of free/large/small. */
 	arena_chunk_map_t map[1]; /* Dynamically sized. */
 };
 typedef rb_tree(arena_chunk_t) arena_chunk_tree_t;
 
-#ifdef MALLOC_DOUBLE_PURGE
-template<>
-struct mozilla::GetDoublyLinkedListElement<arena_chunk_t>
-{
-  static mozilla::DoublyLinkedListElement<arena_chunk_t>& Get(arena_chunk_t* aThis)
-  {
-    return aThis->chunks_madvised_elem;
-  }
-};
-#endif
-
 struct arena_run_t {
 #if defined(MOZ_DEBUG) || defined(MOZ_DIAGNOSTIC_ASSERT_ENABLED)
 	uint32_t	magic;
 #  define ARENA_RUN_MAGIC 0x384adf93
 #endif
 
 	/* Bin this run is associated with. */
 	arena_bin_t	*bin;
@@ -715,17 +704,17 @@ struct arena_t {
 	arena_stats_t		stats;
 
 	/* Tree of dirty-page-containing chunks this arena manages. */
 	arena_chunk_tree_t	chunks_dirty;
 
 #ifdef MALLOC_DOUBLE_PURGE
 	/* Head of a linked list of MADV_FREE'd-page-containing chunks this
 	 * arena manages. */
-	mozilla::DoublyLinkedList<arena_chunk_t> chunks_madvised;
+	LinkedList		chunks_madvised;
 #endif
 
 	/*
 	 * In order to avoid rapid chunk allocation/deallocation when an arena
 	 * oscillates right on the cusp of needing a new chunk, cache the most
 	 * recently freed chunk.  The spare is left in the arena's chunk trees
 	 * until it is deleted.
 	 *
@@ -2705,36 +2694,35 @@ arena_chunk_init(arena_t *arena, arena_c
 #endif
 	arena->stats.committed += arena_chunk_header_npages;
 
 	/* Insert the run into the runs_avail tree. */
 	arena_avail_tree_insert(&arena->runs_avail,
 	    &chunk->map[arena_chunk_header_npages]);
 
 #ifdef MALLOC_DOUBLE_PURGE
-	new (&chunk->chunks_madvised_elem) mozilla::DoublyLinkedListElement<arena_chunk_t>();
+	LinkedList_Init(&chunk->chunks_madvised_elem);
 #endif
 }
 
 static void
 arena_chunk_dealloc(arena_t *arena, arena_chunk_t *chunk)
 {
 
 	if (arena->spare) {
 		if (arena->spare->ndirty > 0) {
 			arena_chunk_tree_dirty_remove(
 			    &chunk->arena->chunks_dirty, arena->spare);
 			arena->ndirty -= arena->spare->ndirty;
 			arena->stats.committed -= arena->spare->ndirty;
 		}
 
 #ifdef MALLOC_DOUBLE_PURGE
-		if (arena->chunks_madvised.ElementProbablyInList(arena->spare)) {
-			arena->chunks_madvised.remove(arena->spare);
-		}
+		/* This is safe to do even if arena->spare is not in the list. */
+		LinkedList_Remove(&arena->spare->chunks_madvised_elem);
 #endif
 
 		chunk_dealloc((void *)arena->spare, chunksize, ARENA_CHUNK);
 		arena->stats.mapped -= chunksize;
 		arena->stats.committed -= arena_chunk_header_npages;
 	}
 
 	/*
@@ -2885,20 +2873,18 @@ arena_purge(arena_t *arena, bool all)
 		if (chunk->ndirty == 0) {
 			arena_chunk_tree_dirty_remove(&arena->chunks_dirty,
 			    chunk);
 		}
 #ifdef MALLOC_DOUBLE_PURGE
 		if (madvised) {
 			/* The chunk might already be in the list, but this
 			 * makes sure it's at the front. */
-			if (arena->chunks_madvised.ElementProbablyInList(chunk)) {
-				arena->chunks_madvised.remove(chunk);
-			}
-			arena->chunks_madvised.pushFront(chunk);
+			LinkedList_Remove(&chunk->chunks_madvised_elem);
+			LinkedList_InsertHead(&arena->chunks_madvised, &chunk->chunks_madvised_elem);
 		}
 #endif
 	}
 }
 
 static void
 arena_run_dalloc(arena_t *arena, arena_run_t *run, bool dirty)
 {
@@ -4032,17 +4018,17 @@ arena_new(arena_t *arena)
 	if (malloc_spin_init(&arena->lock))
 		return (true);
 
 	memset(&arena->stats, 0, sizeof(arena_stats_t));
 
 	/* Initialize chunks. */
 	arena_chunk_tree_dirty_new(&arena->chunks_dirty);
 #ifdef MALLOC_DOUBLE_PURGE
-	new (&arena->chunks_madvised) mozilla::DoublyLinkedList<arena_chunk_t>();
+	LinkedList_Init(&arena->chunks_madvised);
 #endif
 	arena->spare = nullptr;
 
 	arena->ndirty = 0;
 
 	arena_avail_tree_new(&arena->runs_avail);
 
 	/* Initialize bins. */
@@ -5087,19 +5073,22 @@ hard_purge_chunk(arena_chunk_t *chunk)
 }
 
 /* Explicitly remove all of this arena's MADV_FREE'd pages from memory. */
 static void
 hard_purge_arena(arena_t *arena)
 {
 	malloc_spin_lock(&arena->lock);
 
-	while (!arena->chunks_madvised.isEmpty()) {
-		arena_chunk_t *chunk = arena->chunks_madvised.popFront();
+	while (!LinkedList_IsEmpty(&arena->chunks_madvised)) {
+		arena_chunk_t *chunk =
+			LinkedList_Get(arena->chunks_madvised.next,
+				       arena_chunk_t, chunks_madvised_elem);
 		hard_purge_chunk(chunk);
+		LinkedList_Remove(&chunk->chunks_madvised_elem);
 	}
 
 	malloc_spin_unlock(&arena->lock);
 }
 
 template<> inline void
 MozJemalloc::jemalloc_purge_freed_pages()
 {
--- a/mfbt/DoublyLinkedList.h
+++ b/mfbt/DoublyLinkedList.h
@@ -60,76 +60,70 @@
  *       }
  *     }
  *   };
  */
 
 namespace mozilla {
 
 /**
+ * Provides access to a next and previous element pointer named |mNext| and
+ * |mPrev| respectively. This class is the default and will work if the list
+ * element derives from DoublyLinkedListElement.
+ *
+ * Although designed to work with DoublyLinkedListElement this will als work
+ * with any class that defines |mNext| and |mPrev| members with the correct
+ * type.
+ */
+template <typename T>
+struct DoublyLinkedSiblingAccess {
+  static void SetNext(T* aElm, T* aNext) { aElm->mNext = aNext; }
+  static T* GetNext(T* aElm) { return aElm->mNext; }
+  static void SetPrev(T* aElm, T* aPrev) { aElm->mPrev = aPrev; }
+  static T* GetPrev(T* aElm) { return aElm->mPrev; }
+};
+
+/**
  *  Deriving from this will allow T to be inserted into and removed from a
  *  DoublyLinkedList.
  */
 template <typename T>
-class DoublyLinkedListElement
+struct DoublyLinkedListElement
 {
-  template<typename U, typename E> friend class DoublyLinkedList;
-  friend T;
+  friend struct DoublyLinkedSiblingAccess<T>;
   T* mNext;
   T* mPrev;
 
 public:
   DoublyLinkedListElement() : mNext(nullptr), mPrev(nullptr) {}
 };
 
 /**
- * Provides access to a DoublyLinkedListElement within T.
- *
- * The default implementation of this template works for types that derive
- * from DoublyLinkedListElement, but one can specialize for their class so
- * that some appropriate DoublyLinkedListElement reference is returned.
- *
- * For more complex cases (multiple DoublyLinkedListElements, for example),
- * one can define their own trait class and use that as ElementAccess for
- * DoublyLinkedList. See TestDoublyLinkedList.cpp for an example.
- */
-template <typename T>
-struct GetDoublyLinkedListElement
-{
-  static_assert(mozilla::IsBaseOf<DoublyLinkedListElement<T>, T>::value,
-                "You need your own specialization of GetDoublyLinkedListElement"
-                " or use a separate Trait.");
-  static DoublyLinkedListElement<T>& Get(T* aThis)
-  {
-    return *aThis;
-  }
-};
-
-/**
  * A doubly linked list. |T| is the type of element stored in this list. |T|
  * must contain or have access to unique next and previous element pointers.
- * The template argument |ElementAccess| provides code to tell this list how to
- * get a reference to a DoublyLinkedListElement that may reside anywhere.
+ * The template argument |SiblingAccess| provides code to tell this list how to
+ * get and set the next and previous pointers. The actual storage of next/prev
+ * links may reside anywhere and be encoded in any way.
  */
-template <typename T, typename ElementAccess = GetDoublyLinkedListElement<T>>
+template <typename T, typename SiblingAccess = DoublyLinkedSiblingAccess<T>>
 class DoublyLinkedList final
 {
   T* mHead;
   T* mTail;
 
   /**
    * Checks that either the list is empty and both mHead and mTail are nullptr
    * or the list has entries and both mHead and mTail are non-null.
    */
   bool isStateValid() const {
     return (mHead != nullptr) == (mTail != nullptr);
   }
 
   static bool ElementNotInList(T* aElm) {
-    return !ElementAccess::Get(aElm).mNext && !ElementAccess::Get(aElm).mPrev;
+    return !SiblingAccess::GetNext(aElm) && !SiblingAccess::GetPrev(aElm);
   }
 
 public:
   DoublyLinkedList() : mHead(nullptr), mTail(nullptr) {}
 
   class Iterator final {
     T* mCurrent;
 
@@ -142,28 +136,28 @@ public:
 
     Iterator() : mCurrent(nullptr) {}
     explicit Iterator(T* aCurrent) : mCurrent(aCurrent) {}
 
     T& operator *() const { return *mCurrent; }
     T* operator ->() const { return mCurrent; }
 
     Iterator& operator++() {
-      mCurrent = ElementAccess::Get(mCurrent).mNext;
+      mCurrent = SiblingAccess::GetNext(mCurrent);
       return *this;
     }
 
     Iterator operator++(int) {
       Iterator result = *this;
       ++(*this);
       return result;
     }
 
     Iterator& operator--() {
-      mCurrent = ElementAccess::Get(mCurrent).mPrev;
+      mCurrent = SiblingAccess::GetPrev(mCurrent);
       return *this;
     }
 
     Iterator operator--(int) {
       Iterator result = *this;
       --(*this);
       return result;
     }
@@ -201,20 +195,20 @@ public:
    * Inserts aElm into the list at the head position. |aElm| must not already
    * be in a list.
    */
   void pushFront(T* aElm) {
     MOZ_ASSERT(aElm);
     MOZ_ASSERT(ElementNotInList(aElm));
     MOZ_ASSERT(isStateValid());
 
-    ElementAccess::Get(aElm).mNext = mHead;
+    SiblingAccess::SetNext(aElm, mHead);
     if (mHead) {
-      MOZ_ASSERT(!ElementAccess::Get(mHead).mPrev);
-      ElementAccess::Get(mHead).mPrev = aElm;
+      MOZ_ASSERT(!SiblingAccess::GetPrev(mHead));
+      SiblingAccess::SetPrev(mHead, aElm);
     }
 
     mHead = aElm;
     if (!mTail) {
       mTail = aElm;
     }
   }
 
@@ -222,47 +216,47 @@ public:
    * Remove the head of the list and return it. Calling this on an empty list
    * will assert.
    */
   T* popFront() {
     MOZ_ASSERT(!isEmpty());
     MOZ_ASSERT(isStateValid());
 
     T* result = mHead;
-    mHead = result ? ElementAccess::Get(result).mNext : nullptr;
+    mHead = result ? SiblingAccess::GetNext(result) : nullptr;
     if (mHead) {
-      ElementAccess::Get(mHead).mPrev = nullptr;
+      SiblingAccess::SetPrev(mHead, nullptr);
     }
 
     if (mTail == result) {
       mTail = nullptr;
     }
 
     if (result) {
-      ElementAccess::Get(result).mNext = nullptr;
-      ElementAccess::Get(result).mPrev = nullptr;
+      SiblingAccess::SetNext(result, nullptr);
+      SiblingAccess::SetPrev(result, nullptr);
     }
 
     return result;
   }
 
   /**
    * Inserts aElm into the list at the tail position. |aElm| must not already
    * be in a list.
    */
   void pushBack(T* aElm) {
     MOZ_ASSERT(aElm);
     MOZ_ASSERT(ElementNotInList(aElm));
     MOZ_ASSERT(isStateValid());
 
-    ElementAccess::Get(aElm).mNext = nullptr;
-    ElementAccess::Get(aElm).mPrev = mTail;
+    SiblingAccess::SetNext(aElm, nullptr);
+    SiblingAccess::SetPrev(aElm, mTail);
     if (mTail) {
-      MOZ_ASSERT(!ElementAccess::Get(mTail).mNext);
-      ElementAccess::Get(mTail).mNext = aElm;
+      MOZ_ASSERT(!SiblingAccess::GetNext(mTail));
+      SiblingAccess::SetNext(mTail, aElm);
     }
 
     mTail = aElm;
     if (!mHead) {
       mHead = aElm;
     }
   }
 
@@ -270,28 +264,28 @@ public:
    * Remove the tail of the list and return it. Calling this on an empty list
    * will assert.
    */
   T* popBack() {
     MOZ_ASSERT(!isEmpty());
     MOZ_ASSERT(isStateValid());
 
     T* result = mTail;
-    mTail = result ? ElementAccess::Get(result).mPrev : nullptr;
+    mTail = result ? SiblingAccess::GetPrev(result) : nullptr;
     if (mTail) {
-      ElementAccess::Get(mTail).mNext = nullptr;
+      SiblingAccess::SetNext(mTail, nullptr);
     }
 
     if (mHead == result) {
       mHead = nullptr;
     }
 
     if (result) {
-      ElementAccess::Get(result).mNext = nullptr;
-      ElementAccess::Get(result).mPrev = nullptr;
+      SiblingAccess::SetNext(result, nullptr);
+      SiblingAccess::SetPrev(result, nullptr);
     }
 
     return result;
   }
 
   /**
    * Insert the given |aElm| *before* |aIter|.
    */
@@ -302,50 +296,50 @@ public:
 
     if (!aIter) {
       return pushBack(aElm);
     } else if (aIter == begin()) {
       return pushFront(aElm);
     }
 
     T* after = &(*aIter);
-    T* before = ElementAccess::Get(after).mPrev;
+    T* before = SiblingAccess::GetPrev(after);
     MOZ_ASSERT(before);
 
-    ElementAccess::Get(before).mNext = aElm;
-    ElementAccess::Get(aElm).mPrev = before;
-    ElementAccess::Get(aElm).mNext = after;
-    ElementAccess::Get(after).mPrev = aElm;
+    SiblingAccess::SetNext(before, aElm);
+    SiblingAccess::SetPrev(aElm, before);
+    SiblingAccess::SetNext(aElm, after);
+    SiblingAccess::SetPrev(after, aElm);
   }
 
   /**
    * Removes the given element from the list. The element must be in this list.
    */
   void remove(T* aElm) {
     MOZ_ASSERT(aElm);
-    MOZ_ASSERT(ElementAccess::Get(aElm).mNext || ElementAccess::Get(aElm).mPrev ||
+    MOZ_ASSERT(SiblingAccess::GetNext(aElm) || SiblingAccess::GetPrev(aElm) ||
                (aElm == mHead && aElm == mTail),
                "Attempted to remove element not in this list");
 
-    if (T* prev = ElementAccess::Get(aElm).mPrev) {
-      ElementAccess::Get(prev).mNext = ElementAccess::Get(aElm).mNext;
+    if (T* prev = SiblingAccess::GetPrev(aElm)) {
+      SiblingAccess::SetNext(prev, SiblingAccess::GetNext(aElm));
     } else {
       MOZ_ASSERT(mHead == aElm);
-      mHead = ElementAccess::Get(aElm).mNext;
+      mHead = SiblingAccess::GetNext(aElm);
     }
 
-    if (T* next = ElementAccess::Get(aElm).mNext) {
-      ElementAccess::Get(next).mPrev = ElementAccess::Get(aElm).mPrev;
+    if (T* next = SiblingAccess::GetNext(aElm)) {
+      SiblingAccess::SetPrev(next, SiblingAccess::GetPrev(aElm));
     } else {
       MOZ_ASSERT(mTail == aElm);
-      mTail = ElementAccess::Get(aElm).mPrev;
+      mTail = SiblingAccess::GetPrev(aElm);
     }
 
-    ElementAccess::Get(aElm).mNext = nullptr;
-    ElementAccess::Get(aElm).mPrev = nullptr;
+    SiblingAccess::SetNext(aElm, nullptr);
+    SiblingAccess::SetPrev(aElm, nullptr);
   }
 
   /**
    * Returns an iterator referencing the first found element whose value matches
    * the given element according to operator==.
    */
   Iterator find(const T& aElm) {
     return std::find(begin(), end(), aElm);
@@ -353,26 +347,13 @@ public:
 
   /**
    * Returns whether the given element is in the list. Note that this uses
    * T::operator==, not pointer comparison.
    */
   bool contains(const T& aElm) {
     return find(aElm) != Iterator();
   }
-
-  /**
-   * Returns whether the given element might be in the list. Note that this
-   * assumes the element is either in the list or not in the list, and ignores
-   * the case where the element might be in another list in order to make the
-   * check fast.
-   */
-  bool ElementProbablyInList(T* aElm) {
-    if (isEmpty()) {
-      return false;
-    }
-    return !ElementNotInList(aElm);
-  }
 };
 
 } // namespace mozilla
 
 #endif // mozilla_DoublyLinkedList_h
--- a/mfbt/tests/TestDoublyLinkedList.cpp
+++ b/mfbt/tests/TestDoublyLinkedList.cpp
@@ -116,41 +116,42 @@ TestDoublyLinkedList()
 
   MOZ_RELEASE_ASSERT(*list.begin() == two);
   MOZ_RELEASE_ASSERT(*++list.begin() == three);
 
   SomeClass four(4);
   MOZ_RELEASE_ASSERT(++list.begin() == list.find(four));
 }
 
-struct InTwoLists {
-  explicit InTwoLists(unsigned int aValue) : mValue(aValue) {}
-  DoublyLinkedListElement<InTwoLists> mListOne;
-  DoublyLinkedListElement<InTwoLists> mListTwo;
-  unsigned int mValue;
-
-  struct GetListOneTrait {
-    static DoublyLinkedListElement<InTwoLists>& Get(InTwoLists *aThis) { return aThis->mListOne; }
-  };
-};
-
-namespace mozilla {
-
-template<>
-struct GetDoublyLinkedListElement<InTwoLists> {
-  static DoublyLinkedListElement<InTwoLists>& Get(InTwoLists* aThis) { return aThis->mListTwo; }
-};
-
-}
-
 static void
 TestCustomAccessor()
 {
-  DoublyLinkedList<InTwoLists, InTwoLists::GetListOneTrait> listOne;
-  DoublyLinkedList<InTwoLists> listTwo;
+  struct InTwoLists {
+    explicit InTwoLists(unsigned int aValue) : mValue(aValue) {}
+    DoublyLinkedListElement<InTwoLists> mListOne;
+    DoublyLinkedListElement<InTwoLists> mListTwo;
+    unsigned int mValue;
+  };
+
+  struct ListOneSiblingAccess {
+    static void SetNext(InTwoLists* aElm, InTwoLists* aNext) { aElm->mListOne.mNext = aNext; }
+    static InTwoLists* GetNext(InTwoLists* aElm) { return aElm->mListOne.mNext; }
+    static void SetPrev(InTwoLists* aElm, InTwoLists* aPrev) { aElm->mListOne.mPrev = aPrev; }
+    static InTwoLists* GetPrev(InTwoLists* aElm) { return aElm->mListOne.mPrev; }
+  };
+
+  struct ListTwoSiblingAccess {
+    static void SetNext(InTwoLists* aElm, InTwoLists* aNext) { aElm->mListTwo.mNext = aNext; }
+    static InTwoLists* GetNext(InTwoLists* aElm) { return aElm->mListTwo.mNext; }
+    static void SetPrev(InTwoLists* aElm, InTwoLists* aPrev) { aElm->mListTwo.mPrev = aPrev; }
+    static InTwoLists* GetPrev(InTwoLists* aElm) { return aElm->mListTwo.mPrev; }
+  };
+
+  DoublyLinkedList<InTwoLists, ListOneSiblingAccess> listOne;
+  DoublyLinkedList<InTwoLists, ListTwoSiblingAccess> listTwo;
 
   InTwoLists one(1);
   InTwoLists two(2);
 
   listOne.pushBack(&one);
   listOne.pushBack(&two);
   { unsigned int check[] { 1, 2 }; CheckListValues(listOne, check); }