1123016 - Mark DOMEventTargetHelper::mParentObject/mOwnerWindow as non-owning; r=smaug

Status: RESOLVED FIXED

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

Comment 1

[(no longer active)]

Attachment: Mark DOMEventTargetHelper::mParentObject/mOwnerWindow as non-owning

Comment 3

[(no longer active)]

Attachment: Clarify the ownership of DOMEventTargetHelper::mParentObject/mOwnerWindow

Comment 4

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

Comment on attachment 8550926 [details] [diff] [review]
Clarify the ownership of DOMEventTargetHelper::mParentObject/mOwnerWindow

Sorry, no, we explicitly got rid of the
DETH->nsGlobalWindow edge at some point in order to release memory
faster when necko is keeping XHR/EventSource/WebSocket alive.
Don't want to add that edge back.

Comment 5

[(no longer active)]

So what is the right fix?

Comment 6

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

I don't know yet. Can we require all the nsIGlobalObjects to support nsWeakRefs?

Comment 7

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

Looks like only TabChildGlobal and WorkerGlobalScope don't support weak refs.

Comment 8

[(no longer active)]

Perhaps we can make them support weak refs or two and MOZ_ASSERT the nsWeakPtr we get out of the nsIGlobalObject to catch future implementations that don't support weak refs?

Comment 9

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

Yeah, sounds good.

Comment 10

[(no longer active)]

Attachment: Clarify the ownership of DOMEventTargetHelper::mParentObject/mOwnerWindow

This patch converts DOMEventTargetHelper::mParentObject into an
nsWeakPtr, and adds fatal assertions to ensure that all future
nsIGlobalObject implementations support weak references.  It also marks
DOMEventTargetHelper::mOwnerWindow as a non-owning reference.

Comment 11

[Ryan VanderMeulen (PTO, back July 27)]

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