bug 523894: wstrings are bad news in OPT builds due to chromium/Mozilla -fshort-wchar mismatch

This commit is contained in:
Chris Jones 2009-11-05 18:24:27 -06:00
parent 3d8a76e521
commit 721479dcd3
7 changed files with 103 additions and 53 deletions

View File

@ -61,8 +61,8 @@ PluginProcessParent::~PluginProcessParent()
bool bool
PluginProcessParent::Launch() PluginProcessParent::Launch()
{ {
std::vector<std::wstring> args; std::vector<std::string> args;
args.push_back(UTF8ToWide(mPluginFilePath)); args.push_back(mPluginFilePath);
return SyncLaunch(args); return SyncLaunch(args);
} }

View File

@ -66,15 +66,27 @@ PluginThreadChild::Init()
{ {
GeckoThread::Init(); GeckoThread::Init();
// FIXME/cjones: set up channel stuff, etc. std::string pluginFilename;
#if defined(OS_POSIX)
// NB: need to be very careful in ensuring that the first arg
// (after the binary name) here is indeed the plugin module path.
// Keep in sync with dom/plugins/PluginModuleParent.
std::vector<std::string> values = CommandLine::ForCurrentProcess()->argv();
NS_ABORT_IF_FALSE(values.size() >= 2, "not enough args");
pluginFilename = values[1];
#elif defined(OS_WIN)
std::vector<std::wstring> values = std::vector<std::wstring> values =
CommandLine::ForCurrentProcess()->GetLooseValues(); CommandLine::ForCurrentProcess()->GetLooseValues();
NS_ABORT_IF_FALSE(values.size() >= 1, "not enough loose args");
// XXX need to handle plugin args! pluginFilename = WideToUTF8(values[0]);
DCHECK(values.size() >= 1);
std::string pluginFilename = WideToUTF8(values[0]); #else
# error Sorry
#endif
// FIXME owner_loop() is bad here // FIXME owner_loop() is bad here
mPlugin.Init(pluginFilename, mPlugin.Init(pluginFilename,

View File

@ -250,8 +250,10 @@ AsyncChannel::OnChannelError()
// Must exit the IO loop, which will then join with the UI loop. // Must exit the IO loop, which will then join with the UI loop.
MessageLoop::current()->Quit(); MessageLoop::current()->Quit();
#else #else
// Go ahead and abort here. // FIXME need to devote some thought to the most
NS_DebugBreak(NS_DEBUG_ABORT, nsnull, nsnull, nsnull, 0); // effective/least easily overrideable, yet quiet, way to
// exit. abort() is a little loud
_exit(0);
#endif #endif
} }
} }

View File

@ -67,12 +67,11 @@ GeckoChildProcessHost::GeckoChildProcessHost(GeckoProcessType aProcessType,
} }
bool bool
GeckoChildProcessHost::SyncLaunch(std::vector<std::wstring> aExtraOpts) GeckoChildProcessHost::SyncLaunch(std::vector<std::string> aExtraOpts)
{ {
MessageLoop* loop = MessageLoop::current();
MessageLoop* ioLoop = MessageLoop* ioLoop =
BrowserProcessSubThread::GetMessageLoop(BrowserProcessSubThread::IO); BrowserProcessSubThread::GetMessageLoop(BrowserProcessSubThread::IO);
NS_ASSERTION(loop != ioLoop, "sync launch from the IO thread NYI"); NS_ASSERTION(MessageLoop::current() != ioLoop, "sync launch from the IO thread NYI");
ioLoop->PostTask(FROM_HERE, ioLoop->PostTask(FROM_HERE,
NewRunnableMethod(this, NewRunnableMethod(this,
@ -90,7 +89,7 @@ GeckoChildProcessHost::SyncLaunch(std::vector<std::wstring> aExtraOpts)
} }
bool bool
GeckoChildProcessHost::AsyncLaunch(std::vector<std::wstring> aExtraOpts) GeckoChildProcessHost::AsyncLaunch(std::vector<std::string> aExtraOpts)
{ {
// FIXME/cjones: make this work from non-IO threads, too // FIXME/cjones: make this work from non-IO threads, too
@ -98,45 +97,74 @@ GeckoChildProcessHost::AsyncLaunch(std::vector<std::wstring> aExtraOpts)
return false; return false;
} }
FilePath exePath = base::ProcessHandle process;
FilePath::FromWStringHack(CommandLine::ForCurrentProcess()->program());
exePath = exePath.DirName();
exePath = exePath.AppendASCII(MOZ_CHILD_PROCESS_NAME);
// remap the IPC socket fd to a well-known int, as the OS does for
// STDOUT_FILENO, for example
#if defined(OS_POSIX)
int srcChannelFd, dstChannelFd;
channel().GetClientFileDescriptorMapping(&srcChannelFd, &dstChannelFd);
mFileMap.push_back(std::pair<int,int>(srcChannelFd, dstChannelFd));
#endif
CommandLine cmdLine(exePath.ToWStringHack());
cmdLine.AppendSwitchWithValue(switches::kProcessChannelID, channel_id());
for (std::vector<std::wstring>::iterator it = aExtraOpts.begin();
it != aExtraOpts.end();
++it) {
cmdLine.AppendLooseValue((*it).c_str());
}
// send the child the PID so that it can open a ProcessHandle back to us. // send the child the PID so that it can open a ProcessHandle back to us.
// probably don't want to do this in the long run // probably don't want to do this in the long run
char pidstring[32]; char pidstring[32];
PR_snprintf(pidstring, sizeof(pidstring) - 1, PR_snprintf(pidstring, sizeof(pidstring) - 1,
"%ld", base::Process::Current().pid()); "%ld", base::Process::Current().pid());
const char* const childProcessType =
XRE_ChildProcessTypeToString(mProcessType);
//--------------------------------------------------
#if defined(OS_POSIX)
// For POSIX, we have to be extremely anal about *not* using
// std::wstring in code compiled with Mozilla's -fshort-wchar
// configuration, because chromium is compiled with -fno-short-wchar
// and passing wstrings from one config to the other is unsafe. So
// we split the logic here.
FilePath exePath = FilePath(CommandLine::ForCurrentProcess()->argv()[0]);
exePath = exePath.DirName();
exePath = exePath.AppendASCII(MOZ_CHILD_PROCESS_NAME);
// remap the IPC socket fd to a well-known int, as the OS does for
// STDOUT_FILENO, for example
int srcChannelFd, dstChannelFd;
channel().GetClientFileDescriptorMapping(&srcChannelFd, &dstChannelFd);
mFileMap.push_back(std::pair<int,int>(srcChannelFd, dstChannelFd));
// no need for kProcessChannelID, the child process inherits the
// other end of the socketpair() from us
std::vector<std::string> childArgv;
childArgv.push_back(exePath.value());
childArgv.insert(childArgv.end(), aExtraOpts.begin(), aExtraOpts.end());
childArgv.push_back(pidstring);
childArgv.push_back(childProcessType);
base::LaunchApp(childArgv, mFileMap, false, &process);
//--------------------------------------------------
#elif defined(OS_WIN)
FilePath exePath =
FilePath::FromWStringHack(CommandLine::ForCurrentProcess()->program());
exePath = exePath.DirName();
exePath = exePath.AppendASCII(MOZ_CHILD_PROCESS_NAME);
CommandLine cmdLine(exePath.ToWStringHack());
cmdLine.AppendSwitchWithValue(switches::kProcessChannelID, channel_id());
for (std::vector<std::string>::iterator it = aExtraOpts.begin();
it != aExtraOpts.end();
++it) {
cmdLine.AppendLooseValue(UTF8ToWide(*it));
}
cmdLine.AppendLooseValue(UTF8ToWide(pidstring)); cmdLine.AppendLooseValue(UTF8ToWide(pidstring));
cmdLine.AppendLooseValue(UTF8ToWide(childProcessType));
cmdLine.AppendLooseValue(UTF8ToWide(XRE_ChildProcessTypeToString(mProcessType)));
base::ProcessHandle process;
#if defined(OS_WIN)
base::LaunchApp(cmdLine, false, false, &process); base::LaunchApp(cmdLine, false, false, &process);
#elif defined(OS_POSIX)
base::LaunchApp(cmdLine.argv(), mFileMap, false, &process);
#else #else
#error Bad! # error Sorry
#endif #endif
if (!process) { if (!process) {

View File

@ -61,8 +61,8 @@ public:
GeckoChildProcessHost(GeckoProcessType aProcessType=GeckoProcessType_Default, GeckoChildProcessHost(GeckoProcessType aProcessType=GeckoProcessType_Default,
base::WaitableEventWatcher::Delegate* aDelegate=nsnull); base::WaitableEventWatcher::Delegate* aDelegate=nsnull);
bool SyncLaunch(std::vector<std::wstring> aExtraOpts=std::vector<std::wstring>()); bool SyncLaunch(std::vector<std::string> aExtraOpts=std::vector<std::string>());
bool AsyncLaunch(std::vector<std::wstring> aExtraOpts=std::vector<std::wstring>()); bool AsyncLaunch(std::vector<std::string> aExtraOpts=std::vector<std::string>());
virtual void OnChannelConnected(int32 peer_pid); virtual void OnChannelConnected(int32 peer_pid);
virtual void OnMessageReceived(const IPC::Message& aMsg); virtual void OnMessageReceived(const IPC::Message& aMsg);

View File

@ -64,7 +64,14 @@ ScopedXREEmbed::~ScopedXREEmbed()
void void
ScopedXREEmbed::Start() ScopedXREEmbed::Start()
{ {
std::string path = WideToUTF8(CommandLine::ForCurrentProcess()->program()); std::string path;
#if defined(OS_WIN)
path = WideToUTF8(CommandLine::ForCurrentProcess()->program());
#elif defined(OS_POSIX)
path = CommandLine::ForCurrentProcess()->argv()[0];
#else
# error Sorry
#endif
nsCOMPtr<nsILocalFile> localFile; nsCOMPtr<nsILocalFile> localFile;
nsresult rv = XRE_GetBinaryPath(path.c_str(), getter_AddRefs(localFile)); nsresult rv = XRE_GetBinaryPath(path.c_str(), getter_AddRefs(localFile));

View File

@ -35,15 +35,17 @@ const char* const
IPDLUnitTestName() IPDLUnitTestName()
{ {
if (!gIPDLUnitTestName) { if (!gIPDLUnitTestName) {
#if defined(OS_WIN)
std::vector<std::wstring> args = std::vector<std::wstring> args =
CommandLine::ForCurrentProcess()->GetLooseValues(); CommandLine::ForCurrentProcess()->GetLooseValues();
gIPDLUnitTestName = strdup(WideToUTF8(args[ gIPDLUnitTestName = strdup(WideToUTF8(args[0]).c_str());
#ifndef OS_WIN #elif defined(OS_POSIX)
args.size()-1 std::vector<std::string> argv =
CommandLine::ForCurrentProcess()->argv();
gIPDLUnitTestName = strdup(argv[1].c_str());
#else #else
args.size()-2 # error Sorry
#endif #endif
]).c_str());
} }
return gIPDLUnitTestName; return gIPDLUnitTestName;
} }
@ -129,9 +131,8 @@ IPDLUnitTestMain(void* aData)
} }
gIPDLUnitTestName = testString; gIPDLUnitTestName = testString;
std::wstring testWString = UTF8ToWide(testString); std::vector<std::string> testCaseArgs;
std::vector<std::wstring> testCaseArgs; testCaseArgs.push_back(testString);
testCaseArgs.push_back(testWString);
IPDLUnitTestSubprocess* subprocess = new IPDLUnitTestSubprocess(); IPDLUnitTestSubprocess* subprocess = new IPDLUnitTestSubprocess();
if (!subprocess->SyncLaunch(testCaseArgs)) if (!subprocess->SyncLaunch(testCaseArgs))