Status

()

defect
RESOLVED FIXED
9 years ago
3 months ago

People

(Reporter: jruderman, Assigned: smaug)

Tracking

(Blocks 2 bugs, {memory-leak, testcase})

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 final+, status1.9.2 ?, status1.9.1 ?)

Details

(Whiteboard: [hardblocker])

Attachments

(5 attachments, 1 obsolete attachment)

Reporter

Description

9 years ago
Posted file testcase
Steps:
1. Run with XPCOM_MEM_LEAK_LOG=2
2. Load the testcase.
3. Quit.

Result:
* trace-refcnt reports leaks (8 nsGlobalWindow, 19 nsDocument, 2 nsDocShell)
Reporter

Updated

9 years ago
blocking2.0: --- → ?
Peter, this may be worth looking into as you're debugging similar leaks. Not sure this is strictly a blocker, but blocking for now.
Assignee: nobody → peterv
blocking2.0: ? → final+
Whiteboard: hardblocker
The test case seems to leak in 1.9.1 and 1.9.2 too.
status1.9.1: --- → ?
status1.9.2: --- → ?
OS: Mac OS X → All
Hardware: x86 → All
Mounir is looking into this, reassigning.
Assignee: peterv → mounir.lamouri
Whiteboard: hardblocker → [hardblocker]
Assignee

Comment 4

9 years ago
Posted patch an assertionSplinter Review
The testcase causes this assertion to fire.
Assignee

Comment 5

9 years ago
We probably should do this, though we should perhaps also
traverse docshell's mScriptGlobal and mContentViewer
(and make DocumentViewerImpl ccollectable).
Assignee

Comment 6

9 years ago
Comment on attachment 502221 [details] [diff] [review]
Don't load if the owner document isn't active

Boris, what do you think about this.
We sure shouldn't try to load the frame if ownerDoc
has already been removed from its docshell.
Attachment #502221 - Flags: review?(bzbarsky)
Posted patch WIP PatchSplinter Review
So, this patch should correspond to what Olli suggests in comment 5 but it's crashing :(
Whatever the reason of this crash is, I guess we should stay with Olli's patch for Firefox 4 given that this is fixing the issue and this patch is changing nsIDocShell uuid.
Assigning the bug to Olli given that he has a working and simple patch.
Assignee: mounir.lamouri → Olli.Pettay
Comment on attachment 502221 [details] [diff] [review]
Don't load if the owner document isn't active

This looks fine to me, but the comment needs changing a bit.  r=me with that.
Attachment #502221 - Flags: review?(bzbarsky) → review+
Oh, and can you add a test?
Assignee

Comment 11

9 years ago
Posted patch +commentSplinter Review
Attachment #502221 - Attachment is obsolete: true
Assignee

Comment 12

9 years ago
I hope I can use Jesse's test as a chashtest
Assignee

Comment 13

9 years ago
crashtest
Assignee

Comment 14

9 years ago
Posted patch +testSplinter Review
Assignee

Comment 15

9 years ago
http://hg.mozilla.org/mozilla-central/rev/cee083078957
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.