388573 - ForecastFox 0.9.5.2 extension leaks 2 DOM Windows on Trunk but not on Branch

Status: RESOLVED FIXED

(Firefox :: General, defect, P3)

Description

[Steve England [:stevee]]

1. New Profile, start firefox
2. (If on trunk) about:config, set extensions.checkCompatibility to false
3. Install ForecastFox 0.9.5.2 from https://addons.mozilla.org/en-US/firefox/downloads/file/1969/forecastfox-0.9.5.2-fx+fl+mz+ns+zm.xpi
4. Restart firefox to complete installation.
5. "Forecastfox Options" window appears, use code "12345", click "OK". Observe icons appear on the right hand side of the status bar.
6. Close firefox snd start back up with leak-logging enabled
7. Close firefox any analyse nspr.log

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a7pre) Gecko/2007071803 Minefield/3.0a7pre ID:2007071803 reports:

Leaked inner window 1b59db8 (outer 18ce760) at address 1b59db8.
 ... with URI "about:blank".
Leaked outer window 18ce760 at address 18ce760.
Summary:
Leaked 2 out of 7 DOM Windows
Leaked 0 out of 42 documents
Leaked 0 out of 3 docshells


Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.5pre) Gecko/2007071603 BonEcho/2.0.0.5pre reports:

Leaked 0 out of 7 DOM Windows
Leaked 0 out of 40 documents
Leaked 0 out of 3 docshells

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

We need someone to debug this to figure out where it's happening. Blocking for now, but it might end up being a 1.9 blocker ...

Comment 2

[Mike Connor [:mconnor]]

moving out bugs that don't need to block b1

Comment 3

[Mike Connor [:mconnor]]

Does this still leak?

Comment 4

[Steve England [:stevee]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9b2pre) Gecko/2007110913 Minefield/3.0b2pre ID:2007110913

Yes. STR in comment 0 and leak-gauge.pl still show similar leakage.

Comment 5

[Mike Schroepfer]

Moving to P2 to investigate whether this is a common leak.  If this is fixed or a specific problem in forcastfox please adjust

Comment 6

[Peter Van der Beken [:peterv]]

This seems a combination of the extension's code keeping some objects alive till very late in shutdown and bug 398219.

0xc5712a0 ChromeWindow f59aae0   via /Users/peterv/Library/Application Support/Firefox-debug/Profiles/debug/extensions/{0538E3E3-7E9B-4d49-8831-A227C80A7AD3}/components/nsForecastfox.js(0xc5515e0 BackstagePass).gHelpersBranch(0x16bf5380 XPCWrappedNative_NoHelper).getPrefType(0x16bf53a0 Function).private(0xc7d0e70 function).object(0x16b92500 Function).__parent__

0xc5712a0 ChromeWindow f59aae0   via /Users/peterv/Library/Application Support/Firefox-debug/Profiles/debug/extensions/{0538E3E3-7E9B-4d49-8831-A227C80A7AD3}/components/nsForecastfox.js(0xc5515e0 BackstagePass).gHelpersBranch(0x16bf5380 XPCWrappedNative_NoHelper).getComplexValue(0x16bf53e0 Function).private(0x168f8318 function).object(0xd39c620 Function).__parent__

0xc5712a0 ChromeWindow f59aae0   via /Users/peterv/Library/Application Support/Firefox-debug/Profiles/debug/extensions/{0538E3E3-7E9B-4d49-8831-A227C80A7AD3}/components/nsForecastfox.js(0xc5515e0 BackstagePass).gHelpersBundle(0xc7d1aa0 XPCWrappedNative_NoHelper).GetStringFromName(0x16bf8900 Function).private(0xd3251e0 function).object(0xd3240a0 Function).__parent__

0xc5712a0 ChromeWindow f59aae0   via /Users/peterv/Library/Application Support/Firefox-debug/Profiles/debug/extensions/{0538E3E3-7E9B-4d49-8831-A227C80A7AD3}/components/nsForecastfox.js(0xc5515e0 BackstagePass).ManagerService(0xc7d2c80 Function).prototype(0xc7d1400 Object).__proto__(0xc7d1420 Object)._branch(0xc7d1b40 XPCWrappedNative_NoHelper).addObserver(0x178adb80 Function).private(0x16b9ceb8 function).object(0x16b9a640 Function).__parent__

0xc5712a0 ChromeWindow f59aae0   via /Users/peterv/Library/Application Support/Firefox-debug/Profiles/debug/extensions/{0538E3E3-7E9B-4d49-8831-A227C80A7AD3}/components/nsForecastfox.js(0xc5515e0 BackstagePass).doc(0x178c7160 XMLDocument).XPCWrappedNativeProto::mJSProtoObject(0x16bf8e80 XPC_WN_ModsAllowed_Proto_JSClass).documentElement getter(0x16bf7000 Function).private(0x168f81b0 function).object(0xc576b20 Function).__parent__

0xc5712a0 ChromeWindow f59aae0   via /Users/peterv/Library/Application Support/Firefox-debug/Profiles/debug/extensions/{0538E3E3-7E9B-4d49-8831-A227C80A7AD3}/components/nsForecastfox.js(0xc5515e0 BackstagePass).doc(0x178c7160 XMLDocument).XPCWrappedNativeProto::mJSProtoObject(0x16bf8e80 XPC_WN_ModsAllowed_Proto_JSClass).getElementsByTagName(0x16bf70a0 Function).private(0x16bfa528 function).object(0x16bf7080 Function).__parent__

0xc5712a0 ChromeWindow f59aae0   via /Users/peterv/Library/Application Support/Firefox-debug/Profiles/debug/extensions/{0538E3E3-7E9B-4d49-8831-A227C80A7AD3}/components/nsForecastfox.js(0xc5515e0 BackstagePass).doc(0x178c7160 XMLDocument).XPCWrappedNativeProto::mJSProtoObject(0x16bf8e80 XPC_WN_ModsAllowed_Proto_JSClass).createElementNS(0xc567c20 Function).private(0x106a65e8 function).object(0x106a4200 Function).__parent__


nsForecastfox.js(0xc5515e0 BackstagePass).doc(0x178c7160 XMLDocument) looks like it's coming from _loadPacks in pack-service.js, another undeclared JS variable (holding a document) ending up on the global object (see bug 398270 for a similar problem):

  _loadPacks: function PackService__loadPacks()
  {
    //setup error variables
    const PREFIX = "ff.packs.load.";
    var name = this.bundle.GetStringFromName(PREFIX + "name");
    var message = "";  
    
    //get icons.xml
    var dir = this._dskSvc.get("", TYPE_ICONS);
    var file = this._dskSvc.get("icons.xml", TYPE_PROFILE);
    if (file.exists()) {
    
      //file not readable-writable
      if (!file.isReadable() || !file.isWritable()) {
        message = this.bundle.formatStringFromName(PREFIX + ".perms.message",
                                                   [file.path], 1);
        this._error.init(SEVERITY_ERROR, name, message);
        return false;                                            
      }
      
      //read the file into a document
      doc = this._dskSvc.read(file);
...

Comment 7

[Boris Zbarsky [:bzbarsky]]

Sounds like we need to file a bug on the extension, then?

Comment 8

[Peter Van der Beken [:peterv]]

And fix bug 398219 probably.

Comment 9

[Steve England [:stevee]]

FWIW, after testing about 25 extensions, I found 4 that showed no leakage on branch but similar amounts of leakage on trunk:

- ForecastFox 0.9.5.2 (This bug)
- FoxClocks 2.1.93 (bug 388577)
- NoScript 1.1.6.02
- FlashGot 0.6.1

I didn't file the last two here because Giorgio Maone said he was going to investigate himself.

Comment 10

[Peter Van der Beken [:peterv]]

Fixing the declaration of doc doesn't solve the leaks.

Comment 11

[Peter Van der Beken [:peterv]]

The fix for bug 398219 doesn't seem to fix it. Will need to do some more debugging.

Comment 12

[Peter Van der Beken [:peterv]]

Could someone check whether this still leaks on Windows on trunk? With the patch from bug 409424 I don't see the leak anymore on OS X.

Comment 13

[Peter Van der Beken [:peterv]]

Actually, nevermind comment 12. I still see the leak with the exact steps to reproduce from comment 0.

Comment 14

[Mike Schroepfer]

(In reply to comment #13)
> Actually, nevermind comment 12. I still see the leak with the exact steps to
> reproduce from comment 0.
> 

any ideas on what it is?

Comment 15

[Peter Van der Beken [:peterv]]

Probably bug 412491, so should be fixed now.

Comment 16

[Carsten Book [:Tomcat]]

Hi Mike and Peter, i was not able to reproduce this with a Leak Testing Build that included the patch from Bug 412491. So i think this is fixed for now.

I will retest Forecast Fox as soon its released compatible for Firefox 3b3 (or Firefox3) for leaks.

Comment 17

[Mike Schroepfer]

(In reply to comment #16)
> Hi Mike and Peter, i was not able to reproduce this with a Leak Testing Build
> that included the patch from Bug 412491. So i think this is fixed for now.
> 
> I will retest Forecast Fox as soon its released compatible for Firefox 3b3 (or
> Firefox3) for leaks.
> 

Do we want to re-test the set of extensions you filed bugs on with a recent nightly to see which other leaks are left?

Comment 18

[Steve England [:stevee]]

Carsen, I can't reproduce the leakage anymore either (using leak-gauge.pl) but ForecastFox is currently not working on the trunk atm, so maybe this is the reason why we are seeing no leaks anymore (bug 412598)

Comment 19

[Carsten Book [:Tomcat]]

> (In reply to comment #16)
> Do we want to re-test the set of extensions you filed bugs on with a recent
> nightly to see which other leaks are left?
> 

Yes, working on this :-) 

(In reply to comment #18)
> Carsen, I can't reproduce the leakage anymore either (using leak-gauge.pl) but
> ForecastFox is currently not working on the trunk atm, so maybe this is the
> reason why we are seeing no leaks anymore (bug 412598)
> 

Hey Stevee, yeah i wait till bug 412598 is fixed and also this extension is official released for beta 3/Firefox 3 and will test then also other top extensions.

Comment 20

[Steve England [:stevee]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008012200 Minefield/3.0b3pre ID:2008012200

FWIW with the above hourly (post bug 412598) I can no longer get leak-gauge.pl to report any leakage using the STR in comment 0.

Comment 21

[Mike Connor [:mconnor]]

Marking as FIXED by bug 412491