897707 - Make more things weak in BEP

Status: RESOLVED INVALID

(Firefox OS Graveyard :: General, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

This also seems to be necessary to get rid of moar leaks.

Comment 1

[Justin Lebar (not reading bugmail)]

Comment on attachment 780614 [details] [diff] [review]
Patch

Why do we keep |let self = this| around?

defineMethod and defineDOMRequestMethod are short-lived functions, so they should be able to use |self| instead of |weakSelf|, except for the entraining problem.  So what you have is a reasonable solution, or alternatively you could use |self| in there and then null it out, as you do.

Either way, please add a comment.

Comment 2

[Justin Lebar (not reading bugmail)]

Note that if this fixes leaks, then that means we're leaking these closure objects.

Comment 3

[Justin Lebar (not reading bugmail)]

And so you may need to be more clever about how you remove the message listeners etc when you discover that the BEP has gone away.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Turns out these were just circular references.  Because we don't handle weak map values very well they appeared to chain up to a root, but the BrowserElementParent was not actually a root.  That is fixed in bug 897701.