Last Comment Bug 728663 - [New Tab Page] Moving thumbnail on the grid leaks about:newtab
: [New Tab Page] Moving thumbnail on the grid leaks about:newtab
Status: RESOLVED FIXED
[MemShrink:P2]
:
Product: Core
Classification: Components
Component: Widget: Win32 (show other bugs)
: 13 Branch
: All Windows 7
: -- normal (vote)
: mozilla15
Assigned To: Kyle Huey [:khuey] (khuey@mozilla.com)
:
Mentors:
: 741700 (view as bug list)
Depends on:
Blocks: leakychrome 455553
  Show dependency treegraph
 
Reported: 2012-02-19 04:11 PST by Loic
Modified: 2012-05-11 11:34 PDT (History)
15 users (show)
khuey: in‑litmus?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (4.21 KB, patch)
2012-04-19 18:34 PDT, Kyle Huey [:khuey] (khuey@mozilla.com)
jmathies: review+
Details | Diff | Review

Description Loic 2012-02-19 04:11:01 PST
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] 2012-02-19 05:11:52 PST
Did you try CC'ing again after a few seconds? I can't reproduce the leak.
Comment 2 Loic 2012-02-19 05:22:24 PST
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] 2012-02-19 05:48:00 PST
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 2012-02-19 06:03:39 PST
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] (high review load, please consider other reviewers) 2012-02-19 09:34:47 PST
I can't reproduce this.
Comment 6 Loic 2012-02-19 10:01:16 PST
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 2012-02-19 18:06:16 PST
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] 2012-02-21 02:43:50 PST
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 [:khuey] (khuey@mozilla.com) 2012-04-19 18:02:53 PDT
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 [:khuey] (khuey@mozilla.com) 2012-04-19 18:34:30 PDT
Created attachment 616836 [details] [diff] [review]
Patch

Holding onto the object that causes the leak seems to be unnecessary, so lets not do that.
Comment 11 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-04-19 20:25:43 PDT
*** Bug 741700 has been marked as a duplicate of this bug. ***
Comment 12 Jim Mathies [:jimm] 2012-04-20 04:40:29 PDT
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 [:khuey] (khuey@mozilla.com) 2012-04-21 12:08:28 PDT
> 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 [:khuey] (khuey@mozilla.com) 2012-05-11 11:33:38 PDT
https://hg.mozilla.org/mozilla-central/rev/667ab2c02a6c

Note You need to log in before you can comment on or make changes to this bug.