Bug 54257 – Make DefaultGCActivityCallback for PLATFORM(CF) Easier to Subclass

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Joseph Pecoraro]

Allow the CFRunLoop to be specified in a constructor.

[Joseph Pecoraro]

Created attachment 82071 [details]
[PATCH] Proposed Solution

I'm open to suggestions for a better name for "commonConstructor".
Also I'm fine with making that private.

[Geoffrey Garen]

Comment on attachment 82071 [details]
[PATCH] Proposed Solution

View in context: https://bugs.webkit.org/attachment.cgi?id=82071&action=review

r=me

> Source/JavaScriptCore/ChangeLog:8
> +        A subclass may want to specify the CFRunLoop the Garbage Collection. It

Typo, I think.

> Source/JavaScriptCore/runtime/GCActivityCallbackCF.cpp:86
> +    memset(&d->context, '\0', sizeof(CFRunLoopTimerContext));

'\0' is a strange way to say '0'. I know you didn't write this code initially, but you might as well change it to '0'.

[Joseph Pecoraro]

Landed in r78291:
http://trac.webkit.org/changeset/78291

[Joseph Pecoraro]

> > Source/JavaScriptCore/ChangeLog:8
> > +        A subclass may want to specify the CFRunLoop the Garbage Collection. It
> 
> Typo, I think.

Yep, I cleaned up my ChangeLog mistakes.

> > Source/JavaScriptCore/runtime/GCActivityCallbackCF.cpp:86
> > +    memset(&d->context, '\0', sizeof(CFRunLoopTimerContext));
> 
> '\0' is a strange way to say '0'. I know you didn't write this code initially, but you might as well change it to '0'.

Done. Thanks!

[Pratik Solanki]

(In reply to comment #4)
> > '\0' is a strange way to say '0'. I know you didn't write this code initially, but you might as well change it to '0'.
> 
> Done. Thanks!

I think Geoff meant to say pass 0, not '0'. Passing '0' to memset will initialize the memory to ASCII character 0.

[Joseph Pecoraro]

(In reply to comment #5)
> (In reply to comment #4)
> > > '\0' is a strange way to say '0'. I know you didn't write this code initially, but you might as well change it to '0'.
> > 
> > Done. Thanks!
> 
> I think Geoff meant to say pass 0, not '0'. Passing '0' to memset will initialize the memory to ASCII character 0.

Yes, entirely my fault. I took that literally. I landed a follow-up fix.
Unreviewed since initially this was a review comment.
http://trac.webkit.org/changeset/78292

Thanks!