nsAutoCompleteController should participate in cycle collection

VERIFIED FIXED in Firefox 3.1b2

Status

()

Firefox
Address Bar
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: peterv, Assigned: peterv)

Tracking

(Depends on: 1 bug, Blocks: 1 bug, {memory-leak, verified1.9.1})

Trunk
Firefox 3.1b2
memory-leak, verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3.5 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

10 years ago
Created attachment 347381 [details] [diff] [review]
v1

This fixes some leaks in tests and some new leaks that show up with the patch for bug 457022. I tried to be careful to still hold a strong reference to an element in the mResults/mSearches arrays when I wasn't sure if it could be removed from the array by subsequent code. Also used SafeObjectAt in nsAutoCompleteController::StartSearch because I wasn't sure if the index could be out-of-bound.
Attachment #347381 - Flags: review?(gavin.sharp)
Flags: blocking-firefox3.1?
Flags: blocking-firefox3.1? → blocking-firefox3.1+
Target Milestone: --- → Firefox 3.1
Comment on attachment 347381 [details] [diff] [review]
v1

>diff --git a/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp b/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp

>+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsAutoCompleteController)
>+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mInput)
>+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMARRAY(mSearches)
>+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMARRAY(mResults)
>+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END

Need mTree here too per IRC.
Attachment #347381 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 2

10 years ago
Comment on attachment 347381 [details] [diff] [review]
v1

Leak fix. Patch in bug 457022 triggers the leak, but can probably be triggered fairly easily without it too.
Attachment #347381 - Flags: approval1.9.1b2?
Marking this a beta2 blocker as this blocks beta2 blocker bug 457022.
Target Milestone: Firefox 3.1 → Firefox 3.1b2

Updated

10 years ago
Attachment #347381 - Flags: approval1.9.1b2?
(Assignee)

Comment 4

10 years ago
(In reply to comment #1)
> Need mTree here too per IRC.

I was mistaken, we don't currently traverse box objects, so decided to add that in bug 461640.
(Assignee)

Updated

10 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
I confirm my previous bug 449240 comment 13:

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081113 Minefield/3.1b2pre] (home, optim default) (W2Ksp4)

this very patch fixed 25 |make check| tests (leak):
(4 + 14) "autocomplete" + 7 <test_places/unit/*>.

V.Fixed
Status: RESOLVED → VERIFIED
Keywords: mlk
Blocks: 408905
Depends on: 456414
(Assignee)

Updated

10 years ago
Depends on: 466284
Keywords: fixed1.9.1
Depends on: 469523
No longer depends on: 456414

Updated

9 years ago
Keywords: verified1.9.1
Keywords: fixed1.9.1
You need to log in before you can comment on or make changes to this bug.