Bug 107216 - Don't initialize AppKit for processes that don't use it
Summary: Don't initialize AppKit for processes that don't use it
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-17 19:29 PST by Sam Weinig
Modified: 2013-01-18 18:55 PST (History)
1 user (show)

See Also:


Attachments
Patch (11.96 KB, patch)
2013-01-17 19:35 PST, Sam Weinig
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2013-01-17 19:29:52 PST
Don't initialize AppKit for processes that don't use it
Comment 1 Sam Weinig 2013-01-17 19:35:00 PST
Created attachment 183353 [details]
Patch
Comment 2 Alexey Proskuryakov 2013-01-18 00:24:46 PST
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?
Comment 3 Alexey Proskuryakov 2013-01-18 00:26:28 PST
Also, can we ASSERT in RunLoop::setUseApplicationRunLoopOnMainRunLoop() now?
Comment 4 Sam Weinig 2013-01-18 18:55:28 PST
Committed r140230: <http://trac.webkit.org/changeset/140230>