Bug 139613 - Get rid of the DONT_FINALIZE_ON_MAIN_THREAD #define
Summary: Get rid of the DONT_FINALIZE_ON_MAIN_THREAD #define
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-12-12 18:28 PST by Anders Carlsson
Modified: 2014-12-15 15:52 PST (History)
3 users (show)

See Also:

Attachments
Patch (10.66 KB, patch)
2014-12-12 18:32 PST, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (11.98 KB, patch)
2014-12-15 15:30 PST, Anders Carlsson 		sam: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2014-12-12 18:28:19 PST 
Get rid of the DONT_FINALIZE_ON_MAIN_THREAD #define
Comment 1 Anders Carlsson 2014-12-12 18:32:49 PST 
Created attachment 243240 [details]
Patch
Comment 2 Darin Adler 2014-12-15 09:07:28 PST 
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.
Comment 3 Anders Carlsson 2014-12-15 09:16:47 PST 
(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.
Comment 4 Anders Carlsson 2014-12-15 15:30:23 PST 
Created attachment 243318 [details]
Patch
Comment 5 Anders Carlsson 2014-12-15 15:52:58 PST 
Committed r177319: <http://trac.webkit.org/changeset/177319>