855804 - Add cycle collector traversal helpers for nsRefPtrHashtable

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[(not currently active) Ted Mielczarek]

My gamepad implementation has a nsRefPtrHashTable of nsDOMGamepads on nsGlobalWindow. The WebIDL patches in bug 851542 introduce a cycle because nsDOMGamepad gets a reference to its parent. Implementing cycle collection is a pain because we don't have traversal helpers for nsRefPtrHashTable.

khuey has most of a patch for this, it just needs to be cleaned up a little.

Comment 1

[(not currently active) Ted Mielczarek]

Attachment: Add cycle collector traversal helpers for nsRefPtrHashtable

This is Kyle's patch after I fixed a few things and he told me how to fix some other things.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 730839 [details] [diff] [review]
Add cycle collector traversal helpers for nsRefPtrHashtable

>+struct nsRefPtrHashtableCCTraversalData

MOZ_STACK_CLASS

>+  nsRefPtrHashtableCCTraversalData(nsCycleCollectionTraversalCallback& aCallback,
>+                                          const char* aName,
>+                                          uint32_t aFlags) : mCallback(aCallback),

Please fix the indentation and move the mCallback() bit to the next line

>+    mFlags(aFlags) {}

{} to next line.

But also, mFlags seems to be unused.  Just kill it?  Or pass it to CycleCollectionNoteChild?  Presumably the latter....

>+  aField.EnumerateRead(ImplCycleCollectionTraverse_EnumFunc<typename K::KeyType,T*>, reinterpret_cast<void*>(&userData));

Newline after comma between the two EnumerateRead arguments, and you shouldn't need the reinterpret_cast.

r=me with the above nits.

Comment 3

[Andrew McCreight (out of office until 8/21) [:mccr8]]

You should be able to just aField.Clear() in Unlink(), instead of this enumeration rigamarole.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Andrew McCreight [:mccr8] from comment #3)
> You should be able to just aField.Clear() in Unlink(), instead of this
> enumeration rigamarole.

Indeed.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Now supporting nsRefPtrHashtable and nsInterfaceHashtable, with some converted uses.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 732509 [details] [diff] [review]
Patch

r=me

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ef873e1fb7e9
https://hg.mozilla.org/mozilla-central/rev/c4cec4613cac