Closed Bug 855804 Opened 7 years ago Closed 7 years ago

Add cycle collector traversal helpers for nsRefPtrHashtable

Categories

(Core :: XPCOM, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: ted, Assigned: khuey)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
This is Kyle's patch after I fixed a few things and he told me how to fix some other things.
Attachment #730839 - Flags: review?(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.
Attachment #730839 - Flags: review?(bzbarsky) → review+
You should be able to just aField.Clear() in Unlink(), instead of this enumeration rigamarole.
(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.
Attached patch PatchSplinter Review
Now supporting nsRefPtrHashtable and nsInterfaceHashtable, with some converted uses.
Attachment #730839 - Attachment is obsolete: true
Attachment #732509 - Flags: review?(bzbarsky)
Comment on attachment 732509 [details] [diff] [review]
Patch

r=me
Attachment #732509 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/ef873e1fb7e9
https://hg.mozilla.org/mozilla-central/rev/c4cec4613cac
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.