Closed Bug 464114 Opened 11 years ago Closed 11 years ago

nsAutoCompleteController should participate in cycle collection

Categories

(Firefox :: Address Bar, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 3.1b2

People

(Reporter: peterv, Assigned: peterv)

References

(Blocks 1 open bug)

Details

(Keywords: memory-leak, verified1.9.1)

Attachments

(1 file)

Attached patch v1Splinter Review
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+
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
Attachment #347381 - Flags: approval1.9.1b2?
(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.
Status: ASSIGNED → RESOLVED
Closed: 11 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
Depends on: 456414
Depends on: 466284
Depends on: 469523
No longer depends on: 456414
Keywords: verified1.9.1
Keywords: fixed1.9.1
You need to log in before you can comment on or make changes to this bug.