Cycle between nsXULTooltipListener and nsGlobalWindows causes leaks

RESOLVED FIXED

Status

()

Core
XUL
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: peterv, Assigned: smaug)

Tracking

({memory-leak})

Trunk
memory-leak
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

11 years ago
Created attachment 261968 [details]
Testcase

See bug 376802, comment 11 and others. In the attached testcase, hover over the button and the browser will quit, leaking nsGlobalWindows (the first time you'll need to allow UniversalXPConnect privileges). It shows one way to keep a nsGlobalWindow alive, I'll also attach part of an ownership graph.
(Reporter)

Comment 1

11 years ago
Created attachment 261969 [details]
Onwership graph

The missing edge to nsXULTooltipListener comes from nsLayoutStatics (which keeps it because there are live nsGlobalWindows).

Comment 2

11 years ago
The tooltiplistener keeps a strong reference to the tooltip as long as the tooltip is shown, so waiting for a popuphiding event that never comes. I assume before bug 376802 the tooltip listener would have been destroyed when the source element was unbound from the tree, thus releasing the reference to the tooltip?
(Assignee)

Comment 3

11 years ago
Created attachment 262751 [details] [diff] [review]
using cycle collector, v1
Assignee: neil → Olli.Pettay
Status: NEW → ASSIGNED
Attachment #262751 - Flags: review?(neil)
(Assignee)

Updated

11 years ago
Attachment #262751 - Flags: superreview?(peterv)
(Reporter)

Comment 4

11 years ago
Comment on attachment 262751 [details] [diff] [review]
using cycle collector, v1

That should work, yes.
Attachment #262751 - Flags: superreview?(peterv) → superreview+

Comment 5

11 years ago
Comment on attachment 262751 [details] [diff] [review]
using cycle collector, v1

>-
>-  // nsISupports
>-  NS_DECL_ISUPPORTS
>+  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
>+  NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(nsXULTooltipListener,
>+                                           nsIDOMMouseListener)
> 
>   // nsIDOMMouseListener
Nit: any reason why you removed the nsISupports comment?
Attachment #262751 - Flags: review?(neil) → review+
(Assignee)

Comment 6

11 years ago
Because NS_DECL_CYCLE_COLLECTING_*** does something else too, not only
declares nsISupports.
(Assignee)

Updated

11 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Flags: in-testsuite-

Updated

10 years ago
Keywords: mlk

Updated

10 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.