diff --git a/js/xpconnect/src/nsXPConnect.cpp b/js/xpconnect/src/nsXPConnect.cpp index 6d113b34fc4..735457cfcee 100644 --- a/js/xpconnect/src/nsXPConnect.cpp +++ b/js/xpconnect/src/nsXPConnect.cpp @@ -388,6 +388,7 @@ struct NoteWeakMapChildrenTracer : public JSTracer { } nsCycleCollectionTraversalCallback &mCb; + bool mTracedAny; JSObject *mMap; void *mKey; void *mKeyDelegate; @@ -406,6 +407,7 @@ TraceWeakMappingChild(JSTracer *trc, void **thingp, JSGCTraceKind kind) return; if (AddToCCKind(kind)) { tracer->mCb.NoteWeakMapping(tracer->mMap, tracer->mKey, tracer->mKeyDelegate, thing); + tracer->mTracedAny = true; } else { JS_TraceChildren(trc, thing, kind); } @@ -430,10 +432,12 @@ TraceWeakMapping(js::WeakMapTracer *trc, JSObject *m, { MOZ_ASSERT(trc->callback == TraceWeakMapping); NoteWeakMapsTracer *tracer = static_cast(trc); - if (vkind == JSTRACE_STRING) - return; - if (!xpc_IsGrayGCThing(v) && !tracer->mCb.WantAllTraces()) - return; + + // If nothing that could be held alive by this entry is marked gray, return. + if ((!k || !xpc_IsGrayGCThing(k)) && MOZ_LIKELY(!tracer->mCb.WantAllTraces())) { + if (!v || !xpc_IsGrayGCThing(v) || vkind == JSTRACE_STRING) + return; + } // The cycle collector can only properly reason about weak maps if it can // reason about the liveness of their keys, which in turn requires that @@ -448,17 +452,25 @@ TraceWeakMapping(js::WeakMapTracer *trc, JSObject *m, if (!AddToCCKind(kkind)) k = nullptr; - JSObject *kdelegate = NULL; - if (kkind == JSTRACE_OBJECT) + JSObject *kdelegate = nullptr; + if (k && kkind == JSTRACE_OBJECT) kdelegate = js::GetWeakmapKeyDelegate((JSObject *)k); if (AddToCCKind(vkind)) { tracer->mCb.NoteWeakMapping(m, k, kdelegate, v); } else { + tracer->mChildTracer.mTracedAny = false; tracer->mChildTracer.mMap = m; tracer->mChildTracer.mKey = k; tracer->mChildTracer.mKeyDelegate = kdelegate; - JS_TraceChildren(&tracer->mChildTracer, v, vkind); + + if (v && vkind != JSTRACE_STRING) + JS_TraceChildren(&tracer->mChildTracer, v, vkind); + + // The delegate could hold alive the key, so report something to the CC + // if we haven't already. + if (!tracer->mChildTracer.mTracedAny && k && xpc_IsGrayGCThing(k) && kdelegate) + tracer->mCb.NoteWeakMapping(m, k, kdelegate, nullptr); } } diff --git a/xpcom/base/nsCycleCollector.cpp b/xpcom/base/nsCycleCollector.cpp index 38696f22e25..3b9510edf87 100644 --- a/xpcom/base/nsCycleCollector.cpp +++ b/xpcom/base/nsCycleCollector.cpp @@ -2017,16 +2017,13 @@ GCGraphBuilder::AddWeakMapNode(void *node) NS_IMETHODIMP_(void) GCGraphBuilder::NoteWeakMapping(void *map, void *key, void *kdelegate, void *val) { - PtrInfo *valNode = AddWeakMapNode(val); - - if (!valNode) - return; - + // Don't try to optimize away the entry here, as we've already attempted to + // do that in TraceWeakMapping in nsXPConnect. WeakMapping *mapping = mWeakMaps.AppendElement(); mapping->mMap = map ? AddWeakMapNode(map) : nullptr; mapping->mKey = key ? AddWeakMapNode(key) : nullptr; mapping->mKeyDelegate = kdelegate ? AddWeakMapNode(kdelegate) : mapping->mKey; - mapping->mVal = valNode; + mapping->mVal = val ? AddWeakMapNode(val) : nullptr; } static bool @@ -2251,11 +2248,11 @@ nsCycleCollector::ScanWeakMaps() for (uint32_t i = 0; i < mGraph.mWeakMaps.Length(); i++) { WeakMapping *wm = &mGraph.mWeakMaps[i]; - // If mMap or mKey are null, the original object was marked black. + // If any of these are null, the original object was marked black. uint32_t mColor = wm->mMap ? wm->mMap->mColor : black; uint32_t kColor = wm->mKey ? wm->mKey->mColor : black; uint32_t kdColor = wm->mKeyDelegate ? wm->mKeyDelegate->mColor : black; - PtrInfo *v = wm->mVal; + uint32_t vColor = wm->mVal ? wm->mVal->mColor : black; // All non-null weak mapping maps, keys and values are // roots (in the sense of WalkFromRoots) in the cycle @@ -2264,15 +2261,15 @@ nsCycleCollector::ScanWeakMaps() MOZ_ASSERT(mColor != grey, "Uncolored weak map"); MOZ_ASSERT(kColor != grey, "Uncolored weak map key"); MOZ_ASSERT(kdColor != grey, "Uncolored weak map key delegate"); - MOZ_ASSERT(v->mColor != grey, "Uncolored weak map value"); + MOZ_ASSERT(vColor != grey, "Uncolored weak map value"); if (mColor == black && kColor != black && kdColor == black) { GraphWalker(ScanBlackVisitor(mWhiteNodeCount)).Walk(wm->mKey); anyChanged = true; } - if (mColor == black && kColor == black && v->mColor != black) { - GraphWalker(ScanBlackVisitor(mWhiteNodeCount)).Walk(v); + if (mColor == black && kColor == black && vColor != black) { + GraphWalker(ScanBlackVisitor(mWhiteNodeCount)).Walk(wm->mVal); anyChanged = true; } }