752877 - Cut wrappers after firing [inner,outer]-window-destroyed.

Status: RESOLVED FIXED

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

Description

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

Attachment: Patch

Comment 1

[Manoj]

Thanks for filing the follow-up bug.

Comment 2

[Boris Zbarsky [:bzbarsky]]

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

r=me

Comment 3

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

https://hg.mozilla.org/mozilla-central/rev/0868da9fac99

Comment 4

[Nicholas Nethercote [inactive]]

My tiny brain cannot handle this bug's pithiness.  Which problem does it address?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Before this fix we'd post an async event to the event loop, then cut the wrappers from chrome to content.  When the event fired some time later, we'd send a notification to chrome that the window is going away.  Some chrome (including old addon SDK versions) would then try to run cleanup code that touched the window, and this code would throw because the wrappers had been neutered.

This fix just moves the neutering of the wrappers to run right after the notification, from the async event.

Comment 6

[Nicholas Nethercote [inactive]]

Thanks!  So bug 751466 is less urgent now, good.

Comment 7

[Boris Zbarsky [:bzbarsky]]

That was the idea, yes.  ;)

Comment 8

[Boris Zbarsky [:bzbarsky]]

But we should perhaps put that information in that bug.  Kyle, want to do that, since you did the actual testing here?

Comment 9

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

I think I already did that in comment 31 ...

Comment 10

[Michael Foerster]

Any chance to to get this solution (and the under-laying bug 695480) elevated to Aurora (mozilla14) ?

With this fix, the win in term of memory and leaks is so much greater than the hassle of (some) breaking Add-ons.

Please correct me if I'm wrong.

Comment 11

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

Bug 695480 (and this one as a followup) is quite risky change, so getting it to Aurora
isn't likely.

Comment 12

[Justin Lebar (not reading bugmail)]

> Any chance to to get this solution (and the under-laying bug 695480) elevated to Aurora (mozilla14) ?

Lots of bugfixes landed after bug 695480 -- see the list of dependent bugs.  So there's really no way we're going to port this up to Aurora.

Comment 13

[Nicholas Nethercote [inactive]]

And bug 695480 is a big enough change that we want a full development cycle to shake out any problems it causes.  (This very bug is a perfect example of the kind of problem we need to shake out.)