[webkit-reviews] review granted: [Bug 131634] Objective-C API external object graphs don't handle generational collection properly : [Attachment 229385] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 131634: Objective-C API external object graphs don't handle generational
collection properly
https://bugs.webkit.org/show_bug.cgi?id=131634

Attachment 229385: Patch
https://bugs.webkit.org/attachment.cgi?id=229385&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=229385&action=review


r=me

> Source/JavaScriptCore/API/JSVirtualMachine.mm:162
> +    [m_externalRememberedSet setObject:[NSNumber numberWithBool:true]
forKey:object];

You might be able to use NSSet here with something similar to weakIDOptions.

> Source/JavaScriptCore/API/JSVirtualMachine.mm:293
> +	   JSVirtualMachine *virtualMachine = [JSVMWrapperCache
wrapperForJSContextGroupRef:toRef(&vm)];
> +	   if (!virtualMachine)
> +	       return;

Is it possible that the JSVirtualMachine will go away, by virtue of a zero
retain count? If so, we'll forget all the external object graph stuff. Probably
worth a follow-up bug.