669178 - InspectorUI.embeddedBrowserParents seems to be broken and leaks content windows

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Dão Gottwald [:dao]]

Attachment: patch

I don't understand what this code is trying to do, but it looks like it can't possibly work. embeddedBrowserParents looks like it tries to reflect node relationships, but object keys can only be strings, not objects.

Windows with nodes ending up as values in InspectorUI.embeddedBrowserParents are leaked, as that object is never cleared.

Comment 1

[Dão Gottwald [:dao]]

This is responsible for the following browser-chrome test leaks (bug 658738):

2 http://mochi.test:8888/browser/browser/base/content/test/inspector/browser_inspector_treePanel_input.html
1 data:text/html,iframe%20tests%20for%20inspector
1 data:text/html,little%20iframe
1 data:text/html,mouse%20scrolling%20test%20for%20inspector

Comment 2

[Rob Campbell [:rc] (:robcee)]

oh. wow. nice catch.

This code came over wholesale from firebug. I think Paul's fix for bug 665880 revealed this. Code path might have been unreachable before.

Comment 3

[Dão Gottwald [:dao]]

This leaked before bug 665880 got fixed, as far as I can tell.

Comment 4

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 543780 [details] [diff] [review]
patch

the embeddedBrowserParents object contains references to iframe subdocuments. Nodes that should be skipped by the tree panel during construction or display.

Does the HTML panel still work with this patch applied?

Comment 5

[Dão Gottwald [:dao]]

> Does the HTML panel still work with this patch applied?

I'm not sure what exactly I should be looking for. The tests are passing. Is this meaningful evidence?

Comment 6

[Rob Campbell [:rc] (:robcee)]

yes it is. We have some iframe tests in the suite.

I ran a build and it works fine on gmail which is pretty iframe heavy.

Comment 7

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 543780 [details] [diff] [review]
patch

r+ for me.

Comment 8

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/6c2d53b98177