Summary: | Add a basic NetworkProcess.app to the WebKit2 build | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brady Eidson <beidson> | ||||
Component: | WebKit2 | Assignee: | 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
Brady Eidson
2012-10-05 11:44:43 PDT
You could just call it NetProcess to match the brevity of WebProcess and UIProcess (but I guess PluginProcess is similarly long). (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. (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!!! Created attachment 168440 [details]
Patch v1
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 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. (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. (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. Just for a sanity check will wait for qt-wk2 EWS to finish |