Bug 1571021 - Separate out CCW iterators for objects and strings and remove API to iterate all r=jandem?

This removes Compartment::WrapperEnum and renames NonStringWrapperEnum so there's now just ObjectWrapperEnum and StringWrapperEnum.

In js::VisitGrayWrapperTargets, strings can never be marked gray and so needn't be considered.  In JS::TraceIncomingCCWs, strings have no compartment and so can be ignored here too.

Differential Revision: https://phabricator.services.mozilla.com/D41182

--HG--
extra : moz-landing-system : lando

js/src/gc/GC.cpp

@@ -4613,7 +4613,7 @@ void GCRuntime::markCompartments() {
 
   while (!workList.empty()) {
     Compartment* comp = workList.popCopy();
-    for (Compartment::NonStringWrapperEnum e(comp); !e.empty(); e.popFront()) {
+    for (Compartment::ObjectWrapperEnum e(comp); !e.empty(); e.popFront()) {
       Compartment* dest = e.front().mutableKey().compartment();
       if (dest && !dest->gcState.maybeAlive) {
         dest->gcState.maybeAlive = true;
@@ -5040,9 +5040,8 @@ static void DropStringWrappers(JSRuntime* rt) {
 
 bool Compartment::findSweepGroupEdges() {
   Zone* source = zone();
-  for (WrapperMap::Enum e(crossCompartmentWrappers); !e.empty(); e.popFront()) {
+  for (ObjectWrapperEnum e(this); !e.empty(); e.popFront()) {
     CrossCompartmentKey& key = e.front().mutableKey();
-    MOZ_ASSERT(!key.is<JSString*>());
 
     Zone* target = key.zone();
     if (!target->isGCMarking()) {
@@ -5053,8 +5052,7 @@ bool Compartment::findSweepGroupEdges() {
     // is not still being marked when we start sweeping the wrapped zone. As an
     // optimization, if the wrapped object is already marked black there is no
     // danger of later marking and we can skip this.
-    if (key.is<JSObject*>() &&
-        key.as<JSObject*>()->asTenured().isMarkedBlack()) {
+    if (key.as<JSObject*>()->asTenured().isMarkedBlack()) {
       continue;
     }
 
@@ -5267,7 +5265,7 @@ static void AssertNoWrappersInGrayList(JSRuntime* rt) {
 #ifdef DEBUG
   for (CompartmentsIter c(rt); !c.done(); c.next()) {
     MOZ_ASSERT(!c->gcIncomingGrayPointers);
-    for (Compartment::NonStringWrapperEnum e(c); !e.empty(); e.popFront()) {
+    for (Compartment::ObjectWrapperEnum e(c); !e.empty(); e.popFront()) {
       AssertNotOnGrayList(&e.front().value().unbarrieredGet().toObject());
     }
   }

js/src/gc/Tracer.cpp

@@ -107,16 +107,14 @@ JS_PUBLIC_API void JS::TraceIncomingCCWs(
     if (compartments.has(comp)) {
       continue;
     }
-
-    for (Compartment::WrapperEnum e(comp); !e.empty(); e.popFront()) {
-      mozilla::DebugOnly<const CrossCompartmentKey> prior = e.front().key();
-      e.front().mutableKey().applyToWrapped([trc, &compartments](auto tp) {
-        Compartment* comp = (*tp)->maybeCompartment();
-        if (comp && compartments.has(comp)) {
-          TraceManuallyBarrieredEdge(trc, tp, "cross-compartment wrapper");
-        }
-      });
-      MOZ_ASSERT(e.front().key() == prior);
+    for (Compartment::ObjectWrapperEnum e(comp); !e.empty(); e.popFront()) {
+      JSObject* obj = e.front().key().as<JSObject*>();
+      Compartment* comp = obj->compartment();
+      if (compartments.has(comp)) {
+        mozilla::DebugOnly<JSObject*> prior = obj;
+        TraceManuallyBarrieredEdge(trc, &obj, "cross-compartment wrapper");
+        MOZ_ASSERT(obj == prior);
+      }
     }
   }
 }

js/src/jsapi-tests/testGCMarking.cpp

@@ -138,9 +138,9 @@ BEGIN_TEST(testTracingIncomingCCWs) {
 }
 END_TEST(testTracingIncomingCCWs)
 
-static size_t countWrappers(JS::Compartment* comp) {
+static size_t countObjectWrappers(JS::Compartment* comp) {
   size_t count = 0;
-  for (JS::Compartment::WrapperEnum e(comp); !e.empty(); e.popFront()) {
+  for (JS::Compartment::ObjectWrapperEnum e(comp); !e.empty(); e.popFront()) {
     ++count;
   }
   return count;
@@ -167,9 +167,9 @@ BEGIN_TEST(testDeadNurseryCCW) {
   wrappee = wrapper = nullptr;
 
   // Now a GC should clear the CCW.
-  CHECK(countWrappers(global1->compartment()) == 1);
+  CHECK(countObjectWrappers(global1->compartment()) == 1);
   cx->runtime()->gc.evictNursery();
-  CHECK(countWrappers(global1->compartment()) == 0);
+  CHECK(countObjectWrappers(global1->compartment()) == 0);
 
   // Check for corruption of the CCW table by doing a full GC to force sweeping.
   JS_GC(cx);
@@ -196,9 +196,9 @@ BEGIN_TEST(testLiveNurseryCCW) {
   CHECK(js::gc::IsInsideNursery(wrapper));
 
   // Now a GC should not kill the CCW.
-  CHECK(countWrappers(global1->compartment()) == 1);
+  CHECK(countObjectWrappers(global1->compartment()) == 1);
   cx->runtime()->gc.evictNursery();
-  CHECK(countWrappers(global1->compartment()) == 1);
+  CHECK(countObjectWrappers(global1->compartment()) == 1);
 
   CHECK(!js::gc::IsInsideNursery(wrappee));
   CHECK(!js::gc::IsInsideNursery(wrapper));
@@ -234,9 +234,9 @@ BEGIN_TEST(testLiveNurseryWrapperCCW) {
   wrappee = nullptr;
 
   // Now a GC should not kill the CCW.
-  CHECK(countWrappers(global1->compartment()) == 1);
+  CHECK(countObjectWrappers(global1->compartment()) == 1);
   cx->runtime()->gc.evictNursery();
-  CHECK(countWrappers(global1->compartment()) == 1);
+  CHECK(countObjectWrappers(global1->compartment()) == 1);
 
   CHECK(!js::gc::IsInsideNursery(wrapper));
 
@@ -269,9 +269,9 @@ BEGIN_TEST(testLiveNurseryWrappeeCCW) {
   wrapper = nullptr;
 
   // Now a GC should not kill the CCW.
-  CHECK(countWrappers(global1->compartment()) == 1);
+  CHECK(countObjectWrappers(global1->compartment()) == 1);
   cx->runtime()->gc.evictNursery();
-  CHECK(countWrappers(global1->compartment()) == 0);
+  CHECK(countObjectWrappers(global1->compartment()) == 0);
 
   CHECK(!js::gc::IsInsideNursery(wrappee));
 

js/src/jsfriendapi.cpp

@@ -536,7 +536,7 @@ JS_FRIEND_API void js::VisitGrayWrapperTargets(Zone* zone,
                                                GCThingCallback callback,
                                                void* closure) {
   for (CompartmentsInZoneIter comp(zone); !comp.done(); comp.next()) {
-    for (Compartment::WrapperEnum e(comp); !e.empty(); e.popFront()) {
+    for (Compartment::ObjectWrapperEnum e(comp); !e.empty(); e.popFront()) {
       e.front().mutableKey().applyToWrapped([callback, closure](auto tp) {
         if ((*tp)->isMarkedGray()) {
           callback(closure, JS::GCCellPtr(*tp));

js/src/proxy/CrossCompartmentWrapper.cpp

@@ -439,8 +439,8 @@ JS_FRIEND_API bool js::NukeCrossCompartmentWrappers(
     // |nukeAll| is true. The string wrappers that we're not interested in
     // won't be iterated, we can exclude them easily because they have
     // compartment nullptr. Use Maybe to avoid copying from conditionally
-    // initializing NonStringWrapperEnum.
-    mozilla::Maybe<Compartment::NonStringWrapperEnum> e;
+    // initializing ObjectWrapperEnum.
+    mozilla::Maybe<Compartment::ObjectWrapperEnum> e;
     if (MOZ_LIKELY(!nukeAll)) {
       e.emplace(c, target->compartment());
     } else {
@@ -642,7 +642,7 @@ JS_FRIEND_API bool js::RecomputeWrappers(
     }
 
     // Iterate over the wrappers, filtering appropriately.
-    for (Compartment::NonStringWrapperEnum e(c, targetFilter); !e.empty();
+    for (Compartment::ObjectWrapperEnum e(c, targetFilter); !e.empty();
          e.popFront()) {
       // Filter out non-objects.
       CrossCompartmentKey& k = e.front().mutableKey();

js/src/vm/Compartment.cpp

@@ -66,7 +66,7 @@ void Compartment::checkWrapperMapAfterMovingGC() {
   for (StringWrapperEnum e(this); !e.empty(); e.popFront()) {
     CheckWrapperMapEntry(crossCompartmentWrappers, e);
   }
-  for (NonStringWrapperEnum e(this); !e.empty(); e.popFront()) {
+  for (ObjectWrapperEnum e(this); !e.empty(); e.popFront()) {
     CheckWrapperMapEntry(crossCompartmentWrappers, e);
   }
 }
@@ -428,7 +428,7 @@ void Compartment::traceOutgoingCrossCompartmentWrappers(JSTracer* trc) {
   MOZ_ASSERT(!zone()->isCollectingFromAnyThread() ||
              trc->runtime()->gc.isHeapCompacting());
 
-  for (NonStringWrapperEnum e(this); !e.empty(); e.popFront()) {
+  for (ObjectWrapperEnum e(this); !e.empty(); e.popFront()) {
     if (e.front().key().is<JSObject*>()) {
       Value v = e.front().value().unbarrieredGet();
       ProxyObject* wrapper = &v.toObject().as<ProxyObject>();

js/src/vm/Compartment.h

@@ -178,13 +178,13 @@ class WrapperMap {
     SkipStrings skipStrings;
 
    public:
-    explicit Enum(WrapperMap& m, SkipStrings s = WithStrings)
+    explicit Enum(WrapperMap& m, SkipStrings s)
         : filter(nullptr), skipStrings(s) {
       outer.emplace(m.map);
       goToNext();
     }
 
-    Enum(WrapperMap& m, const CompartmentFilter& f, SkipStrings s = WithStrings)
+    Enum(WrapperMap& m, const CompartmentFilter& f, SkipStrings s)
         : filter(&f), skipStrings(s) {
       outer.emplace(m.map);
       goToNext();
@@ -473,19 +473,14 @@ class JS::Compartment {
     return crossCompartmentWrappers.hasNurseryAllocatedWrapperEntries(f);
   }
 
-  struct WrapperEnum : public js::WrapperMap::Enum {
-    explicit WrapperEnum(JS::Compartment* c)
-        : js::WrapperMap::Enum(c->crossCompartmentWrappers) {}
-  };
-
-  struct NonStringWrapperEnum : public js::WrapperMap::Enum {
-    explicit NonStringWrapperEnum(JS::Compartment* c)
+  struct ObjectWrapperEnum : public js::WrapperMap::Enum {
+    explicit ObjectWrapperEnum(JS::Compartment* c)
         : js::WrapperMap::Enum(c->crossCompartmentWrappers, WithoutStrings) {}
-    explicit NonStringWrapperEnum(JS::Compartment* c,
-                                  const js::CompartmentFilter& f)
+    explicit ObjectWrapperEnum(JS::Compartment* c,
+                               const js::CompartmentFilter& f)
         : js::WrapperMap::Enum(c->crossCompartmentWrappers, f, WithoutStrings) {
     }
-    explicit NonStringWrapperEnum(JS::Compartment* c, JS::Compartment* target)
+    explicit ObjectWrapperEnum(JS::Compartment* c, JS::Compartment* target)
         : js::WrapperMap::Enum(c->crossCompartmentWrappers, target) {
       MOZ_ASSERT(target);
     }