Right now we can end up (and typically do!) in a situation where the reflector (the proxy) is black, but the expando object is gray, because it's only kept alive by the CC. This makes it easy to end up working with a gray object when we don't really want to be; see the try failures for the patch in bug 1283634. Seems like it should be pretty simple to just trace the expando from the proxy instead.

Comment on attachment 8773575 [details] [diff] [review] part 1. Start tracing the expando object, if any, inside the ExpandoAndGeneration of a shadowing DOM proxy from that proxy's trace hook Review of attachment 8773575 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/bindings/DOMJSProxyHandler.h @@ +32,5 @@ > * > * If it is, the proxy is initialized with a PrivateValue, which contains a > * pointer to a js::ExpandoAndGeneration object; this contains a pointer to > + * the actual expando object as well as the "generation" of the object. The > + * proxy handler will handle tracing the expando object stored in the Nit: "handle tracing" -> "trace"?

Comment on attachment 8773576 [details] [diff] [review] part 2. Stop messing with the ExpandoAndGeneration's expando in CC code, since it's now traced by the reflector proxy > > tmp->mIdentifierMap.Clear(); >- tmp->mExpandoAndGeneration.Unlink(); >+ >+ // Rev the generation on our mExpandoAndGeneration, since we just cleared the >+ // identifier map. >+ ++tmp->mExpandoAndGeneration.generation; As discussed, rename Unlink() to OwnerUnlinked() and call it rather than modifying the generation manually. > NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(HTMLFormElement, > nsGenericHTMLElement) > tmp->Clear(); >- tmp->mExpandoAndGeneration.Unlink(); >+ ++tmp->mExpandoAndGeneration.generation; ditto >+++ b/dom/html/nsDOMStringMap.cpp >@@ -27,25 +27,20 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ns > NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER > // Check that mElement exists in case the unlink code is run more than once. > if (tmp->mElement) { > // Call back to element to null out weak reference to this object. > tmp->mElement->ClearDataset(); > tmp->mElement->RemoveMutationObserver(tmp); > tmp->mElement = nullptr; > } >- tmp->mExpandoAndGeneration.Unlink(); >+ ++tmp->mExpandoAndGeneration.generation; ditto

Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/541edb687906 part 1. Start tracing the expando object, if any, inside the ExpandoAndGeneration of a shadowing DOM proxy from that proxy's trace hook. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/ab1504ebfbde part 2. Stop messing with the ExpandoAndGeneration's expando in CC code, since it's now traced by the reflector proxy. r=smaug

Comment on attachment 8773575 [details] [diff] [review] part 1. Start tracing the expando object, if any, inside the ExpandoAndGeneration of a shadowing DOM proxy from that proxy's trace hook ># HG changeset patch ># User Boris Zbarsky <bzbarsky@mit.edu> ># Date 1469154494 14400 ># Thu Jul 21 22:28:14 2016 -0400 ># Node ID 1ce2d3d8b1831d84d05d4bbbac39c7a7f003f7a6 ># Parent 74d28c3922b2519cccb1f41bc0723c643ba63ba0 >Bug 1288581 part 1. Start tracing the expando object, if any, inside the ExpandoAndGeneration of a shadowing DOM proxy from that proxy's trace hook. r=peterv. > >diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py > +// static >+void >+ShadowingDOMProxyHandler::trace(JSTracer* trc, JSObject* proxy) const This isn't static method.

Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0b88f94ed312 followup. Remove a bogus comment. DONTBUILD.

> This isn't static method. Fixed in the comment 7 commit.

https://hg.mozilla.org/mozilla-central/rev/541edb687906 https://hg.mozilla.org/mozilla-central/rev/ab1504ebfbde https://hg.mozilla.org/mozilla-central/rev/0b88f94ed312