Currently we have assertions for the case there actually is a preserved wrapper, but that doesn't catch errors in tests, since expando properties aren't that common.

Could you put some magic debug-only think into the wrappercache tracing macro boilerplate, so that we could tell if you call a wrappercache-y trace hook?

I'm planning to add some debug stuff the bindings code so that it gets triggered whenever a wrapper is created (doesn't need to be preserved). There are couple of different things, I think, we should have. Ensure sane QI implementation for nsISupports objects and ensure wrapper tracing for all CCables

Marking sec-want based on bug 1183450

Let's just make this a security bug for now.

Comment on attachment 8633597 [details] [diff] [review] wip, for nsISupports Review of attachment 8633597 [details] [diff] [review]: ----------------------------------------------------------------- Thank you so much for looking into this and fixing it! ::: dom/bindings/BindingUtils.h @@ +897,5 @@ > + nsWrapperCache* wrapperCacheFromQI = nullptr; > + aObject->QueryInterface(NS_GET_IID(nsWrapperCache), > + reinterpret_cast<void**>(&wrapperCacheFromQI)); > + > + MOZ_ASSERT(wrapperCacheFromQI); Please add a description to this assert so a random DOM developer who hits this assert can figure out how to fix it. @@ +908,5 @@ > + > + nsISupports* ccISupports; > + aObject->QueryInterface(NS_GET_IID(nsCycleCollectionISupports), > + reinterpret_cast<void**>(&ccISupports)); > + MOZ_ASSERT(ccISupports); Likewise. @@ +911,5 @@ > + reinterpret_cast<void**>(&ccISupports)); > + MOZ_ASSERT(ccISupports); > + > + nsXPCOMCycleCollectionParticipant* participant; > + CallQueryInterface(ccISupports, &participant); Do you need to maybe assert that participant is non-null here? If so, also add a description that will help a DOM developer figure out how to fix it. :)

I guess I should add an assert for participant too.

I think I could push this now to m-i. Waiting for try. https://treeherder.mozilla.org/#/jobs?repo=try&revision=483b5b7b238f

Backed out for introducing new hazards. https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-linux64-br-haz/20150728162040/hazards.txt.gz https://hg.mozilla.org/integration/mozilla-inbound/rev/39fba4322e8a

lovely.

I'll use AutoSuppressGCAnalysis here, since we're dealing strictly C++ objects only and QIing to nsWrapperCache, nsCycleCollectionISupports, nsXPCOMCycleCollectionParticipant which are all very much native. So QIing to those can't GC.

suppress the warning.