1456261 - Support cycle collection of WebIDL dictionaries

Status: RESOLVED FIXED

(Core :: DOM: Bindings (WebIDL), enhancement)

Description

[Boris Zbarsky [:bzbarsky]]

This came up in bug 1446961.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Add cycle collection bits for WebIDL dictionaries

MozReview-Commit-ID: 6epU3m0x4B3

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8970344 [details] [diff] [review]
Add cycle collection bits for WebIDL dictionaries

>+# Failure is when we have a type for which we can't actually tell
>+# whether it needs cycle collection or not.  In that case, if
>+# failureIsFatal we will error out; otherwise we will return false so
>+# we don't generate possibly-not-needed cycle collection machinery.
>+def idlTypeNeedsCycleCollection(type, failIfUnknown=True):
The comment talks about failureIsFatal, but the argument name I see is failIfUnknown.


>+template<typename T>
>+inline void
>+ImplCycleCollectionUnlink(OwningNonNull<T>& aField)
>+{
>+  // We can't be null, so nothing to do here....  Yes, this is a leak hazard.
>+}
I don't understand the comment here. If we are non-null, we definitely should unlink, no?
Why can't we forget() the field or something? Or is the idea that once initialized, we wouldn't  want the
field to ever become null. Well, forget() may do that anyway, right?

r- because of OwningNonNull case.  Please explain.

Comment 3

[Boris Zbarsky [:bzbarsky]]

> The comment talks about failureIsFatal

Sorry.  I renamed it but failed to completely update the comment.  :(

> Or is the idea that once initialized, we wouldn't  want the field to ever become null.
> Well, forget() may do that anyway, right?

That was the idea, yes.  I had forgotten that forget() exists.  I should just use it here.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Actually, I thought of a better approach than the failIfUnknown thing.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Add cycle collection bits for WebIDL dictionaries

This should be much saner and clearer.  Thank you for pushing back on the other thing!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f0c781e4886
Add cycle collection bits for WebIDL dictionaries.  r=smaug

Comment 7

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/0f0c781e4886