1138395 - nsDocument::mExpandoAndGeneration.expando shows up in the cc graphs

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

Attachment: wip

In the cases I see it in cc graph expando points to an object with getter/setter and those are Proxies pointing to Location wrapper.

Apparently bug 928336 changes some of the expando handling, so it is not clear to me
what is the lifetime management for mExpandoAndGeneration.expando.
Right now it is traced only if we preserve the wrapper for the document.
I wonder if we could just make it behave more like a normal Heap<JS::Value> -
HoldJSObjects and set the value, and then trace always.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

bz, could you explain the lifetime management of nsDocument::mExpandoAndGeneration.expando?
Is it safe to assume that if expando is an object, document -> expando is a strong edge?

Comment 2

[Boris Zbarsky [:bzbarsky]]

> Apparently bug 928336 changes some of the expando handling, so it is not clear to me
> what is the lifetime management for mExpandoAndGeneration.expando.

After bug 928336, its lifetime will be the same as the nsIDocument (and as the nsIDocument's JS wrapper).

> Right now it is traced only if we preserve the wrapper for the document.

After bug 928336 we'll always preserve the wrapper for the document, afaict.

> Is it safe to assume that if expando is an object, document -> expando is a strong
> edge?

I think yes, since the function that creates the expando object preserves the wrapper for the document, which will ensure that expando is traced.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8571331 [details] [diff] [review]
wip

Thanks.

Comment 4

[Andrew McCreight [:mccr8]]

Comment on attachment 8571331 [details] [diff] [review]
wip

Review of attachment 8571331 [details] [diff] [review]:
-----------------------------------------------------------------

Does this need to wait for bug 928336 to land?

I guess doing the opposite direction, eg marking the document live when the expando object is black, is not going to be super useful. I'd expect the wrapper would be black in that case anyways.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> Does this need to wait for bug 928336 to land?

I don't think so.  Bug 928336 changes nothing about the lifetimes here; those were already changed in bug 1133760.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bd849bbe77da

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bd849bbe77da