728663 - [New Tab Page] Moving thumbnail on the grid leaks about:newtab

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Loic]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0a1) Gecko/13.0a1 Firefox/13.0a1
Build ID: 20120218031156

Steps to reproduce:

After moving a website thumbnail on the grid of new tab page, about:newtab document stays in memory.

STR:
1) Be sure new tab page is enabled and showed
2) Open a new tab
3) Select a thumbnail and move it on the grid
4) Close the tab
5) Open about:cc (see https://bugzilla.mozilla.org/show_bug.cgi?id=728407#c0)
6) Run cycle collector and find documents in the log

Result:
0x86e8000 [rc=3] nsDocument normal (XUL) about:newtab
    Roots: 1
        0x74dfd30 [rc=1] root nsDOMDataTransfer

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Did you try CC'ing again after a few seconds? I can't reproduce the leak.

Comment 2

[Loic]

After CC'ing 2 or 3 times in about:memory and running a cycle collector in about:cc, I see:
0xb3f2800 [rc=3] nsDocument normal (XUL) about:newtab
    Roots: 1
        0x86290c0 [rc=2] root nsNodeInfo (xhtml) div

Anyway I'm able to reproduce the leak each time I move a thumbnail on the grid.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Did you try to reproduce that in safe-mode? Maybe some add-on is the culprit? I'm still not seeing this.

Comment 4

[Loic]

If I try in safe mode, I cannot use add-on about:cc (https://bugzilla.mozilla.org/attachment.cgi?id=598349) anymore to run cycle collector and see the leak.

But I tried with all add-ons/plugins/HWA/themes disabled (only about:cc is enabled) and the leak is still here.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

I can't reproduce this.

Comment 6

[Loic]

It's weird, I'm testing with a new fresh Nightly profile and the leak is still here.

Do you have some ideas to explore why it's leaking on my machine and not on yours?

Comment 7

[Jim Jeffery not reading bug-mail 1/2/11]

Not sure its related, but I see this: 
0x25122000 [rc=3] nsDocument normal (XUL) about:newtab
    Roots: 1
        0x262c9ec0 [rc=2] root nsNodeInfo (xhtml) div

Using latest hourly m-c build based on cset: 
https://hg.mozilla.org/mozilla-central/rev/24f2c7e26fbd

win7 x64

I opened about:cc after opening a new tab with xtrl+t.  Could be the document is normal, since I opened the newtab page.

Comment 8

[Virgil Dicu [:virgil] [QA]]

I can reproduce this on any Windows (W7 with 64 and 32 bits, Vista). 
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/13.0 Firefox/13.0a1

1. Start Firefox with clean profile
2. Install https://bugzilla.mozilla.org/attachment.cgi?id=598349
3. Open about:cc
4. Open a new tab.
5. Move a thumbnail to a new position.
6. Close the new tab
7. Run Cycle collector
8. Find Documents.

The same results as Loic's.

Doesn't occur on Mac and Linux(OS 10.6 and Ubuntu 11.10)

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

The missing Release:

 	xul.dll!nsNodeInfo::Release()  Line 218	C++
 	xul.dll!nsCOMPtr<nsINodeInfo>::~nsCOMPtr<nsINodeInfo>()  Line 521 + 0x6 bytes	C++
 	xul.dll!nsINode::~nsINode()  Line 211 + 0x8 bytes	C++
 	xul.dll!nsHTMLDivElement::`scalar deleting destructor'()  + 0xb bytes	C++
 	xul.dll!nsNodeUtils::LastRelease(nsINode * aNode)  Line 283 + 0xb bytes	C++
 	xul.dll!nsGenericElement::Release()  Line 5100 + 0xd2 bytes	C++
 	xul.dll!nsHTMLDivElement::Release()  Line 97 + 0xc bytes	C++
 	xul.dll!nsCOMPtr<nsIDOMElement>::~nsCOMPtr<nsIDOMElement>()  Line 521 + 0x6 bytes	C++
 	xul.dll!nsDOMDataTransfer::~nsDOMDataTransfer()  Line 114 + 0x8 bytes	C++
 	xul.dll!nsDOMDataTransfer::Release()  Line 80 + 0xc3 bytes	C++
 	xul.dll!nsCOMPtr<nsIDOMDataTransfer>::~nsCOMPtr<nsIDOMDataTransfer>()  Line 521 + 0x6 bytes	C++
>	xul.dll!nsNativeDragSource::Release()  Line 96 + 0x7 bytes	C++
 	xul.dll!nsDragService::~nsDragService()  Line 95 + 0x1a bytes	C++
 	xul.dll!nsDragService::`scalar deleting destructor'()  + 0xb bytes	C++
 	xul.dll!nsBaseDragService::Release()  Line 91 + 0xb9 bytes	C++
 	xul.dll!nsCOMPtr_base::assign_assuming_AddRef(nsISupports * newPtr)  Line 469 + 0x8 bytes	C++
 	xul.dll!nsCOMPtr_base::assign_with_AddRef(nsISupports * rawPtr)  Line 89 + 0x8 bytes	C++
 	xul.dll!FreeFactoryEntries(const nsID & aCID, nsFactoryEntry * aEntry, void * arg)  Line 1088 + 0xa bytes	C++
 	xul.dll!nsBaseHashtable<nsIDHashKey,nsFactoryEntry *,nsFactoryEntry *>::s_EnumReadStub(PLDHashTable * table, PLDHashEntryHdr * hdr, unsigned int number, void * arg)  Line 396 + 0x13 bytes	C++
 	xul.dll!PL_DHashTableEnumerate(PLDHashTable * table, PLDHashOperator (PLDHashTable *, PLDHashEntryHdr *, unsigned int, void *)* etor, void * arg)  Line 750 + 0xb bytes	C++
 	xul.dll!nsBaseHashtable<nsIDHashKey,nsFactoryEntry *,nsFactoryEntry *>::EnumerateRead(PLDHashOperator (const nsID &, nsFactoryEntry *, void *)* enumFunc, void * userArg)  Line 206 + 0xf bytes	C++
 	xul.dll!nsComponentManagerImpl::FreeServices()  Line 1101	C++
 	xul.dll!mozilla::ShutdownXPCOM(nsIServiceManager * servMgr)  Line 673	C++
 	xul.dll!ScopedXPCOMStartup::~ScopedXPCOMStartup()  Line 1129 + 0x7 bytes	C++

Comment 10

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Holding onto the object that causes the leak seems to be unnecessary, so lets not do that.

Comment 12

[Jim Mathies [:jimm]]

Comment on attachment 616836 [details] [diff] [review]
Patch

Can't see a reason to store that in a member variable. According to bonsai, we've been doing it this way since the beginning. Let's see what happens with your change. I'd suggest waiting till after the next merge to land this.

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

> I'd suggest waiting till after the next merge to land this.

I agree, but if we don' see any fallout I think we may want to uplift this.

Comment 14

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/mozilla-central/rev/667ab2c02a6c