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

RESOLVED FIXED in mozilla15

Status

()

Core
Widget: Win32
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Loic, Assigned: khuey)

Tracking

(Blocks: 1 bug)

13 Branch
mozilla15
All
Windows 7
Points:
---
Dependency tree / graph
Bug Flags:
in-litmus ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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
(Reporter)

Updated

6 years ago
Blocks: 728407, 455553
Component: Untriaged → Tabbed Browser
OS: Windows 7 → All
Hardware: x86_64 → All
Whiteboard: [MemShrink]
Did you try CC'ing again after a few seconds? I can't reproduce the leak.
(Reporter)

Comment 2

6 years ago
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.
Did you try to reproduce that in safe-mode? Maybe some add-on is the culprit? I'm still not seeing this.
(Reporter)

Comment 4

6 years ago
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

6 years ago
I can't reproduce this.
(Reporter)

Comment 6

6 years ago
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?
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.
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)
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Updated

6 years ago
OS: All → Windows 7
Whiteboard: [MemShrink] → [MemShrink:P2]

Updated

6 years ago
QA Contact: untriaged → tabbed.browser
Assignee: nobody → khuey
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++
Component: Tabbed Browser → Drag and Drop
Product: Firefox → Core
QA Contact: tabbed.browser → drag-drop
Created attachment 616836 [details] [diff] [review]
Patch

Holding onto the object that causes the leak seems to be unnecessary, so lets not do that.
Attachment #616836 - Flags: review?(jmathies)
Component: Drag and Drop → Widget: Win32
QA Contact: drag-drop → win32
Duplicate of this bug: 741700

Comment 12

5 years ago
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.
Attachment #616836 - Flags: review?(jmathies) → review+
> 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.
https://hg.mozilla.org/mozilla-central/rev/667ab2c02a6c
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Flags: in-litmus?
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.