Bug 98543 – Add a basic NetworkProcess.app to the WebKit2 build

• Component: WebKit2
• Status: RESOLVED FIXED

[Brady Eidson]

Add a basic NetworkProcess.app to the WebKit2 build

I intend to make a few changes to cross platform WK2 code that will show up in all ports.

I'll also add an actual new NetworkProcess.app to the Mac build only, since I'm certainly not an expert in getting a new process running on any other port.

Using the NetworkProcess will be completely optional and other ports that wish to adopt it can bring it up in the build on their own schedule.

[Maciej Stachowiak]

You could just call it NetProcess to match the brevity of WebProcess and UIProcess (but I guess PluginProcess is similarly long).

[Sam Weinig]

(In reply to comment #1)
> You could just call it NetProcess to match the brevity of WebProcess and UIProcess (but I guess PluginProcess is similarly long).

Ick. I prefer Network if we are voting.

[Brady Eidson]

(In reply to comment #2)
> (In reply to comment #1)
> > You could just call it NetProcess to match the brevity of WebProcess and UIProcess (but I guess PluginProcess is similarly long).
> 
> Ick. I prefer Network if we are voting.

Me too!!!

[Brady Eidson]

Created attachment 168440 [details]
Patch v1

[WebKit Review Bot]

Attachment 168440 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1
Source/WebKit2/NetworkProcess/NetworkProcess.h:36:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Sam Weinig]

Comment on attachment 168440 [details]
Patch v1

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.

[Brady Eidson]

(In reply to comment #6)
> (From update of attachment 168440 [details])
> 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.

True, not yet.  Will fix.

> > 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.

It does handle each case.  Will fix.

> > 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?

Not in this patch, but in later patches in my tree.  Trying to split this up...  will remove.

> 
> > Source/WebKit2/UIProcess/WebContext.h:35
> > +#if ENABLE(NETWORK_PROCESS)
> > +#include "NetworkProcessProxy.h"
> > +#endif
> 
> Please move this to the bottom.

[Brady Eidson]

(In reply to comment #6)

> > 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.

They are Mac specific, but are in the shared constructor.  The other processes follow this pattern with PLATFORM(MAC) ifdefs.  I implemented that same pattern here.

[Brady Eidson]

Just for a sanity check will wait for qt-wk2 EWS to finish

[Brady Eidson]

http://trac.webkit.org/changeset/131196