Last Comment Bug 752877 - Cut wrappers after firing [inner,outer]-window-destroyed.
: Cut wrappers after firing [inner,outer]-window-destroyed.
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: All All
-- normal with 3 votes (vote)
: mozilla15
Assigned To: Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on: hueyfix 751420 753621
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-08 05:56 PDT by Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
Modified: 2013-12-27 14:22 PST (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (4.13 KB, patch)
2012-05-08 05:56 PDT, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
bzbarsky: review+
Details | Diff | Splinter Review

Description User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-05-08 05:56:05 PDT
Created attachment 621945 [details] [diff] [review]
Patch
Comment 1 User image Manoj 2012-05-08 14:47:05 PDT
Thanks for filing the follow-up bug.
Comment 2 User image Boris Zbarsky [:bz] (still a bit busy) 2012-05-08 19:07:16 PDT
Comment on attachment 621945 [details] [diff] [review]
Patch

r=me
Comment 3 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-05-09 08:12:51 PDT
https://hg.mozilla.org/mozilla-central/rev/0868da9fac99
Comment 4 User image Nicholas Nethercote [:njn] 2012-05-09 16:46:59 PDT
My tiny brain cannot handle this bug's pithiness.  Which problem does it address?
Comment 5 User image Boris Zbarsky [:bz] (still a bit busy) 2012-05-09 16:51:28 PDT
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 User image Nicholas Nethercote [:njn] 2012-05-09 16:59:21 PDT
Thanks!  So bug 751466 is less urgent now, good.
Comment 7 User image Boris Zbarsky [:bz] (still a bit busy) 2012-05-09 17:02:12 PDT
That was the idea, yes.  ;)
Comment 8 User image Boris Zbarsky [:bz] (still a bit busy) 2012-05-09 17:02:40 PDT
But we should perhaps put that information in that bug.  Kyle, want to do that, since you did the actual testing here?
Comment 9 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-05-09 17:05:06 PDT
I think I already did that in comment 31 ...
Comment 10 User image Michael Foerster 2012-05-15 03:38:13 PDT
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 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2012-05-15 03:47:05 PDT
Bug 695480 (and this one as a followup) is quite risky change, so getting it to Aurora
isn't likely.
Comment 12 User image Justin Lebar (not reading bugmail) 2012-05-15 07:18:53 PDT
> 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 User image Nicholas Nethercote [:njn] 2012-05-15 17:33:12 PDT
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.)

Note You need to log in before you can comment on or make changes to this bug.