Bug 139613 – Get rid of the DONT_FINALIZE_ON_MAIN_THREAD #define

• Component: New Bugs
• Status: RESOLVED FIXED

[Anders Carlsson]

Get rid of the DONT_FINALIZE_ON_MAIN_THREAD #define

[Anders Carlsson]

Created attachment 243240 [details]
Patch

[Darin Adler]

Comment on attachment 243240 [details]
Patch

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

> Source/WebCore/platform/mac/WebCoreObjCExtras.h:36
> +#if defined(OBJC_NO_GC) && OBJC_NO_GC

This checks both the defined’ness and value of OBJC_NO_GC.

> Source/WebCore/platform/mac/WebCoreObjCExtras.mm:39
> +#ifndef OBJC_NO_GC

This checks only the defined’ness of OBJC_NO_GC.

These two can’t both be correct.

> Source/WebCore/platform/mac/WebCoreObjCExtras.mm:62
> +        Method method = class_getInstanceMethod(cls, @selector(dealloc));
> +
> +        IMP imp = method_getImplementation(method);
> +        wtfCallIMP<void>(imp, object, @selector(dealloc));

We should remove the strangely arbitrary blank line.

[Anders Carlsson]

(In reply to comment #2)
> Comment on attachment 243240 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=243240&action=review
> 
> > Source/WebCore/platform/mac/WebCoreObjCExtras.h:36
> > +#if defined(OBJC_NO_GC) && OBJC_NO_GC
> 
> This checks both the defined’ness and value of OBJC_NO_GC.
> 
> > Source/WebCore/platform/mac/WebCoreObjCExtras.mm:39
> > +#ifndef OBJC_NO_GC
> 
> This checks only the defined’ness of OBJC_NO_GC.
> 
> These two can’t both be correct.

Here's what the API docs say:

 * OBJC_NO_GC 1: GC is not supported
 * OBJC_NO_GC undef: GC is supported

in the first case, we want to check that OBJC_NO_GC is define to 1. IN the second case, we want to check that it's not set.

I could make the second check more stringent by checking !defined(OBJC_NO_GC) || !OBJC_NO_GC, but I'm going to change things and make WebCoreObjCFinalizeOnMainThread be inlined unconditionally.

> 
> > Source/WebCore/platform/mac/WebCoreObjCExtras.mm:62
> > +        Method method = class_getInstanceMethod(cls, @selector(dealloc));
> > +
> > +        IMP imp = method_getImplementation(method);
> > +        wtfCallIMP<void>(imp, object, @selector(dealloc));
> 
> We should remove the strangely arbitrary blank line.

Agreed.

[Anders Carlsson]

Created attachment 243318 [details]
Patch

[Anders Carlsson]

Committed r177319: <http://trac.webkit.org/changeset/177319>