464114 - nsAutoCompleteController should participate in cycle collection

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[Peter Van der Beken [:peterv]]

Attachment: 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.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

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.

Comment 2

[Peter Van der Beken [:peterv]]

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.

Comment 3

[Johnny Stenback (:jst)]

Marking this a beta2 blocker as this blocks beta2 blocker bug 457022.

Comment 4

[Peter Van der Beken [:peterv]]

(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.

Comment 5

[Serge Gautherie (:sgautherie)]

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