336961 - DOM Inspector leaks (dom.js)

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, defect)

Description

[Pavel Cvrcek [:JasnaPaka]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060506 Minefield/3.0a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060506 Minefield/3.0a1

Firefox Trunk, clean profile.

Reproducible: Always

Steps to Reproduce:
1. Install Leak Monitor Extension (http://dbaron.org/mozilla/leak-monitor/) and
Nightly Tester Tools (https://addons.mozilla.org/firefox/958/).
2. Close Firefox.
3. Open Firefox, open DOM Inspector (Tools -> DOM Inspector), close DOM Inspector.

Actual Results:  
Leak alert is showed.

Comment 1

[Pavel Cvrcek [:JasnaPaka]]

Attachment: Leak Monitor Extension - Result

Comment 2

[timeless]

this isn't useful. 1. devs shouldn't be forced to load huge images that ff auto shrinks. if you can't retype the text into the bug, and if your extension can't copy the text for you (which you should file a bug about), then please don't file a bug about it.

oh, and i hope you realize that no one is actively working on this component, so if you want something done about it, you're expect to write the patch yourself (or not use the extension).

Comment 3

[Jindrich Pozlovsky]

I can confirm this with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20060507 BonEcho/2.0a1.

OS -> All?

Comment 4

[Boris Zbarsky [:bzbarsky]]

We add a pref observer at http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/extensions/inspector/resources/content/viewers/dom/dom.js&rev=1.38&mark=80#79 and never remove it.

Comment 5

[Régis Caspar]

Attachment: Fix

Add a unload routine to remove the pref observer

Comment 6

[[e-mail]]

Comment on attachment 221243 [details] [diff] [review]
Fix

> function DOMViewer_initialize()
> {
>@@ -61,6 +62,12 @@
>   viewer.initialize(parent.FrameExchange.receiveData(window));
> }
Note to patch author: sometimes it helps to specify a non-default -u value.

Comment 7

[Régis Caspar]

(In reply to comment #6)
> Note to patch author: sometimes it helps to specify a non-default -u value.
Yes, I will pay attention to this the next time. Thanks

Comment 8

[Régis Caspar]

 -> updated keyword / ASSIGNED.

Is it something missing or this is just waiting for someone to check this in?

Comment 9

[:Gavin Sharp [email: [e-mail]]]

mozilla/extensions/inspector/resources/content/viewers/dom/dom.js 	1.40

Comment 10

[Régis Caspar]

VERIFIED on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060530 Minefield/3.0a1 ID:2006053004 [cairo]

Will ask for branch approval in a few days.

Comment 11

[Régis Caspar]

(In reply to comment #10)
> Will ask for branch approval in a few days.

This seems to need to be checked-in in branch but I don't know where/who to ask for approval/check-in. Any direction would be great :)

Comment 12

[Adam Guthrie]

*** Bug 333364 has been marked as a duplicate of this bug. ***

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Transferring blocking flag from duplicate.

Comment 14

[Zulkarnain K. [:LouCypher]]

I found another leak in DOMi

Reproducible: Always

Steps to Reproduce:
1. Install Leak Monitor Extension (http://dbaron.org/mozilla/leak-monitor/)
2. Restart Firefox.
3. Open DOM Inspector (Tools -> DOM Inspector)
4. From DOM Inspector window, click the Select Element By Click button.
5. Close DOM Inspector without selecting any object/element.

Actual Results:
Leak alert is showed (see attachment).

Tested on:
- User Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4 Build ID: 2006050817
- User Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060526 BonEcho/2.0a3 Build ID: 2006052616

Comment 15

[Zulkarnain K. [:LouCypher]]

Attachment: DOM Inspector leak

Solution:
Add these before viewer = null; to function DOMViewer_destroy() in the patch:

try {
  viewer.removeClickListeners();
} catch(ex) {
}

Comment 16

[Adam Guthrie]

LouCypher, please test again after this patch actually lands on the 1.8 branch. Then if you can still reproduce, file a new bug.

Comment 17

[Zulkarnain K. [:LouCypher]]

(In reply to comment #16)
> LouCypher, please test again after this patch actually lands on the 1.8 branch.
> Then if you can still reproduce, file a new bug.
> 

Looks like I have to file a new bug.
Not in this weekend, though.

Comment 18

[:Gavin Sharp [email: [e-mail]]]

mozilla/extensions/inspector/resources/content/viewers/dom/dom.js 	1.35.4.2

Comment 19

[Zulkarnain K. [:LouCypher]]

Bug 344545 reported.