Closed Bug 725875 Opened 13 years ago Closed 13 years ago

Gridtube 1.4.3 add-on causes a single youtube.com zombie compartment

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: nmaier, Unassigned)

References

Details

(Whiteboard: [MemShrink])

Attachments

(1 file)

Attached file Affected version 1.4.3
STR: - Open a youtube search results page - Wiggle your mouse over some result item, in particular the question mark icon that is displayed as the result item description is expanded on mouse over. - Close the tab - Open about:memory?verbose - Hit Minimize memory use a couple of times - Look for any compartments for youtube -> Still present This seems to happen due to implicit variable declarations in overlay.js#popupShow/popupHide: node is missing a |var|. I cannot reproduce after adding the missing |var| declarations. As this seems to be due to an implicit variable declaration, at most one content window + compartment will be leaked. Using the steps above, it can be determined that in fact at most one window + compartment is leaked. Previous versions (1.4.2) might be affected, too, as they share the same suspected culprit, but in the add-on default configuration the leak is not reproducible.
This is the second one this week where an add-on leak has been found to be caused by a missing |var|, and which "use strict;" would have caught. Grr.
Well, it's usually not as easy as sticking in "use strict"; You'll actually spend a lot of time fixing your code. I'd be all in favor of a new "use noimplicitdecl"; or something similar and force public add-ons to use that or strict mode in all code via AMO policy, maybe turning it on by default for chrome code after a grace period. Here is a long-fixed window leak I created myself. Leaked our DownThemAll! windows, not content pages: https://bugs.downthemall.net/changeset/2744 Another one regarding references keeping windows alive: https://bugs.downthemall.net/changeset/2734 And a very funky one: https://bugs.downthemall.net/changeset/2750
Nils reviewed the add-on and pointed out the leak to the developer. It should be fixed soon, hopefully.
I am the addon author. Thanks for noticing this bug. Indeed it caused the aforementioned memory leak problem. I didn't declare the specific variable 'node' as 'var'. I fixed this, by declaring the variable properly in both functions (popupShow/popupHide). After than I didn't get the same problem again. I will resubmit for full review as new version.
Version 1.4.4 was submitted and made public that fixes the leaks
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: