From 69604af50f8d3f63a7e208a44d092fbb7885af22 Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Mon, 20 Jul 2009 13:14:41 -0500 Subject: [PATCH] add SyncLaunch() method to GeckoChildProcess class. remove much newly obseleted code. --- dom/ipc/Makefile.in | 1 - dom/ipc/TabParent.cpp | 26 +----------- dom/ipc/TabParent.h | 12 +++--- dom/ipc/TabProcessParent.cpp | 23 ----------- dom/ipc/TabProcessParent.h | 38 ------------------ dom/plugins/NPAPIPluginParent.cpp | 36 +---------------- dom/plugins/NPAPIPluginParent.h | 5 --- dom/plugins/PluginProcessParent.cpp | 2 +- ipc/glue/GeckoChildProcessHost.cpp | 61 ++++++++++++++++++++++------- ipc/glue/GeckoChildProcessHost.h | 11 +++++- toolkit/xre/nsEmbedFunctions.cpp | 34 ++++------------ 11 files changed, 72 insertions(+), 177 deletions(-) delete mode 100644 dom/ipc/TabProcessParent.cpp delete mode 100644 dom/ipc/TabProcessParent.h diff --git a/dom/ipc/Makefile.in b/dom/ipc/Makefile.in index f64d23773a01..bef7402d2a54 100644 --- a/dom/ipc/Makefile.in +++ b/dom/ipc/Makefile.in @@ -49,7 +49,6 @@ EXPORT_LIBRARY = 1 CPPSRCS = \ TabParent.cpp \ - TabProcessParent.cpp \ TabChild.cpp \ TabThread.cpp \ $(NULL) diff --git a/dom/ipc/TabParent.cpp b/dom/ipc/TabParent.cpp index c8eca3c6f1a7..ecdc88d74b46 100644 --- a/dom/ipc/TabParent.cpp +++ b/dom/ipc/TabParent.cpp @@ -6,27 +6,13 @@ using mozilla::ipc::BrowserProcessSubThread; -template<> -struct RunnableMethodTraits -{ - static void RetainCallee(mozilla::tabs::TabParent* obj) { } - static void ReleaseCallee(mozilla::tabs::TabParent* obj) { } -}; - namespace mozilla { namespace tabs { TabParent::TabParent(MagicWindowHandle parentWidget) - : mSubprocess() - , mMonitor("mozilla.dom.ipc.TabParent") + : mSubprocess(GeckoChildProcess_Tab) { - { - MonitorAutoEnter mon(mMonitor); - BrowserProcessSubThread::GetMessageLoop(BrowserProcessSubThread::IO)-> - PostTask(FROM_HERE, NewRunnableMethod(this, &TabParent::LaunchSubprocess)); - mon.Wait(); - } - + mSubprocess.SyncLaunch(); Open(mSubprocess.GetChannel()); Sendinit(parentWidget); @@ -36,14 +22,6 @@ TabParent::~TabParent() { } -void -TabParent::LaunchSubprocess() -{ - MonitorAutoEnter mon(mMonitor); - mSubprocess.Launch(); - mon.Notify(); -} - void TabParent::LoadURL(nsIURI* aURI) { diff --git a/dom/ipc/TabParent.h b/dom/ipc/TabParent.h index 4aecd03b35b8..42797c42074b 100644 --- a/dom/ipc/TabParent.h +++ b/dom/ipc/TabParent.h @@ -7,9 +7,8 @@ #include "TabTypes.h" #include "IFrameEmbeddingProtocol.h" #include "IFrameEmbeddingProtocolParent.h" -#include "TabProcessParent.h" -#include "mozilla/Monitor.h" +#include "mozilla/ipc/GeckoChildProcessHost.h" class nsIURI; @@ -19,6 +18,9 @@ namespace tabs { class TabParent : private IFrameEmbeddingProtocolParent { +private: + typedef mozilla::ipc::GeckoChildProcessHost GeckoChildProcessHost; + public: TabParent(MagicWindowHandle parentWidget); virtual ~TabParent(); @@ -27,11 +29,7 @@ public: void Move(PRUint32 x, PRUint32 y, PRUint32 width, PRUint32 height); private: - void LaunchSubprocess(); - - TabProcessParent mSubprocess; - - mozilla::Monitor mMonitor; + GeckoChildProcessHost mSubprocess; }; } // namespace tabs diff --git a/dom/ipc/TabProcessParent.cpp b/dom/ipc/TabProcessParent.cpp deleted file mode 100644 index 85013072db0f..000000000000 --- a/dom/ipc/TabProcessParent.cpp +++ /dev/null @@ -1,23 +0,0 @@ -/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ -/* vim: sw=4 ts=4 et : */ - -#include "TabProcessParent.h" - -using mozilla::ipc::GeckoChildProcessHost; - -namespace mozilla { -namespace tabs { - - -TabProcessParent::TabProcessParent() : - GeckoChildProcessHost(GeckoChildProcess_Tab) -{ -} - -TabProcessParent::~TabProcessParent() -{ -} - - -} // namespace tabs -} // namespace mozilla diff --git a/dom/ipc/TabProcessParent.h b/dom/ipc/TabProcessParent.h deleted file mode 100644 index a4289c6958b2..000000000000 --- a/dom/ipc/TabProcessParent.h +++ /dev/null @@ -1,38 +0,0 @@ -/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ -/* vim: sw=4 ts=4 et : */ - -#ifndef mozilla_tabs_TabProcessParent_h -#define mozilla_tabs_TabProcessParent_h - -#include "mozilla/ipc/GeckoChildProcessHost.h" - -namespace mozilla { -namespace tabs { - -class TabProcessParent - : public mozilla::ipc::GeckoChildProcessHost -{ -public: - TabProcessParent(); - ~TabProcessParent(); - - IPC::Channel* GetChannel() { - return channelp(); - } - - virtual bool CanShutdown() { - return true; - } - - base::WaitableEvent* GetShutDownEvent() { - return GetProcessEvent(); - } - -private: - DISALLOW_EVIL_CONSTRUCTORS(TabProcessParent); -}; - -} -} - -#endif diff --git a/dom/plugins/NPAPIPluginParent.cpp b/dom/plugins/NPAPIPluginParent.cpp index 2b2311bfa30f..f004eff828ad 100644 --- a/dom/plugins/NPAPIPluginParent.cpp +++ b/dom/plugins/NPAPIPluginParent.cpp @@ -38,21 +38,6 @@ #include "mozilla/plugins/NPAPIPluginParent.h" -#include "base/task.h" - -#include "mozilla/ipc/GeckoThread.h" - -using mozilla::Monitor; -using mozilla::MonitorAutoEnter; -using mozilla::ipc::BrowserProcessSubThread; - -template<> -struct RunnableMethodTraits -{ - static void RetainCallee(mozilla::plugins::NPAPIPluginParent* obj) { } - static void ReleaseCallee(mozilla::plugins::NPAPIPluginParent* obj) { } -}; - namespace mozilla { namespace plugins { @@ -64,17 +49,7 @@ NPAPIPluginParent::LoadModule(const char* aFilePath, PRLibrary* aLibrary) // Block on the child process being launched and initialized. NPAPIPluginParent* parent = new NPAPIPluginParent(aFilePath); - - // launch the process synchronously - {MonitorAutoEnter mon(parent->mMonitor); - BrowserProcessSubThread::GetMessageLoop(BrowserProcessSubThread::IO) - ->PostTask( - FROM_HERE, - NewRunnableMethod(parent, - &NPAPIPluginParent::LaunchSubprocess)); - mon.Wait(); - } - + parent->mSubprocess.Launch(); parent->Open(parent->mSubprocess.GetChannel()); // FIXME/cjones: leaking NPAPIPluginParents ... @@ -85,7 +60,6 @@ NPAPIPluginParent::LoadModule(const char* aFilePath, PRLibrary* aLibrary) NPAPIPluginParent::NPAPIPluginParent(const char* aFilePath) : mFilePath(aFilePath), mSubprocess(aFilePath), - mMonitor("mozilla.plugins.NPAPIPluginParent.LaunchPluginProcess"), ALLOW_THIS_IN_INITIALIZER_LIST(mShim(new Shim(this))) { } @@ -96,14 +70,6 @@ NPAPIPluginParent::~NPAPIPluginParent() delete mShim; } -void -NPAPIPluginParent::LaunchSubprocess() -{ - MonitorAutoEnter mon(mMonitor); - mSubprocess.Launch(); - mon.Notify(); -} - NPPProtocolParent* NPAPIPluginParent::NPPConstructor(const String& aMimeType, const uint16_t& aMode, diff --git a/dom/plugins/NPAPIPluginParent.h b/dom/plugins/NPAPIPluginParent.h index d92c981f78b0..44e074eba553 100644 --- a/dom/plugins/NPAPIPluginParent.h +++ b/dom/plugins/NPAPIPluginParent.h @@ -51,7 +51,6 @@ #include "base/string_util.h" -#include "mozilla/Monitor.h" #include "mozilla/SharedLibrary.h" #include "mozilla/plugins/NPAPIProtocol.h" #include "mozilla/plugins/NPAPIProtocolParent.h" @@ -111,8 +110,6 @@ public: PRLibrary* aLibrary); private: - void LaunchSubprocess(); - void SetPluginFuncs(NPPluginFuncs* aFuncs); // Implement the module-level functions from NPAPI; these are @@ -477,8 +474,6 @@ private: PluginProcessParent mSubprocess; const NPNetscapeFuncs* mNPNIface; - mozilla::Monitor mMonitor; - // NPObject interface #if 0 diff --git a/dom/plugins/PluginProcessParent.cpp b/dom/plugins/PluginProcessParent.cpp index 48ab45b9b027..41514106561f 100644 --- a/dom/plugins/PluginProcessParent.cpp +++ b/dom/plugins/PluginProcessParent.cpp @@ -62,7 +62,7 @@ PluginProcessParent::Launch() { std::vector args; args.push_back(UTF8ToWide(mPluginFilePath)); - return mozilla::ipc::GeckoChildProcessHost::Launch(args); + return SyncLaunch(args); } diff --git a/ipc/glue/GeckoChildProcessHost.cpp b/ipc/glue/GeckoChildProcessHost.cpp index 398506b6ded4..87a5bbeb399f 100644 --- a/ipc/glue/GeckoChildProcessHost.cpp +++ b/ipc/glue/GeckoChildProcessHost.cpp @@ -41,17 +41,54 @@ #include "base/string_util.h" #include "chrome/common/chrome_switches.h" +#include "mozilla/ipc/GeckoThread.h" + +using mozilla::MonitorAutoEnter; using mozilla::ipc::GeckoChildProcessHost; +template<> +struct RunnableMethodTraits +{ + static void RetainCallee(GeckoChildProcessHost* obj) { } + static void ReleaseCallee(GeckoChildProcessHost* obj) { } +}; + GeckoChildProcessHost::GeckoChildProcessHost(GeckoChildProcessType aProcessType) : ChildProcessHost(RENDER_PROCESS), // FIXME/cjones: we should own this enum - mProcessType(aProcessType) + mProcessType(aProcessType), + mMonitor("mozilla.ipc.GeckChildProcessHost.mMonitor"), + mLaunched(false) { } bool -GeckoChildProcessHost::Launch(std::vector aExtraOpts) +GeckoChildProcessHost::SyncLaunch(std::vector aExtraOpts) { + MessageLoop* loop = MessageLoop::current(); + MessageLoop* ioLoop = + BrowserProcessSubThread::GetMessageLoop(BrowserProcessSubThread::IO); + NS_ASSERTION(loop != ioLoop, "sync launch from the IO thread NYI"); + + ioLoop->PostTask(FROM_HERE, + NewRunnableMethod(this, + &GeckoChildProcessHost::AsyncLaunch, + aExtraOpts)); + + // NB: this uses a different mechanism than the chromium parent + // class. + MonitorAutoEnter mon(mMonitor); + while (!mLaunched) { + mon.Wait(); + } + + return true; +} + +bool +GeckoChildProcessHost::AsyncLaunch(std::vector aExtraOpts) +{ + // FIXME/cjones: make this work from non-IO threads, too + if (!CreateChannel()) { return false; } @@ -95,30 +132,24 @@ GeckoChildProcessHost::Launch(std::vector aExtraOpts) } SetHandle(process); - // FIXME/cjones: should have the option for sync/async launch. - // however, since most clients already expect this launch to be - // synchronous wrt the channel connecting, we'll hack a bit here. - // (at least we're on the IO thread and not blocking main ...) - MessageLoop* loop = MessageLoop::current(); - bool old_state = loop->NestableTasksAllowed(); - loop->SetNestableTasksAllowed(true); - // spin the loop until OnChannelConnected() comes in, which will Quit() us - loop->Run(); - loop->SetNestableTasksAllowed(old_state); - return true; } void GeckoChildProcessHost::OnChannelConnected(int32 peer_pid) { - MessageLoop::current()->Quit(); + MonitorAutoEnter mon(mMonitor); + mLaunched = true; + mon.Notify(); } + +// XXX/cjones: these next two methods should basically never be called. +// after the process is launched, its channel will be used to create +// one of our channels, AsyncChannel et al. void GeckoChildProcessHost::OnMessageReceived(const IPC::Message& aMsg) { } - void GeckoChildProcessHost::OnChannelError() { diff --git a/ipc/glue/GeckoChildProcessHost.h b/ipc/glue/GeckoChildProcessHost.h index 2f0ece6f91b3..09abcaafdc71 100644 --- a/ipc/glue/GeckoChildProcessHost.h +++ b/ipc/glue/GeckoChildProcessHost.h @@ -43,6 +43,8 @@ #include "base/waitable_event.h" #include "chrome/common/child_process_host.h" +#include "mozilla/Monitor.h" + #include "nsXULAppAPI.h" // for GeckoChildProcessType namespace mozilla { @@ -50,12 +52,15 @@ namespace ipc { class GeckoChildProcessHost : public ChildProcessHost { +protected: + typedef mozilla::Monitor Monitor; + public: GeckoChildProcessHost(GeckoChildProcessType aProcessType=GeckoChildProcess_Default); - bool Launch(std::vector aExtraOpts=std::vector()); + bool SyncLaunch(std::vector aExtraOpts=std::vector()); + bool AsyncLaunch(std::vector aExtraOpts=std::vector()); - // FIXME/cjones: these should probably disappear virtual void OnChannelConnected(int32 peer_pid); virtual void OnMessageReceived(const IPC::Message& aMsg); virtual void OnChannelError(); @@ -72,6 +77,8 @@ public: protected: GeckoChildProcessType mProcessType; + Monitor mMonitor; + bool mLaunched; FilePath mProcessPath; #if defined(OS_POSIX) diff --git a/toolkit/xre/nsEmbedFunctions.cpp b/toolkit/xre/nsEmbedFunctions.cpp index f4279c6499df..adc6a7dffeb2 100644 --- a/toolkit/xre/nsEmbedFunctions.cpp +++ b/toolkit/xre/nsEmbedFunctions.cpp @@ -404,7 +404,8 @@ public: GeckoChildProcessHost* host = new GeckoChildProcessHost(GeckoChildProcess_TestShell); if (host) { - if (!host->Launch()) { + // FIXME/bent: use SyncLaunch() API to simplify this code + if (!host->AsyncLaunch()) { delete host; } // ChildProcessHost deletes itself once the child process exits, on @@ -525,42 +526,23 @@ XRE_RunTestShell(int aArgc, char* aArgv[]) // TestHarness static void -IPCTestHarnessMain(TestProcessParent* subprocess) +IPCTestHarnessMain(void* data) { - TestParent* parent = new TestParent(); // leaks + TestProcessParent* subprocess = new TestProcessParent(); // leaks + bool launched = subprocess->SyncLaunch(); + NS_ASSERTION(launched, "can't launch subprocess"); + TestParent* parent = new TestParent(); // leaks parent->Open(subprocess->GetChannel()); parent->DoStuff(); } -static void -IPCTestHarnessLaunchSubprocess(TestProcessParent* subprocess, - MessageLoop* mainLoop) -{ - bool launched = subprocess->Launch(); - NS_ASSERTION(launched, "can't launch subprocess"); - mainLoop->PostTask(FROM_HERE, - NewRunnableFunction(IPCTestHarnessMain, subprocess)); -} - -static void -IPCTestHarnessPostLaunchSubprocessTask(void* data) -{ - TestProcessParent* subprocess = new TestProcessParent(); - MessageLoop* ioLoop = - BrowserProcessSubThread::GetMessageLoop(BrowserProcessSubThread::IO); - ioLoop->PostTask(FROM_HERE, - NewRunnableFunction(IPCTestHarnessLaunchSubprocess, - subprocess, - MessageLoop::current())); -} - int XRE_RunIPCTestHarness(int aArgc, char* aArgv[]) { nsresult rv = XRE_InitParentProcess( - aArgc, aArgv, IPCTestHarnessPostLaunchSubprocessTask, NULL); + aArgc, aArgv, IPCTestHarnessMain, NULL); NS_ENSURE_SUCCESS(rv, 1); return 0; }