Bug 98543

Summary: Add a basic NetworkProcess.app to the WebKit2 build
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebKit2Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, ap, donggwan.kim, mjs, sam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 98537    
Attachments:
Description Flags
Patch v1 sam: review+

Description Brady Eidson 2012-10-05 11:44:43 PDT
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.
Comment 1 Maciej Stachowiak 2012-10-06 15:13:28 PDT
You could just call it NetProcess to match the brevity of WebProcess and UIProcess (but I guess PluginProcess is similarly long).
Comment 2 Sam Weinig 2012-10-07 17:19:09 PDT
(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.
Comment 3 Brady Eidson 2012-10-08 08:41:02 PDT
(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!!!
Comment 4 Brady Eidson 2012-10-12 09:47:26 PDT
Created attachment 168440 [details]
Patch v1
Comment 5 WebKit Review Bot 2012-10-12 09:50:07 PDT
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.
Comment 6 Sam Weinig 2012-10-12 09:53:42 PDT
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.
Comment 7 Brady Eidson 2012-10-12 09:55:46 PDT
(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.
Comment 8 Brady Eidson 2012-10-12 10:01:04 PDT
(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.
Comment 9 Brady Eidson 2012-10-12 10:04:40 PDT
Just for a sanity check will wait for qt-wk2 EWS to finish
Comment 10 Brady Eidson 2012-10-12 10:13:17 PDT
http://trac.webkit.org/changeset/131196