Last Comment Bug 788015 - Speed Dial add-on keeps preview pages loaded in hidden iframe when no longer needed
: Speed Dial add-on keeps preview pages loaded in hidden iframe when no longer ...
Status: RESOLVED FIXED
[MemShrink:P3]
:
Product: Tech Evangelism
Classification: Other
Component: Add-ons (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
:
Mentors:
Depends on:
Blocks: LeakyAddons ZombieCompartments
  Show dependency treegraph
 
Reported: 2012-09-03 14:08 PDT by pvelkovski
Modified: 2012-10-09 11:36 PDT (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
about_compartments (19.93 KB, text/plain)
2012-09-03 14:08 PDT, pvelkovski
no flags Details

Description pvelkovski 2012-09-03 14:08:25 PDT
Created attachment 657913 [details]
about_compartments

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:15.0) Gecko/20100101 Firefox/15.0
Build ID: 20120825192204

Steps to reproduce:

Having Speed Dial add-on with the following sites memorized www.time.mk and www.plusinfo.mk. Opening those pages in new tabs, and then closing them causes ghost windows reported in about:compartments?verbose
Comment 1 Kris Maglione [:kmag] 2012-09-04 16:44:06 PDT
As far as I can tell, what's happening here is that the add-on is loading the web pages into a document global to generate the preview and never unloading it. I'm only ever seeing one of these compartments created, and I don't think it's a zombie.
Comment 2 Nicholas Nethercote [:njn] 2012-09-04 19:28:57 PDT
I can only reproduce this when I add a site to the speed dial.  Simply loading a site via the speed dial doesn't cause the zombie.

> As far as I can tell, what's happening here is that the add-on is loading
> the web pages into a document global to generate the preview and never
> unloading it. 

Which document?  Why doesn't the chrome->content link cutting doesn't prevent it?

> I'm only ever seeing one of these compartments created, and I
> don't think it's a zombie.

I've seen multiple compartments if they all correspond to a single page.  They aren't immortal, because if you add a new site to the speed dial the old zombies will disappear (actually, this seems to happen sometimes but not always).

It's still something that should be fixed.  Can we contact the author?
Comment 3 Kris Maglione [:kmag] 2012-09-04 19:32:51 PDT
(In reply to Nicholas Nethercote [:njn] from comment #2)
> I can only reproduce this when I add a site to the speed dial.  Simply
> loading a site via the speed dial doesn't cause the zombie.

Agreed.

> > As far as I can tell, what's happening here is that the add-on is loading
> > the web pages into a document global to generate the preview and never
> > unloading it. 
> 
> Which document?  Why doesn't the chrome->content link cutting doesn't
> prevent it?

I'm just guessing that it gets loaded into an iframe that doesn't get destroyed (there would be no links to cut in that case because the document would be rightly alive). I haven't had a chance to look at the code yet.

> > I'm only ever seeing one of these compartments created, and I
> > don't think it's a zombie.
> 
> I've seen multiple compartments if they all correspond to a single page. 
> They aren't immortal, because if you add a new site to the speed dial the
> old zombies will disappear (actually, this seems to happen sometimes but not
> always).

Right, I've seen multiple compartments, but the others are for subframes of the page I'm adding. Adding a new document to Speed Dial wipes all of those out and adds compartments for the new page that it snapshots.

> It's still something that should be fixed.  Can we contact the author?

Yes.
Comment 4 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-09-04 19:42:32 PDT
Speed Dial seems to load a new copy of the webpage in a hidden iframe somewhere.  You can see this yourself by loading a website, noting the id of the outer window in about memory, and then adding it to the speed dial.  There will be a new outer window with a different id.  If you close the tab the original will be GCd, but speed dial keeps the other around somewhere.
Comment 5 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-09-04 19:55:04 PDT
Not directly relevant here, but Speed Dial causes shutdown leaks too.
Comment 6 pvelkovski 2012-09-05 05:23:16 PDT
Can someone with a more experience change the title of this bug report, because when I created it, I really did notice that I had ghost windows, and though that the culprit was the  Speed Dial add-on. What I realized after being more thorough is that the Speed Dial add-on causes User Compartments to be left behind, and the culprit for the Ghost Windows is DownloadHelper add-on. Can someone please test the DownloadHelper add-on (which is not even working properly in Firefox 15), because after filing a misleading bug report, I'm not sure if I'm the right person to file a new bug report.
Comment 7 Andrew McCreight [:mccr8] 2012-09-05 11:08:26 PDT
(In reply to pvelkovski from comment #6)
> because after filing a misleading bug
> report, I'm not sure if I'm the right person to file a new bug report.
Don't be so hard on yourself, you narrowed it down to a single addon, so it was a very useful report. :)
Comment 8 Jorge Villalobos [:jorgev] 2012-09-05 12:08:57 PDT
So, given that the pages are being kept intentionally, should this still be considered a bug?
Comment 9 Kris Maglione [:kmag] 2012-09-05 12:10:23 PDT
They're kept after they're no longer needed, so I'd say yes.
Comment 10 Josep del Rio 2012-09-05 12:41:32 PDT
I'm Speed Dial's author; I can confirm that this is a "feature"; while there's no reason to keep it in memory, the idea was to keep it there just in case the user would repeatedly make thumbnails from it (like creating a thumbnail from the crop editor, and then refreshing the page due to that change).

The reason why I didn't consider this memory consumption important was that it's limited (as it's cleared when another site is loaded), and it's for a page that is supposed to be loaded a number of times, so if the resources are shared, they should load faster.

That said, I can see how the recent push for low memory consumption in Firefox makes this decision wrong; this can be fixed easily from Speed Dial, so the next version will not have this behavior.
Comment 11 Josep del Rio 2012-09-05 12:42:20 PDT
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #5)
> Not directly relevant here, but Speed Dial causes shutdown leaks too.

What is a shutdown leak?
Comment 12 Jorge Villalobos [:jorgev] 2012-10-03 11:23:12 PDT
Josep, do you have a timeline for your new version?
Comment 13 Josep del Rio 2012-10-06 12:55:45 PDT
A new version with a change that resolves this bug is on queue for reviewing at AMO.

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