Bug 107216 – Don't initialize AppKit for processes that don't use it

• Component: WebKit2
• Status: RESOLVED FIXED

[Sam Weinig]

Don't initialize AppKit for processes that don't use it

[Sam Weinig]

Created attachment 183353 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 183353 [details]
Patch

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

r=me, but please double-check [[NSApplication sharedApplication] _installAutoreleasePoolsOnCurrentThreadIfNecessary] usage. It looks very suspicious to me.

> Source/WebKit2/PluginProcess/mac/PluginProcessMainMac.mm:65
> +        [NSApplication sharedApplication];
> +
> +        // Installs autorelease pools on the current CFRunLoop which prevents memory from accumulating between user events.
> +        // FIXME: Remove when <rdar://problem/8929426> is fixed.
> +        [[NSApplication sharedApplication] _installAutoreleasePoolsOnCurrentThreadIfNecessary];

This code is confusing.

1. We call [NSApplication sharedApplication] twice, without any clear reason. There was a previously a FIXME about WebCore::RunLoop limitations that sounded obsolete (NSApp is equivalent to [NSApplication sharedApplication], and RunLoop uses NSApp).

2. PluginProcess doesn't use a plain CFRunLoop, so the comment is not immediately helpful at best, or misleading otherwise. Perhaps we actually need this call in processes that do not use NSApplication run loop, not in those that do? But do we even have a "current CFRunLoop" here?

> Source/WebKit2/PluginProcess/mac/PluginProcessMainMac.mm:71
> +            // We are never going to get to the actual initialization, so initialize WebKit2 now.

What is "actual initialization"?

> Source/WebKit2/WebProcess/mac/WebProcessMainMac.mm:42
>  #import <wtf/text/WTFString.h>
> +#import <WebCore/RunLoop.h>

Alphabetic ordering.

> Source/WebKit2/WebProcess/mac/WebProcessServiceEntryPoints.mm:85
> +    WebCore::RunLoop::setUseApplicationRunLoopOnMainRunLoop();

Can we be "using namespace WebCore" in this file?

[Alexey Proskuryakov]

Also, can we ASSERT in RunLoop::setUseApplicationRunLoopOnMainRunLoop() now?

[Sam Weinig]

Committed r140230: <http://trac.webkit.org/changeset/140230>