344545 - DOM Inspector 1.8.1b1 Leak (dom.js)

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, defect)

Description

[Zulkarnain K. [:LouCypher]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060710 Firefox/2.0b1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060710 Firefox/2.0b1 - Build ID: 2006071020

Clean profile

Reproducible: Always

Steps to Reproduce:
1. Run Firefox
2. Open Error Console
3. Run 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:  
Error Console will display:

Error: viewer is not defined
Source File: chrome://inspector/content/viewers/dom/dom.js
Line: 1005



Error Console will continue to display the error message until the inspected document/window is closed.
On prior version of Firefox (with patched DOMI from bug 3366961) where Leak Monitor is installed and running, Leak Monitor will report memory leaks (see next attachment).

Solution:
Modify dom.js and edit function DOMViewer_destroy() to:

  PrefUtils.removeObserver("inspector", PrefChangeObserver);
+ try {
+   viewer.removeClickListeners();
+ } catch(ex) {
+ }
  viewer = null;

Comment 1

[Zulkarnain K. [:LouCypher]]

Attachment: Leak Monitor report

Comment 2

[Alex Vincent [:WeirdAl]]

Patches accepted, Lou :)

Comment 3

[Shawn Wilsher :sdwilsh]

Attachment: v1

Can someone see if this takes care of it (ie the reporter)?  I don't have the extension installed.  If this works, I'll request review.

Comment 4

[Shawn Wilsher :sdwilsh]

any takers on the whole testing thing?

Comment 5

[Daniel Brooks [:db48x]]

Comment on attachment 230745 [details] [diff] [review]
v1

r=db48x

Comment 6

[[e-mail]]

Unless you can give me steps to reproduce an exception removing the click listeners, I'd only give you sr+ without the try/catch.

Comment 7

[Shawn Wilsher :sdwilsh]

Attachment: v1.1

Can't figure out why we need the try catch block - no exceptions thrown.

Comment 8

[Daniel Brooks [:db48x]]

Comment on attachment 251398 [details] [diff] [review]
v1.1

r=db48x

Comment 9

[Shawn Wilsher :sdwilsh]

Checked in by timeless.