Backed out changeset 8daa186bd18b (bug 1593399) for causing crashes @js::gcstats::Statistics. CLOSED TREE
authorCosmin Sabou <csabou@mozilla.com>
Wed, 13 Nov 2019 07:00:37 +0200
changeset 501702 7d9725c3f834323fa9b39cf1b1d2604ce9d5c35d
parent 501701 f644542d11c15980eed8368d5b52539d571c35ce
child 501703 c73a5bc0e22516c832a374fd0ebf9d6dec3c7886
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1593399
milestone72.0a1
backs out8daa186bd18b8a894f95e22f32c9ecc45481553a
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 8daa186bd18b (bug 1593399) for causing crashes @js::gcstats::Statistics. CLOSED TREE
js/public/HeapAPI.h
js/src/gc/Cell.h
js/src/gc/GC.cpp
js/src/gc/GCMarker.h
js/src/gc/Marking.cpp
js/src/gc/Verifier.cpp
js/src/gc/Verifier.h
js/src/gc/WeakMap-inl.h
js/src/gc/WeakMap.cpp
js/src/gc/WeakMap.h
js/src/gc/Zone.cpp
js/src/jit-test/tests/gc/weak-marking-03.js
js/src/jsapi-tests/testGCGrayMarking.cpp
--- a/js/public/HeapAPI.h
+++ b/js/public/HeapAPI.h
@@ -374,16 +374,18 @@ class JS_FRIEND_API GCCellPtr {
     // Note: the OutOfLineTraceKindMask bits are set on all out-of-line kinds
     // so that we can mask instead of branching.
     MOZ_ASSERT_IF(uintptr_t(traceKind) >= OutOfLineTraceKindMask,
                   (uintptr_t(traceKind) & OutOfLineTraceKindMask) ==
                       OutOfLineTraceKindMask);
     return uintptr_t(p) | (uintptr_t(traceKind) & OutOfLineTraceKindMask);
   }
 
+  bool mayBeOwnedByOtherRuntimeSlow() const;
+
   JS::TraceKind outOfLineKind() const;
 
   uintptr_t ptr;
 } JS_HAZ_GC_POINTER;
 
 // Unwraps the given GCCellPtr, calls the functor |f| with a template argument
 // of the actual type of the pointer, and returns the result.
 template <typename F>
--- a/js/src/gc/Cell.h
+++ b/js/src/gc/Cell.h
@@ -47,64 +47,16 @@ enum class AllocKind : uint8_t;
 struct Chunk;
 class StoreBuffer;
 class TenuredCell;
 
 #ifdef DEBUG
 extern bool CurrentThreadIsGCMarking();
 #endif
 
-// Like gc::MarkColor but allows the possibility of the cell being unmarked.
-//
-// This class mimics an enum class, but supports operator overloading.
-class CellColor {
- public:
-  enum Color { White = 0, Gray = 1, Black = 2 };
-
-  CellColor() : color(White) {}
-
-  MOZ_IMPLICIT CellColor(MarkColor markColor)
-      : color(markColor == MarkColor::Black ? Black : Gray) {}
-
-  MOZ_IMPLICIT constexpr CellColor(Color c) : color(c) {}
-
-  MarkColor asMarkColor() const {
-    MOZ_ASSERT(color != White);
-    return color == Black ? MarkColor::Black : MarkColor::Gray;
-  }
-
-  // Implement a total ordering for CellColor, with white being 'least marked'
-  // and black being 'most marked'.
-  bool operator<(const CellColor other) const { return color < other.color; }
-  bool operator>(const CellColor other) const { return color > other.color; }
-  bool operator<=(const CellColor other) const { return color <= other.color; }
-  bool operator>=(const CellColor other) const { return color >= other.color; }
-  bool operator!=(const CellColor other) const { return color != other.color; }
-  bool operator==(const CellColor other) const { return color == other.color; }
-  explicit operator bool() const { return color != White; }
-
-#if defined(JS_GC_ZEAL) || defined(DEBUG)
-  const char* name() const {
-    switch (color) {
-      case CellColor::White:
-        return "white";
-      case CellColor::Black:
-        return "black";
-      case CellColor::Gray:
-        return "gray";
-      default:
-        MOZ_CRASH("Unexpected cell color");
-    }
-  }
-#endif
-
- private:
-  Color color;
-};
-
 // [SMDOC] GC Cell
 //
 // A GC cell is the base class for all GC things. All types allocated on the GC
 // heap extend either gc::Cell or gc::TenuredCell. If a type is always tenured,
 // prefer the TenuredCell class as base.
 //
 // The first word (a pointer or uintptr_t) of each Cell must reserve the low
 // Cell::ReservedBits bits for GC purposes. The remaining bits are available to
@@ -133,22 +85,16 @@ struct alignas(gc::CellAlignBytes) Cell 
   MOZ_ALWAYS_INLINE TenuredCell& asTenured();
 
   MOZ_ALWAYS_INLINE bool isMarkedAny() const;
   MOZ_ALWAYS_INLINE bool isMarkedBlack() const;
   MOZ_ALWAYS_INLINE bool isMarkedGray() const;
   MOZ_ALWAYS_INLINE bool isMarked(gc::MarkColor color) const;
   MOZ_ALWAYS_INLINE bool isMarkedAtLeast(gc::MarkColor color) const;
 
-  MOZ_ALWAYS_INLINE CellColor color() const {
-    return isMarkedBlack()
-               ? CellColor::Black
-               : isMarkedGray() ? CellColor::Gray : CellColor::White;
-  }
-
   inline JSRuntime* runtimeFromMainThread() const;
 
   // Note: Unrestricted access to the runtime of a GC thing from an arbitrary
   // thread can easily lead to races. Use this method very carefully.
   inline JSRuntime* runtimeFromAnyThread() const;
 
   // May be overridden by GC thing kinds that have a compartment pointer.
   inline JS::Compartment* maybeCompartment() const { return nullptr; }
@@ -218,23 +164,16 @@ class TenuredCell : public Cell {
     return true;
   }
 
   // Mark bit management.
   MOZ_ALWAYS_INLINE bool isMarkedAny() const;
   MOZ_ALWAYS_INLINE bool isMarkedBlack() const;
   MOZ_ALWAYS_INLINE bool isMarkedGray() const;
 
-  // Same as Cell::color, but skips nursery checks.
-  MOZ_ALWAYS_INLINE CellColor color() const {
-    return isMarkedBlack()
-               ? CellColor::Black
-               : isMarkedGray() ? CellColor::Gray : CellColor::White;
-  }
-
   // The return value indicates if the cell went from unmarked to marked.
   MOZ_ALWAYS_INLINE bool markIfUnmarked(
       MarkColor color = MarkColor::Black) const;
   MOZ_ALWAYS_INLINE void markBlack() const;
   MOZ_ALWAYS_INLINE void copyMarkBitsFrom(const TenuredCell* src);
   MOZ_ALWAYS_INLINE void unmark();
 
   // Access to the arena.
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -4295,17 +4295,17 @@ void js::gc::MarkingValidator::nonIncrem
     }
   }
 
   /*
    * Temporarily clear the weakmaps' mark flags for the compartments we are
    * collecting.
    */
 
-  WeakMapColors markedWeakMaps;
+  WeakMapSet markedWeakMaps;
 
   /*
    * For saving, smush all of the keys into one big table and split them back
    * up into per-zone tables when restoring.
    */
   gc::WeakKeyTable savedWeakKeys(SystemAllocPolicy(),
                                  runtime->randomHashCodeScrambler());
   if (!savedWeakKeys.init()) {
--- a/js/src/gc/GCMarker.h
+++ b/js/src/gc/GCMarker.h
@@ -279,16 +279,31 @@ class GCMarker : public JSTracer {
   void setMarkColorBlack();
   void setMarkColor(gc::MarkColor newColor);
   gc::MarkColor markColor() const { return color; }
 
   // Declare which color the main mark stack will be used for. The whole stack
   // must be empty when this is called.
   void setMainStackColor(gc::MarkColor newColor);
 
+  // Return whether a cell is marked relative to the current marking color. If
+  // the cell is black then this returns true, but if it's gray it will return
+  // false if the mark color is black.
+  template <typename T>
+  bool isMarked(T* thingp) {
+    return color == gc::MarkColor::Black ? gc::IsMarkedBlack(runtime(), thingp)
+                                         : gc::IsMarked(runtime(), thingp);
+  }
+  template <typename T>
+  bool isMarkedUnbarriered(T* thingp) {
+    return color == gc::MarkColor::Black
+               ? gc::IsMarkedBlackUnbarriered(runtime(), thingp)
+               : gc::IsMarkedUnbarriered(runtime(), thingp);
+  }
+
   void enterWeakMarkingMode();
   void leaveWeakMarkingMode();
   void abortLinearWeakMarking() {
     leaveWeakMarkingMode();
     linearWeakMarkingDisabled_ = true;
   }
 
   void delayMarkingChildren(gc::Cell* cell);
@@ -493,19 +508,16 @@ class MOZ_RAII AutoSetMarkColor {
   MarkColor initialColor_;
 
  public:
   AutoSetMarkColor(GCMarker& marker, MarkColor newColor)
       : marker_(marker), initialColor_(marker.markColor()) {
     marker_.setMarkColor(newColor);
   }
 
-  AutoSetMarkColor(GCMarker& marker, CellColor newColor)
-      : AutoSetMarkColor(marker, newColor.asMarkColor()) {}
-
   ~AutoSetMarkColor() { marker_.setMarkColor(initialColor_); }
 };
 
 } /* namespace gc */
 
 } /* namespace js */
 
 // Exported for Tracer.cpp
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -7,17 +7,16 @@
 #include "gc/Marking-inl.h"
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/IntegerRange.h"
 #include "mozilla/ReentrancyGuard.h"
 #include "mozilla/ScopeExit.h"
 #include "mozilla/TypeTraits.h"
-#include "mozilla/Unused.h"
 
 #include <algorithm>
 
 #include "jsfriendapi.h"
 
 #include "builtin/ModuleObject.h"
 #include "debugger/DebugAPI.h"
 #include "gc/GCInternals.h"
@@ -37,17 +36,16 @@
 #include "vm/Shape.h"
 #include "vm/SymbolType.h"
 #include "vm/TypedArrayObject.h"
 #include "wasm/WasmJS.h"
 
 #include "gc/GC-inl.h"
 #include "gc/Nursery-inl.h"
 #include "gc/PrivateIterators-inl.h"
-#include "gc/WeakMap-inl.h"
 #include "gc/Zone-inl.h"
 #include "vm/GeckoProfiler-inl.h"
 #include "vm/NativeObject-inl.h"
 #include "vm/Realm-inl.h"
 #include "vm/StringType-inl.h"
 
 using namespace js;
 using namespace js::gc;
@@ -651,17 +649,22 @@ struct ImplicitEdgeHolderType<LazyScript
   typedef LazyScript* Type;
 };
 
 void GCMarker::markEphemeronValues(gc::Cell* markedCell,
                                    WeakEntryVector& values) {
   DebugOnly<size_t> initialLen = values.length();
 
   for (const auto& markable : values) {
-    markable.weakmap->markKey(this, markedCell, markable.key);
+    if (color == gc::MarkColor::Black &&
+        markable.weakmap->markColor == gc::MarkColor::Gray) {
+      continue;
+    }
+
+    markable.weakmap->markEntry(this, markedCell, markable.key);
   }
 
   // The vector should not be appended to during iteration because the key is
   // already marked, and even in cases where we have a multipart key, we
   // should only be inserting entries for the unmarked portions.
   MOZ_ASSERT(values.length() == initialLen);
 }
 
@@ -2599,18 +2602,18 @@ void GCMarker::enterWeakMarkingMode() {
     // If there was an 'enter-weak-marking-mode' token in the queue, then it
     // and everything after it will still be in the queue so we can process
     // them now.
     while (processMarkQueue() == QueueYielded) {
     };
 
     for (SweepGroupZonesIter zone(runtime()); !zone.done(); zone.next()) {
       for (WeakMapBase* m : zone->gcWeakMapList()) {
-        if (m->mapColor) {
-          mozilla::Unused << m->markEntries(this);
+        if (m->marked) {
+          (void)m->markEntries(this);
         }
       }
     }
   }
 }
 
 void GCMarker::leaveWeakMarkingMode() {
   MOZ_ASSERT_IF(
--- a/js/src/gc/Verifier.cpp
+++ b/js/src/gc/Verifier.cpp
@@ -5,18 +5,16 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "gc/Verifier.h"
 
 #include "mozilla/DebugOnly.h"
 #include "mozilla/IntegerPrintfMacros.h"
 #include "mozilla/Sprintf.h"
 
-#include <algorithm>
-
 #ifdef MOZ_VALGRIND
 #  include <valgrind/memcheck.h>
 #endif
 
 #include "gc/GCInternals.h"
 #include "gc/GCLock.h"
 #include "gc/PublicIterators.h"
 #include "gc/WeakMap.h"
@@ -450,16 +448,33 @@ void js::gc::GCRuntime::finishVerifier()
     verifyPreData = nullptr;
   }
 }
 
 #endif /* JS_GC_ZEAL */
 
 #if defined(JS_GC_ZEAL) || defined(DEBUG)
 
+static const char* CellColorName(CellColor color) {
+  switch (color) {
+    case CellColor::White:
+      return "white";
+    case CellColor::Black:
+      return "black";
+    case CellColor::Gray:
+      return "gray";
+    default:
+      MOZ_CRASH("Unexpected cell color");
+  }
+}
+
+static const char* GetCellColorName(Cell* cell) {
+  return CellColorName(GetCellColor(cell));
+}
+
 class HeapCheckTracerBase : public JS::CallbackTracer {
  public:
   explicit HeapCheckTracerBase(JSRuntime* rt, WeakMapTraceKind weakTraceKind);
   bool traceHeap(AutoTraceSession& session);
   virtual void checkCell(Cell* cell) = 0;
 
  protected:
   void dumpCellInfo(Cell* cell);
@@ -567,17 +582,17 @@ bool HeapCheckTracerBase::traceHeap(Auto
   return !oom;
 }
 
 void HeapCheckTracerBase::dumpCellInfo(Cell* cell) {
   auto kind = cell->getTraceKind();
   JSObject* obj =
       kind == JS::TraceKind::Object ? static_cast<JSObject*>(cell) : nullptr;
 
-  fprintf(stderr, "%s %s", cell->color().name(), GCTraceKindToAscii(kind));
+  fprintf(stderr, "%s %s", GetCellColorName(cell), GCTraceKindToAscii(kind));
   if (obj) {
     fprintf(stderr, " %s", obj->getClass()->name);
   }
   fprintf(stderr, " %p", cell);
   if (obj) {
     fprintf(stderr, " (compartment %p)", obj->compartment());
   }
 }
@@ -749,36 +764,41 @@ bool js::gc::CheckWeakMapEntryMarking(co
   // Debugger weak maps can have keys in different zones.
   Zone* keyZone = GetCellZoneFromAnyThread(key);
   MOZ_ASSERT_IF(!map->allowKeysInOtherZones(),
                 keyZone == zone || keyZone->isAtomsZone());
 
   Zone* valueZone = GetCellZoneFromAnyThread(value);
   MOZ_ASSERT(valueZone == zone || valueZone->isAtomsZone());
 
-  if (object && object->color() != map->mapColor) {
+  CellColor mapColor =
+      map->markColor == MarkColor::Black ? CellColor::Black : CellColor::Gray;
+  if (object && GetCellColor(object) != mapColor) {
     fprintf(stderr, "WeakMap object is marked differently to the map\n");
     fprintf(stderr, "(map %p is %s, object %p is %s)\n", map,
-            map->mapColor.name(), object, object->color().name());
+            CellColorName(mapColor), object,
+            CellColorName(GetCellColor(object)));
     ok = false;
   }
 
+  CellColor keyColor = GetCellColor(key);
+
   // Values belonging to other runtimes or in uncollected zones are treated as
   // black.
   CellColor valueColor = CellColor::Black;
   if (value->runtimeFromAnyThread() == zone->runtimeFromAnyThread() &&
       valueZone->isGCMarking()) {
-    valueColor = value->color();
+    valueColor = GetCellColor(value);
   }
 
-  if (valueColor < std::min(map->mapColor, key->color())) {
+  if (valueColor < ExpectedWeakMapValueColor(mapColor, keyColor)) {
     fprintf(stderr, "WeakMap value is less marked than map and key\n");
     fprintf(stderr, "(map %p is %s, key %p is %s, value %p is %s)\n", map,
-            map->mapColor.name(), key, key->color().name(), value,
-            valueColor.name());
+            CellColorName(mapColor), key, CellColorName(keyColor), value,
+            CellColorName(valueColor));
     ok = false;
   }
 
   // Debugger weak maps map have keys in zones that are not or are
   // no longer collecting. We can't make guarantees about the mark
   // state of these keys.
   if (map->allowKeysInOtherZones() &&
       !(keyZone->isGCMarking() || keyZone->isGCSweeping())) {
@@ -787,26 +807,26 @@ bool js::gc::CheckWeakMapEntryMarking(co
 
   JSObject* delegate = MaybeGetDelegate(key);
   if (!delegate) {
     return ok;
   }
 
   CellColor delegateColor;
   if (delegate->zone()->isGCMarking() || delegate->zone()->isGCSweeping()) {
-    delegateColor = delegate->color();
+    delegateColor = GetCellColor(delegate);
   } else {
     // IsMarked() assumes cells in uncollected zones are marked.
     delegateColor = CellColor::Black;
   }
 
-  if (key->color() < std::min(map->mapColor, delegateColor)) {
-    fprintf(stderr, "WeakMap key is less marked than map or delegate\n");
+  if (keyColor < ExpectedWeakMapKeyColor(mapColor, delegateColor)) {
+    fprintf(stderr, "WeakMap key is less marked than map and delegate\n");
     fprintf(stderr, "(map %p is %s, delegate %p is %s, key %p is %s)\n", map,
-            map->mapColor.name(), delegate, delegateColor.name(), key,
-            key->color().name());
+            CellColorName(mapColor), delegate, CellColorName(delegateColor),
+            key, CellColorName(keyColor));
     ok = false;
   }
 
   return ok;
 }
 
 #endif  // defined(JS_GC_ZEAL) || defined(DEBUG)
--- a/js/src/gc/Verifier.h
+++ b/js/src/gc/Verifier.h
@@ -13,18 +13,53 @@
 
 #include <algorithm>
 
 #include "gc/Cell.h"
 
 namespace js {
 namespace gc {
 
+// Like gc::MarkColor but allows the possibility of the cell being
+// unmarked.
+enum class CellColor : uint8_t {
+  White = 0,
+  Gray = uint8_t(MarkColor::Gray),
+  Black = uint8_t(MarkColor::Black)
+};
+
 static constexpr CellColor AllCellColors[] = {CellColor::White, CellColor::Gray,
                                               CellColor::Black};
 
 static constexpr CellColor MarkedCellColors[] = {CellColor::Gray,
                                                  CellColor::Black};
 
+inline CellColor GetCellColor(Cell* cell) {
+  if (cell->isMarkedBlack()) {
+    return CellColor::Black;
+  }
+
+  if (cell->isMarkedGray()) {
+    return CellColor::Gray;
+  }
+
+  return CellColor::White;
+}
+
+inline CellColor ExpectedWeakMapValueColor(CellColor keyColor,
+                                           CellColor mapColor) {
+  return std::min(keyColor, mapColor);
+}
+
+inline CellColor ExpectedWeakMapKeyColor(CellColor mapColor,
+                                         CellColor delegateColor) {
+  return std::min(mapColor, delegateColor);
+}
+
+inline CellColor ExpectedKeyAndDelegateColor(CellColor keyColor,
+                                             CellColor delegateColor) {
+  return std::max(keyColor, delegateColor);
+}
+
 }  // namespace gc
 }  // namespace js
 
 #endif /* gc_Verifier_h */
--- a/js/src/gc/WeakMap-inl.h
+++ b/js/src/gc/WeakMap-inl.h
@@ -4,181 +4,117 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef gc_WeakMap_inl_h
 #define gc_WeakMap_inl_h
 
 #include "gc/WeakMap.h"
 
-#include "mozilla/DebugOnly.h"
-#include "mozilla/Unused.h"
-
-#include <algorithm>
-
 #include "gc/Zone.h"
 #include "js/TraceKind.h"
 #include "vm/JSContext.h"
 
 namespace js {
-namespace gc {
-
-// Specializations for barriered types.
-template <typename T>
-inline Cell* ToMarkable(WriteBarriered<T>* thingp) {
-  return ToMarkable(thingp->get());
-}
-
-namespace detail {
 
 template <typename T>
-static T ExtractUnbarriered(const WriteBarriered<T>& v) {
+static T extractUnbarriered(const WriteBarriered<T>& v) {
   return v.get();
 }
 
 template <typename T>
-static T* ExtractUnbarriered(T* v) {
+static T* extractUnbarriered(T* v) {
   return v;
 }
 
-// Return the effective cell color given the current marking state.
-// This must be kept in sync with ShouldMark in Marking.cpp.
-template <typename T>
-static CellColor GetEffectiveColor(JSRuntime* rt, const T& item) {
-  Cell* cell = ToMarkable(item);
-  if (!cell->isTenured()) {
-    return CellColor::Black;
+inline /* static */ JSObject* WeakMapBase::getDelegate(JSObject* key) {
+  if (!IsWrapper(key)) {
+    return nullptr;
   }
-  const TenuredCell& t = cell->asTenured();
-  if (rt != t.runtimeFromAnyThread()) {
-    return CellColor::Black;
-  }
-  if (!t.zoneFromAnyThread()->shouldMarkInZone()) {
-    return CellColor::Black;
-  }
-  return cell->color();
+
+  return UncheckedUnwrapWithoutExpose(key);
 }
 
-// Only objects have delegates, so default to returning nullptr. Note that some
-// compilation units will only ever use the object version.
-static MOZ_MAYBE_UNUSED JSObject* GetDelegateInternal(gc::Cell* key) {
+inline /* static */ JSObject* WeakMapBase::getDelegate(JSScript* script) {
   return nullptr;
 }
 
-static JSObject* GetDelegateInternal(JSObject* key) {
-  JSObject* delegate = UncheckedUnwrapWithoutExpose(key);
-  return (key == delegate) ? nullptr : delegate;
+inline /* static */ JSObject* WeakMapBase::getDelegate(LazyScript* script) {
+  return nullptr;
 }
 
-// Use a helper function to do overload resolution to handle cases like
-// Heap<ObjectSubclass*>: find everything that is convertible to JSObject* (and
-// avoid calling barriers).
-template <typename T>
-static inline JSObject* GetDelegate(const T& key) {
-  return GetDelegateInternal(ExtractUnbarriered(key));
-}
-
-template <>
-inline JSObject* GetDelegate(gc::Cell* const&) = delete;
-
-} /* namespace detail */
-} /* namespace gc */
-
 template <class K, class V>
 WeakMap<K, V>::WeakMap(JSContext* cx, JSObject* memOf)
     : Base(cx->zone()), WeakMapBase(memOf, cx->zone()) {
   using ElemType = typename K::ElementType;
   using NonPtrType = typename mozilla::RemovePointer<ElemType>::Type;
 
   // The object's TraceKind needs to be added to CC graph if this object is
   // used as a WeakMap key, otherwise the key is considered to be pointed from
   // somewhere unknown, and results in leaking the subgraph which contains the
   // key. See the comments in NoteWeakMapsTracer::trace for more details.
   static_assert(JS::IsCCTraceKind(NonPtrType::TraceKind),
                 "Object's TraceKind should be added to CC graph.");
 
   zone()->gcWeakMapList().insertFront(this);
   if (zone()->wasGCStarted()) {
-    mapColor = CellColor::Black;
+    marked = true;
+    markColor = gc::MarkColor::Black;
   }
 }
 
 // Trace a WeakMap entry based on 'markedCell' getting marked, where 'origKey'
-// is the key in the weakmap. In the absence of delegates, these will be the
-// same, but when a delegate is marked then origKey will be its wrapper.
-// `markedCell` is only used for an assertion.
+// is the key in the weakmap. These will probably be the same, but can be
+// different eg when markedCell is a delegate for origKey.
+//
+// This implementation does not use 'markedCell'; it looks up origKey and checks
+// the mark bits on everything it cares about, one of which will be
+// markedCell. But a subclass might use it to optimize the liveness check.
 template <class K, class V>
-void WeakMap<K, V>::markKey(GCMarker* marker, gc::Cell* markedCell,
-                            gc::Cell* origKey) {
-  MOZ_ASSERT(mapColor);
+void WeakMap<K, V>::markEntry(GCMarker* marker, gc::Cell* markedCell,
+                              gc::Cell* origKey) {
+  MOZ_ASSERT(marked);
 
   Ptr p = Base::lookup(static_cast<Lookup>(origKey));
   MOZ_ASSERT(p.found());
 
-  mozilla::DebugOnly<gc::Cell*> oldKey = gc::ToMarkable(p->key());
-  MOZ_ASSERT((markedCell == oldKey) ||
-             (markedCell == gc::detail::GetDelegate(p->key())));
-
-  markEntry(marker, p->mutableKey(), p->value());
-  MOZ_ASSERT(oldKey == gc::ToMarkable(p->key()), "no moving GC");
-}
-
-// If the entry is live, ensure its key and value are marked. Also make sure
-// the key is at least as marked as the delegate, so it cannot get discarded
-// and then recreated by rewrapping the delegate.
-template <class K, class V>
-bool WeakMap<K, V>::markEntry(GCMarker* marker, K& key, V& value) {
-  bool marked = false;
-  JSRuntime* rt = zone()->runtimeFromAnyThread();
-  CellColor keyColor = gc::detail::GetEffectiveColor(rt, key);
-  JSObject* delegate = gc::detail::GetDelegate(key);
-  if (delegate) {
-    CellColor delegateColor = gc::detail::GetEffectiveColor(rt, delegate);
-    if (keyColor < delegateColor) {
-      gc::AutoSetMarkColor autoColor(*marker, delegateColor);
-      TraceEdge(marker, &key, "proxy-preserved WeakMap entry key");
-      MOZ_ASSERT(key->color() >= delegateColor);
-      marked = true;
-      keyColor = delegateColor;
-    }
+  K key(p->key());
+  MOZ_ASSERT((markedCell == extractUnbarriered(key)) ||
+             (markedCell == getDelegate(key)));
+  if (marker->isMarked(&key)) {
+    TraceEdge(marker, &p->value(), "ephemeron value");
+  } else if (keyNeedsMark(marker, key)) {
+    TraceEdge(marker, &p->value(), "WeakMap ephemeron value");
+    TraceEdge(marker, &key, "proxy-preserved WeakMap ephemeron key");
+    MOZ_ASSERT(key == p->key());  // No moving
   }
-
-  if (keyColor) {
-    gc::Cell* cellValue = gc::ToMarkable(&value);
-    if (cellValue) {
-      gc::AutoSetMarkColor autoColor(*marker, std::min(mapColor, keyColor));
-      CellColor valueColor = gc::detail::GetEffectiveColor(rt, cellValue);
-      if (valueColor < marker->markColor()) {
-        TraceEdge(marker, &value, "WeakMap entry value");
-        MOZ_ASSERT(cellValue->color() >= std::min(mapColor, keyColor));
-        marked = true;
-      }
-    }
-  }
-
-  return marked;
+  key.unsafeSet(nullptr);  // Prevent destructor from running barriers.
 }
 
 template <class K, class V>
 void WeakMap<K, V>::trace(JSTracer* trc) {
   MOZ_ASSERT_IF(JS::RuntimeHeapIsBusy(), isInList());
 
   TraceNullableEdge(trc, &memberOf, "WeakMap owner");
 
   if (trc->isMarkingTracer()) {
     MOZ_ASSERT(trc->weakMapAction() == ExpandWeakMaps);
     auto marker = GCMarker::fromTracer(trc);
 
-    // Don't downgrade the map color from black to gray. This can happen when a
-    // barrier pushes the map object onto the black mark stack when it's
-    // already present on the gray mark stack, which is marked later.
-    if (mapColor < marker->markColor()) {
-      mapColor = marker->markColor();
-      mozilla::Unused << markEntries(marker);
+    // Don't change the map color from black to gray. This can happen when a
+    // barrier pushes the map object onto the black mark stack when it's already
+    // present on the gray mark stack, which is marked later.
+    if (marked && markColor == gc::MarkColor::Black &&
+        marker->markColor() == gc::MarkColor::Gray) {
+      return;
     }
+
+    marked = true;
+    markColor = marker->markColor();
+    (void)markEntries(marker);
     return;
   }
 
   if (trc->weakMapAction() == DoNotTraceWeakMaps) {
     return;
   }
 
   // Trace keys only if weakMapAction() says to.
@@ -213,51 +149,79 @@ template <class K, class V>
     if (!weakKeys.put(key, std::move(weakEntries))) {
       marker->abortLinearWeakMarking();
     }
   }
 }
 
 template <class K, class V>
 bool WeakMap<K, V>::markEntries(GCMarker* marker) {
-  MOZ_ASSERT(mapColor);
+  MOZ_ASSERT(marked);
+  if (marker->markColor() == gc::MarkColor::Black &&
+      markColor == gc::MarkColor::Gray) {
+    return false;
+  }
+
   bool markedAny = false;
 
   for (Enum e(*this); !e.empty(); e.popFront()) {
-    if (markEntry(marker, e.front().mutableKey(), e.front().value())) {
+    // If the entry is live, ensure its key and value are marked.
+    bool keyIsMarked = marker->isMarked(&e.front().mutableKey());
+    if (!keyIsMarked && keyNeedsMark(marker, e.front().key())) {
+      TraceEdge(marker, &e.front().mutableKey(),
+                "proxy-preserved WeakMap entry key");
+      keyIsMarked = true;
       markedAny = true;
     }
 
-    JSRuntime* rt = zone()->runtimeFromAnyThread();
-    CellColor keyColor =
-        gc::detail::GetEffectiveColor(rt, e.front().key().get());
-
-    // Changes in the map's mark color will be handled in this code, but
-    // changes in the key's mark color are handled through the weak keys table.
-    // So we only need to populate the table if the key is less marked than the
-    // map, to catch later updates in the key's mark color.
-    if (keyColor < mapColor) {
-      MOZ_ASSERT(marker->weakMapAction() == ExpandWeakMaps);
-      // The final color of the key is not yet known. Record this weakmap and
-      // the lookup key in the list of weak keys. If the key has a delegate,
-      // then the lookup key is the delegate (because marking the key will end
-      // up marking the delegate and thereby mark the entry.)
-      gc::Cell* weakKey = gc::detail::ExtractUnbarriered(e.front().key());
+    if (keyIsMarked) {
+      if (!marker->isMarked(&e.front().value())) {
+        TraceEdge(marker, &e.front().value(), "WeakMap entry value");
+        markedAny = true;
+      }
+    } else if (marker->isWeakMarkingTracer()) {
+      // Entry is not yet known to be live. Record this weakmap and
+      // the lookup key in the list of weak keys. Also record the
+      // delegate, if any, because marking the delegate also marks
+      // the entry.
+      gc::Cell* weakKey = extractUnbarriered(e.front().key());
       gc::WeakMarkable markable(this, weakKey);
       addWeakEntry(marker, weakKey, markable);
-      if (JSObject* delegate = gc::detail::GetDelegate(e.front().key())) {
+      if (JSObject* delegate = getDelegate(e.front().key())) {
         addWeakEntry(marker, delegate, markable);
       }
     }
   }
 
   return markedAny;
 }
 
 template <class K, class V>
+inline bool WeakMap<K, V>::keyNeedsMark(GCMarker* marker, JSObject* key) const {
+  JSObject* delegate = getDelegate(key);
+  /*
+   * Check if the delegate is marked with any color to properly handle
+   * gray marking when the key's delegate is black and the map is gray.
+   */
+  return delegate && marker->isMarkedUnbarriered(&delegate);
+}
+
+template <class K, class V>
+inline bool WeakMap<K, V>::keyNeedsMark(GCMarker* marker,
+                                        JSScript* script) const {
+  return false;
+}
+
+template <class K, class V>
+inline bool WeakMap<K, V>::keyNeedsMark(GCMarker* marker,
+                                        LazyScript* script) const {
+  return false;
+}
+
+template <class K, class V>
 void WeakMap<K, V>::sweep() {
   /* Remove all entries whose keys remain unmarked. */
   for (Enum e(*this); !e.empty(); e.popFront()) {
     if (gc::IsAboutToBeFinalized(&e.front().mutableKey())) {
       e.removeFront();
     }
   }
 
@@ -282,17 +246,17 @@ void WeakMap<K, V>::traceMappings(WeakMa
 }
 
 #if DEBUG
 template <class K, class V>
 void WeakMap<K, V>::assertEntriesNotAboutToBeFinalized() {
   for (Range r = Base::all(); !r.empty(); r.popFront()) {
     K k(r.front().key());
     MOZ_ASSERT(!gc::IsAboutToBeFinalized(&k));
-    JSObject* delegate = gc::detail::GetDelegate(k);
+    JSObject* delegate = getDelegate(k);
     if (delegate) {
       MOZ_ASSERT(!gc::IsAboutToBeFinalizedUnbarriered(&delegate),
                  "weakmap marking depends on a key tracing its delegate");
     }
     MOZ_ASSERT(!gc::IsAboutToBeFinalized(&r.front().value()));
     MOZ_ASSERT(k == r.front().key());
   }
 }
--- a/js/src/gc/WeakMap.cpp
+++ b/js/src/gc/WeakMap.cpp
@@ -18,27 +18,27 @@
 #include "vm/JSObject.h"
 
 #include "vm/JSObject-inl.h"
 
 using namespace js;
 using namespace js::gc;
 
 WeakMapBase::WeakMapBase(JSObject* memOf, Zone* zone)
-    : memberOf(memOf), zone_(zone), mapColor(CellColor::White) {
+    : memberOf(memOf), zone_(zone), marked(false), markColor(MarkColor::Black) {
   MOZ_ASSERT_IF(memberOf, memberOf->compartment()->zone() == zone);
 }
 
 WeakMapBase::~WeakMapBase() {
   MOZ_ASSERT(CurrentThreadIsGCSweeping() || CurrentThreadCanAccessZone(zone_));
 }
 
 void WeakMapBase::unmarkZone(JS::Zone* zone) {
   for (WeakMapBase* m : zone->gcWeakMapList()) {
-    m->mapColor = CellColor::White;
+    m->marked = false;
   }
 }
 
 void WeakMapBase::traceZone(JS::Zone* zone, JSTracer* tracer) {
   MOZ_ASSERT(tracer->weakMapAction() != DoNotTraceWeakMaps);
   for (WeakMapBase* m : zone->gcWeakMapList()) {
     m->trace(tracer);
     TraceNullableEdge(tracer, &m->memberOf, "memberOf");
@@ -47,29 +47,29 @@ void WeakMapBase::traceZone(JS::Zone* zo
 
 #if defined(JS_GC_ZEAL) || defined(DEBUG)
 bool WeakMapBase::checkMarkingForZone(JS::Zone* zone) {
   // This is called at the end of marking.
   MOZ_ASSERT(zone->isGCMarking());
 
   bool ok = true;
   for (WeakMapBase* m : zone->gcWeakMapList()) {
-    if (m->mapColor && !m->checkMarking()) {
+    if (m->marked && !m->checkMarking()) {
       ok = false;
     }
   }
 
   return ok;
 }
 #endif
 
 bool WeakMapBase::markZoneIteratively(JS::Zone* zone, GCMarker* marker) {
   bool markedAny = false;
   for (WeakMapBase* m : zone->gcWeakMapList()) {
-    if (m->mapColor && m->markEntries(marker)) {
+    if (m->marked && m->markEntries(marker)) {
       markedAny = true;
     }
   }
   return markedAny;
 }
 
 bool WeakMapBase::findSweepGroupEdgesForZone(JS::Zone* zone) {
   for (WeakMapBase* m : zone->gcWeakMapList()) {
@@ -78,60 +78,59 @@ bool WeakMapBase::findSweepGroupEdgesFor
     }
   }
   return true;
 }
 
 void WeakMapBase::sweepZone(JS::Zone* zone) {
   for (WeakMapBase* m = zone->gcWeakMapList().getFirst(); m;) {
     WeakMapBase* next = m->getNext();
-    if (m->mapColor) {
+    if (m->marked) {
       m->sweep();
     } else {
       m->clearAndCompact();
       m->removeFrom(zone->gcWeakMapList());
     }
     m = next;
   }
 
 #ifdef DEBUG
   for (WeakMapBase* m : zone->gcWeakMapList()) {
-    MOZ_ASSERT(m->isInList() && m->mapColor);
+    MOZ_ASSERT(m->isInList() && m->marked);
   }
 #endif
 }
 
 void WeakMapBase::traceAllMappings(WeakMapTracer* tracer) {
   JSRuntime* rt = tracer->runtime;
   for (ZonesIter zone(rt, SkipAtoms); !zone.done(); zone.next()) {
     for (WeakMapBase* m : zone->gcWeakMapList()) {
       // The WeakMapTracer callback is not allowed to GC.
       JS::AutoSuppressGCAnalysis nogc;
       m->traceMappings(tracer);
     }
   }
 }
 
 bool WeakMapBase::saveZoneMarkedWeakMaps(JS::Zone* zone,
-                                         WeakMapColors& markedWeakMaps) {
+                                         WeakMapSet& markedWeakMaps) {
   for (WeakMapBase* m : zone->gcWeakMapList()) {
-    if (m->mapColor && !markedWeakMaps.put(m, m->mapColor)) {
+    if (m->marked && !markedWeakMaps.put(m)) {
       return false;
     }
   }
   return true;
 }
 
-void WeakMapBase::restoreMarkedWeakMaps(WeakMapColors& markedWeakMaps) {
-  for (WeakMapColors::Range r = markedWeakMaps.all(); !r.empty();
-       r.popFront()) {
-    WeakMapBase* map = r.front().key();
+void WeakMapBase::restoreMarkedWeakMaps(WeakMapSet& markedWeakMaps) {
+  for (WeakMapSet::Range r = markedWeakMaps.all(); !r.empty(); r.popFront()) {
+    WeakMapBase* map = r.front();
     MOZ_ASSERT(map->zone()->isGCMarking());
-    MOZ_ASSERT(map->mapColor == CellColor::White);
-    map->mapColor = r.front().value();
+    MOZ_ASSERT(!map->marked);
+    map->marked = true;
   }
 }
 
 size_t ObjectValueWeakMap::sizeOfIncludingThis(
     mozilla::MallocSizeOf mallocSizeOf) {
   return mallocSizeOf(this) + shallowSizeOfExcludingThis(mallocSizeOf);
 }
 
@@ -142,17 +141,17 @@ bool ObjectValueWeakMap::findSweepGroupE
    * zone.
    */
   JS::AutoSuppressGCAnalysis nogc;
   for (Range r = all(); !r.empty(); r.popFront()) {
     JSObject* key = r.front().key();
     if (key->asTenured().isMarkedBlack()) {
       continue;
     }
-    JSObject* delegate = gc::detail::GetDelegate(key);
+    JSObject* delegate = getDelegate(key);
     if (!delegate) {
       continue;
     }
     Zone* delegateZone = delegate->zone();
     if (delegateZone == zone() || !delegateZone->isGCMarking()) {
       continue;
     }
     if (!delegateZone->addSweepGroupEdgeTo(key->zone())) {
--- a/js/src/gc/WeakMap.h
+++ b/js/src/gc/WeakMap.h
@@ -42,27 +42,25 @@ bool CheckWeakMapEntryMarking(const Weak
 //     key are live. An entry holds a strong reference to its value.
 //
 // You must call this table's 'trace' method when its owning object is reached
 // by the garbage collection tracer. Once a table is known to be live, the
 // implementation takes care of the special weak marking (ie, marking through
 // the implicit edges stored in the map) and of removing (sweeping) table
 // entries when collection is complete.
 
-using WeakMapColors = HashMap<WeakMapBase*, js::gc::CellColor,
-                              DefaultHasher<WeakMapBase*>, SystemAllocPolicy>;
+typedef HashSet<WeakMapBase*, DefaultHasher<WeakMapBase*>, SystemAllocPolicy>
+    WeakMapSet;
 
 // Common base class for all WeakMap specializations, used for calling
 // subclasses' GC-related methods.
 class WeakMapBase : public mozilla::LinkedListElement<WeakMapBase> {
   friend class js::GCMarker;
 
  public:
-  using CellColor = js::gc::CellColor;
-
   WeakMapBase(JSObject* memOf, JS::Zone* zone);
   virtual ~WeakMapBase();
 
   JS::Zone* zone() const { return zone_; }
 
   // Garbage collector entry points.
 
   // Unmark all weak maps in a zone.
@@ -85,37 +83,42 @@ class WeakMapBase : public mozilla::Link
   // entries of live weak maps whose keys are dead.
   static void sweepZone(JS::Zone* zone);
 
   // Trace all weak map bindings. Used by the cycle collector.
   static void traceAllMappings(WeakMapTracer* tracer);
 
   // Save information about which weak maps are marked for a zone.
   static bool saveZoneMarkedWeakMaps(JS::Zone* zone,
-                                     WeakMapColors& markedWeakMaps);
+                                     WeakMapSet& markedWeakMaps);
 
   // Restore information about which weak maps are marked for many zones.
-  static void restoreMarkedWeakMaps(WeakMapColors& markedWeakMaps);
+  static void restoreMarkedWeakMaps(WeakMapSet& markedWeakMaps);
 
 #if defined(JS_GC_ZEAL) || defined(DEBUG)
   static bool checkMarkingForZone(JS::Zone* zone);
 #endif
 
+  static JSObject* getDelegate(JSObject* key);
+  static JSObject* getDelegate(JSScript* script);
+  static JSObject* getDelegate(LazyScript* script);
+
  protected:
   // Instance member functions called by the above. Instantiations of WeakMap
   // override these with definitions appropriate for their Key and Value types.
   virtual void trace(JSTracer* tracer) = 0;
   virtual bool findSweepGroupEdges() = 0;
   virtual void sweep() = 0;
   virtual void traceMappings(WeakMapTracer* tracer) = 0;
   virtual void clearAndCompact() = 0;
 
   // Any weakmap key types that want to participate in the non-iterative
   // ephemeron marking must override this method.
-  virtual void markKey(GCMarker* marker, gc::Cell* markedCell, gc::Cell* l) = 0;
+  virtual void markEntry(GCMarker* marker, gc::Cell* markedCell,
+                         gc::Cell* l) = 0;
 
   virtual bool markEntries(GCMarker* marker) = 0;
 
 #ifdef JS_GC_ZEAL
   virtual bool checkMarking() const = 0;
   virtual bool allowKeysInOtherZones() const { return false; }
   friend bool gc::CheckWeakMapEntryMarking(const WeakMapBase*, gc::Cell*,
                                            gc::Cell*);
@@ -124,31 +127,20 @@ class WeakMapBase : public mozilla::Link
   // Object that this weak map is part of, if any.
   GCPtrObject memberOf;
 
   // Zone containing this weak map.
   JS::Zone* zone_;
 
   // Whether this object has been marked during garbage collection and which
   // color it was marked.
-  gc::CellColor mapColor;
+  bool marked;
+  gc::MarkColor markColor;
 };
 
-namespace detail {
-
-template <typename T>
-struct RemoveBarrier {};
-
-template <typename T>
-struct RemoveBarrier<js::HeapPtr<T>> {
-  using Type = T;
-};
-
-}  // namespace detail
-
 template <class Key, class Value>
 class WeakMap
     : private HashMap<Key, Value, MovableCellHasher<Key>, ZoneAllocPolicy>,
       public WeakMapBase {
  public:
   using Base = HashMap<Key, Value, MovableCellHasher<Key>, ZoneAllocPolicy>;
 
   using Lookup = typename Base::Lookup;
@@ -164,18 +156,16 @@ class WeakMap
   using Base::all;
   using Base::clear;
   using Base::has;
   using Base::shallowSizeOfExcludingThis;
 
   // Resolve ambiguity with LinkedListElement<>::remove.
   using Base::remove;
 
-  using UnbarrieredKey = typename detail::RemoveBarrier<Key>::Type;
-
   explicit WeakMap(JSContext* cx, JSObject* memOf = nullptr);
 
   // Add a read barrier to prevent an incorrectly gray value from escaping the
   // weak map. See the UnmarkGrayTracer::onChild comment in gc/Marking.cpp.
   Ptr lookup(const Lookup& l) const {
     Ptr p = Base::lookup(l);
     if (p) {
       exposeGCThingToActiveJS(p->value());
@@ -216,20 +206,18 @@ class WeakMap
 #ifdef DEBUG
   template <typename KeyInput, typename ValueInput>
   bool hasEntry(KeyInput&& key, ValueInput&& value) {
     Ptr p = Base::lookup(std::forward<KeyInput>(key));
     return p && p->value() == value;
   }
 #endif
 
-  void markKey(GCMarker* marker, gc::Cell* markedCell,
-               gc::Cell* origKey) override;
-
-  bool markEntry(GCMarker* marker, Key& key, Value& value);
+  void markEntry(GCMarker* marker, gc::Cell* markedCell,
+                 gc::Cell* origKey) override;
 
   void trace(JSTracer* trc) override;
 
  protected:
   // We have a key that, if it or its delegate is marked, may lead to a WeakMap
   // value getting marked. Insert it or its delegate (if any) into the
   // appropriate zone's gcWeakKeys or gcNurseryWeakKeys.
   static void addWeakEntry(GCMarker* marker, gc::Cell* key,
@@ -249,16 +237,20 @@ class WeakMap
  private:
   void exposeGCThingToActiveJS(const JS::Value& v) const {
     JS::ExposeValueToActiveJS(v);
   }
   void exposeGCThingToActiveJS(JSObject* obj) const {
     JS::ExposeObjectToActiveJS(obj);
   }
 
+  bool keyNeedsMark(GCMarker* marker, JSObject* key) const;
+  bool keyNeedsMark(GCMarker* marker, JSScript* script) const;
+  bool keyNeedsMark(GCMarker* marker, LazyScript* script) const;
+
   bool findSweepGroupEdges() override {
     // This is overridden by ObjectValueWeakMap and DebuggerWeakMap.
     return true;
   }
 
   void sweep() override;
 
   void clearAndCompact() override {
--- a/js/src/gc/Zone.cpp
+++ b/js/src/gc/Zone.cpp
@@ -223,17 +223,17 @@ void Zone::sweepWeakKeysAfterMinorGC() {
         AutoEnterOOMUnsafeRegion oomUnsafe;
         oomUnsafe.crash("Failed to tenure weak keys entry");
       }
     }
 
     // If the key has a delegate, then it will map to a WeakKeyEntryVector
     // containing the key that needs to be updated.
 
-    JSObject* delegate = gc::detail::GetDelegate(key->as<JSObject>());
+    JSObject* delegate = WeakMapBase::getDelegate(key->as<JSObject>());
     if (!delegate) {
       continue;
     }
     MOZ_ASSERT(delegate->isTenured());
 
     // If delegate was formerly nursery-allocated, we will sweep its
     // entries when we visit its gcNurseryWeakKeys (if we haven't already).
     // Note that we don't know the nursery address of the delegate, since
--- a/js/src/jit-test/tests/gc/weak-marking-03.js
+++ b/js/src/jit-test/tests/gc/weak-marking-03.js
@@ -407,17 +407,16 @@ function grayMapKey() {
   assertEq(getMarks().join("/"), "gray/unmarked/unmarked",
            "marked the map gray");
 
   gcslice(100000);
   assertEq(getMarks().join("/"), "gray/black/unmarked",
            "key is now marked black");
 
   finishgc(); // Finish the GC
-
   assertEq(getMarks().join("/"), "gray/black/gray",
            "at end: black/gray => gray");
 
   clearMarkQueue();
   clearMarkObservers();
 }
 
 if (this.enqueueMark)
@@ -479,88 +478,8 @@ function grayKeyMap() {
            "further marked the map black, so value should also be blackened");
 
   clearMarkQueue();
   clearMarkObservers();
 }
 
 if (this.enqueueMark)
   runtest(grayKeyMap);
-
-// Cause a key to be marked black *during gray marking*, by first marking a
-// delegate black, then marking the map and key gray. When the key is scanned,
-// it should be seen to be a CCW of a black delegate and so should itself be
-// marked black.
-//
-// Note that this is currently buggy -- the key will be marked because the
-// delegate is marked, but the color won't be taken into account. So the key
-// will be marked gray (or rather, it will see that it's already gray.) The bad
-// behavior this would cause is if:
-//
-//  1. You wrap an object in a CCW and use it as a weakmap key to some
-//     information.
-//  2. You keep a strong reference to the object (in its compartment).
-//  3. The only references to the CCW are gray, and are in fact part of a cycle.
-//  4. The CC runs and discards the CCW.
-//  5. You look up the object in the weakmap again. This creates a new wrapper
-//     to use as a key. It is not in the weakmap, so the information you stored
-//     before is not found. (It may have even been collected, if you had no
-//     other references to it.)
-//
-function blackDuringGray() {
-  const g = newGlobal({newCompartment: true});
-  const vals = {};
-  vals.map = new WeakMap();
-  vals.key = g.eval("Object.create(null)");
-  vals.val = Object.create(null);
-  vals.map.set(vals.key, vals.val);
-
-  g.delegate = vals.key;
-  addMarkObservers([vals.map, vals.key]);
-  g.addMarkObservers([vals.key]);
-  addMarkObservers([vals.val]);
-  // Mark observers: map, key, delegate, value
-
-  gc();
-
-  g.enqueueMark(vals.key); // Mark the delegate black
-  enqueueMark("yield"); // checkpoint 1
-
-  // Mark the map gray. This will scan through all entries, find our key, and
-  // mark it black because its delegate is black.
-  enqueueMark("set-color-gray");
-  enqueueMark(vals.map); // Mark the map gray
-
-  vals.map = null;
-  vals.val = null;
-  vals.key = null;
-  g.delegate = null;
-
-  const showmarks = () => {
-    print("[map,key,delegate,value] marked " + JSON.stringify(getMarks()));
-  };
-
-  print("Starting incremental GC");
-  startgc(100000);
-  // Checkpoint 1, after marking delegate black
-  showmarks();
-  var marks = getMarks();
-  assertEq(marks[0], "unmarked", "map is not marked yet");
-  assertEq(marks[1], "unmarked", "key is not marked yet");
-  assertEq(marks[2], "black", "delegate is black");
-  assertEq(marks[3], "unmarked", "values is not marked yet");
-
-  finishgc();
-  showmarks();
-  marks = getMarks();
-  assertEq(marks[0], "gray", "map is gray");
-  assertEq(marks[1], "black", "key inherits black even though in gray marking");
-  assertEq(marks[2], "black", "delegate is still black");
-  assertEq(marks[3], "gray", "gray map + black key => gray value");
-
-  clearMarkQueue();
-  clearMarkObservers();
-  grayRoot().length = 0;
-  g.eval("grayRoot().length = 0");
-}
-
-if (this.enqueueMark)
-  runtest(blackDuringGray);
--- a/js/src/jsapi-tests/testGCGrayMarking.cpp
+++ b/js/src/jsapi-tests/testGCGrayMarking.cpp
@@ -1,17 +1,15 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*-
  * vim: set ts=8 sts=2 et sw=2 tw=80:
  */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#include <algorithm>
-
 #include "gc/Heap.h"
 #include "gc/Verifier.h"
 #include "gc/WeakMap.h"
 #include "gc/Zone.h"
 #include "js/Proxy.h"
 #include "jsapi-tests/tests.h"
 
 using namespace js;
@@ -152,25 +150,26 @@ bool TestMarking() {
   blackRoot1 = nullptr;
   blackRoot2 = nullptr;
   grayRoots.grayRoot1 = nullptr;
   grayRoots.grayRoot2 = nullptr;
 
   return true;
 }
 
-static constexpr CellColor DontMark = CellColor::White;
+static const CellColor DontMark = CellColor::White;
 
 enum MarkKeyOrDelegate : bool { MarkKey = true, MarkDelegate = false };
 
 bool TestJSWeakMaps() {
   for (auto keyOrDelegateColor : MarkedCellColors) {
     for (auto mapColor : MarkedCellColors) {
       for (auto markKeyOrDelegate : {MarkKey, MarkDelegate}) {
-        CellColor expected = std::min(keyOrDelegateColor, mapColor);
+        CellColor expected =
+            ExpectedWeakMapValueColor(keyOrDelegateColor, mapColor);
         CHECK(TestJSWeakMap(markKeyOrDelegate, keyOrDelegateColor, mapColor,
                             expected));
 #ifdef JS_GC_ZEAL
         CHECK(TestJSWeakMapWithGrayUnmarking(
             markKeyOrDelegate, keyOrDelegateColor, mapColor, expected));
 #endif
       }
     }
@@ -182,20 +181,20 @@ bool TestJSWeakMaps() {
 bool TestInternalWeakMaps() {
   for (auto keyMarkColor : AllCellColors) {
     for (auto delegateMarkColor : AllCellColors) {
       if (keyMarkColor == CellColor::White &&
           delegateMarkColor == CellColor::White) {
         continue;
       }
 
-      // The map is black. The delegate marks its key via wrapper preservation.
-      // The key maps its delegate and the value. Thus, all three end up the
-      // maximum of the key and delegate colors.
-      CellColor expected = std::max(keyMarkColor, delegateMarkColor);
+      CellColor keyOrDelegateColor =
+          ExpectedKeyAndDelegateColor(keyMarkColor, delegateMarkColor);
+      CellColor expected =
+          ExpectedWeakMapValueColor(keyOrDelegateColor, CellColor::Black);
       CHECK(TestInternalWeakMap(keyMarkColor, delegateMarkColor, expected));
 
 #ifdef JS_GC_ZEAL
       CHECK(TestInternalWeakMapWithGrayUnmarking(keyMarkColor,
                                                  delegateMarkColor, expected));
 #endif
     }
   }
@@ -236,19 +235,19 @@ bool TestJSWeakMap(MarkKeyOrDelegate mar
       mozilla::Swap(blackRoot1.get(), blackRoot2.get());
       mozilla::Swap(grayRoots.grayRoot1, grayRoots.grayRoot2);
     }
 
     JS_GC(cx);
 
     ClearGrayRoots();
 
-    CHECK(weakMap->color() == weakMapMarkColor);
-    CHECK(keyOrDelegate->color() == keyOrDelegateMarkColor);
-    CHECK(value->color() == expectedValueColor);
+    CHECK(GetCellColor(weakMap) == weakMapMarkColor);
+    CHECK(GetCellColor(keyOrDelegate) == keyOrDelegateMarkColor);
+    CHECK(GetCellColor(value) == expectedValueColor);
   }
 
   return true;
 }
 
 #ifdef JS_GC_ZEAL
 
 bool TestJSWeakMapWithGrayUnmarking(MarkKeyOrDelegate markKey,
@@ -292,19 +291,19 @@ bool TestJSWeakMapWithGrayUnmarking(Mark
       MaybeExposeObject(keyOrDelegate, keyOrDelegateMarkColor);
       MaybeExposeObject(weakMap, weakMapMarkColor);
     }
 
     JS::FinishIncrementalGC(cx, JS::GCReason::API);
 
     ClearGrayRoots();
 
-    CHECK(weakMap->color() == weakMapMarkColor);
-    CHECK(keyOrDelegate->color() == keyOrDelegateMarkColor);
-    CHECK(value->color() == expectedValueColor);
+    CHECK(GetCellColor(weakMap) == weakMapMarkColor);
+    CHECK(GetCellColor(keyOrDelegate) == keyOrDelegateMarkColor);
+    CHECK(GetCellColor(value) == expectedValueColor);
   }
 
   JS_UnsetGCZeal(cx, uint8_t(ZealMode::YieldWhileGrayMarking));
 
   return true;
 }
 
 static void MaybeExposeObject(JSObject* object, CellColor color) {
@@ -365,19 +364,19 @@ bool TestInternalWeakMap(CellColor keyMa
       mozilla::Swap(blackRoot1.get(), blackRoot2.get());
       mozilla::Swap(grayRoots.grayRoot1, grayRoots.grayRoot2);
     }
 
     JS_GC(cx);
 
     ClearGrayRoots();
 
-    CHECK(key->color() == expectedColor);
-    CHECK(delegate->color() == expectedColor);
-    CHECK(value->color() == expectedColor);
+    CHECK(GetCellColor(key) == expectedColor);
+    CHECK(GetCellColor(delegate) == expectedColor);
+    CHECK(GetCellColor(value) == expectedColor);
   }
 
   return true;
 }
 
 #ifdef JS_GC_ZEAL
 
 bool TestInternalWeakMapWithGrayUnmarking(CellColor keyMarkColor,
@@ -417,19 +416,19 @@ bool TestInternalWeakMapWithGrayUnmarkin
       MaybeExposeObject(key, keyMarkColor);
       MaybeExposeObject(delegate, delegateMarkColor);
     }
 
     JS::FinishIncrementalGC(cx, JS::GCReason::API);
 
     ClearGrayRoots();
 
-    CHECK(key->color() == expectedColor);
-    CHECK(delegate->color() == expectedColor);
-    CHECK(value->color() == expectedColor);
+    CHECK(GetCellColor(key) == expectedColor);
+    CHECK(GetCellColor(delegate) == expectedColor);
+    CHECK(GetCellColor(value) == expectedColor);
   }
 
   JS_UnsetGCZeal(cx, uint8_t(ZealMode::YieldWhileGrayMarking));
 
   return true;
 }
 
 #endif  // JS_GC_ZEAL