[webkit-reviews] review granted: [Bug 98543] Add a basic NetworkProcess.app to the WebKit2 build : [Attachment 168440] Patch v1

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Brady Eidson <beidson at apple.com>'s
request for review:
Bug 98543: Add a basic NetworkProcess.app to the WebKit2 build
https://bugs.webkit.org/show_bug.cgi?id=98543

Attachment 168440: Patch v1
https://bugs.webkit.org/attachment.cgi?id=168440&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=168440&action=review


> Source/WebKit2/NetworkProcess/NetworkProcess.h:33
> +#include <wtf/text/WTFString.h>

This should not be needed.

> Source/WebKit2/UIProcess/Launcher/mac/ProcessLauncherMac.mm:384
> +    switch(launchOptions.processType) {
> +    case ProcessLauncher::WebProcess:
> +	   processPath = [webKit2Bundle
pathForAuxiliaryExecutable:@"WebProcess.app"];
> +	   break;
> +    case ProcessLauncher::PluginProcess:
> +	   processPath = [webKit2Bundle
pathForAuxiliaryExecutable:@"PluginProcess.app"];
> +	   break;
> +#if ENABLE(NETWORK_PROCESS)
> +    case ProcessLauncher::NetworkProcess:
> +	   processPath = [webKit2Bundle
pathForAuxiliaryExecutable:@"NetworkProcess.app"];
> +	   break;
> +#endif
> +    default:
> +	   ASSERT_NOT_REACHED();

The switch needs a space after it.  Also, I would rather not have the default,
but rather handle each case.

> Source/WebKit2/UIProcess/Network/NetworkProcessProxy.cpp:49
> +    launchOptions.architecture =
ProcessLauncher::LaunchOptions::MatchCurrentArchitecture;
> +    launchOptions.executableHeap = false;
> +    launchOptions.useXPC = false;

I believe these are mac specific and should go in NetworkProcessProxyMac.mm
file.

> Source/WebKit2/UIProcess/Network/NetworkProcessProxy.h:38
> +class NetworkProcessManager;

Does this class exist?

> Source/WebKit2/UIProcess/WebContext.h:35
> +#if ENABLE(NETWORK_PROCESS)
> +#include "NetworkProcessProxy.h"
> +#endif

Please move this to the bottom.