Bug 1333915, part 2 - Stop using bridges and spawns in plugins. r=jimm
The old code would do the content process portion of the open by
immediately sending a message back to the content process, but this
has some weird issues with nesting and priorities. Instead of doing
that, I return the endpoint for the content process back to the
original sync call. This requires more code changes, to thread the
endpoint along, but it is conceptually simpler.
Once I removed the bridges and got it working, I was just able to
remove the spawns from the IPDL file and it worked.
MozReview-Commit-ID: 1tfiJrV4jbV
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -1063,23 +1063,16 @@ ContentChild::DeallocPCycleCollectWithLo
{
// ...so when we get here, there's nothing for us to do.
//
// Also, we're already in ~CycleCollectWithLogsChild (q.v.) at
// this point, so we shouldn't touch the actor in any case.
return true;
}
-mozilla::plugins::PPluginModuleParent*
-ContentChild::AllocPPluginModuleParent(mozilla::ipc::Transport* aTransport,
- base::ProcessId aOtherProcess)
-{
- return plugins::PluginModuleContentParent::Initialize(aTransport, aOtherProcess);
-}
-
PContentBridgeChild*
ContentChild::AllocPContentBridgeChild(mozilla::ipc::Transport* aTransport,
base::ProcessId aOtherProcess)
{
return ContentBridgeChild::Create(aTransport, aOtherProcess);
}
PContentBridgeParent*
@@ -2541,20 +2534,23 @@ ContentChild::RecvGatherProfile()
return IPC_OK();
}
mozilla::ipc::IPCResult
ContentChild::RecvLoadPluginResult(const uint32_t& aPluginId,
const bool& aResult)
{
nsresult rv;
- bool finalResult = aResult && SendConnectPluginBridge(aPluginId, &rv) &&
+ Endpoint<PPluginModuleParent> endpoint;
+ bool finalResult = aResult &&
+ SendConnectPluginBridge(aPluginId, &rv, &endpoint) &&
NS_SUCCEEDED(rv);
plugins::PluginModuleContentParent::OnLoadPluginResult(aPluginId,
- finalResult);
+ finalResult,
+ Move(endpoint));
return IPC_OK();
}
mozilla::ipc::IPCResult
ContentChild::RecvAssociatePluginId(const uint32_t& aPluginId,
const base::ProcessId& aProcessId)
{
plugins::PluginModuleContentParent::AssociatePluginId(aPluginId, aProcessId);
--- a/dom/ipc/ContentChild.h
+++ b/dom/ipc/ContentChild.h
@@ -142,20 +142,16 @@ public:
MOZ_ASSERT(mLastBridge);
ContentBridgeParent* parent = mLastBridge;
mLastBridge = nullptr;
return parent;
}
RefPtr<ContentBridgeParent> mLastBridge;
- PPluginModuleParent *
- AllocPPluginModuleParent(mozilla::ipc::Transport* transport,
- base::ProcessId otherProcess) override;
-
PContentBridgeParent*
AllocPContentBridgeParent(mozilla::ipc::Transport* transport,
base::ProcessId otherProcess) override;
PContentBridgeChild*
AllocPContentBridgeChild(mozilla::ipc::Transport* transport,
base::ProcessId otherProcess) override;
mozilla::ipc::IPCResult
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -791,20 +791,24 @@ ContentParent::RecvCreateGMPService()
MOZ_ASSERT(false, "SendInitGMPService failed");
return IPC_FAIL_NO_REASON(this);
}
return IPC_OK();
}
mozilla::ipc::IPCResult
-ContentParent::RecvLoadPlugin(const uint32_t& aPluginId, nsresult* aRv, uint32_t* aRunID)
+ContentParent::RecvLoadPlugin(const uint32_t& aPluginId,
+ nsresult* aRv,
+ uint32_t* aRunID,
+ Endpoint<PPluginModuleParent>* aEndpoint)
{
*aRv = NS_OK;
- if (!mozilla::plugins::SetupBridge(aPluginId, this, false, aRv, aRunID)) {
+ if (!mozilla::plugins::SetupBridge(aPluginId, this, false, aRv, aRunID,
+ aEndpoint)) {
return IPC_FAIL_NO_REASON(this);
}
return IPC_OK();
}
mozilla::ipc::IPCResult
ContentParent::RecvUngrabPointer(const uint32_t& aTime)
{
@@ -821,25 +825,27 @@ mozilla::ipc::IPCResult
ContentParent::RecvRemovePermission(const IPC::Principal& aPrincipal,
const nsCString& aPermissionType,
nsresult* aRv) {
*aRv = Permissions::RemovePermission(aPrincipal, aPermissionType.get());
return IPC_OK();
}
mozilla::ipc::IPCResult
-ContentParent::RecvConnectPluginBridge(const uint32_t& aPluginId, nsresult* aRv)
+ContentParent::RecvConnectPluginBridge(const uint32_t& aPluginId,
+ nsresult* aRv,
+ Endpoint<PPluginModuleParent>* aEndpoint)
{
*aRv = NS_OK;
// We don't need to get the run ID for the plugin, since we already got it
// in the first call to SetupBridge in RecvLoadPlugin, so we pass in a dummy
// pointer and just throw it away.
uint32_t dummy = 0;
- if (!mozilla::plugins::SetupBridge(aPluginId, this, true, aRv, &dummy)) {
- return IPC_FAIL_NO_REASON(this);
+ if (!mozilla::plugins::SetupBridge(aPluginId, this, true, aRv, &dummy, aEndpoint)) {
+ return IPC_FAIL(this, "SetupBridge failed");
}
return IPC_OK();
}
mozilla::ipc::IPCResult
ContentParent::RecvGetBlocklistState(const uint32_t& aPluginId,
uint32_t* aState)
{
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@@ -252,20 +252,22 @@ public:
bool* aIsForBrowser,
TabId* aTabId) override;
virtual mozilla::ipc::IPCResult RecvBridgeToChildProcess(const ContentParentId& aCpId) override;
virtual mozilla::ipc::IPCResult RecvCreateGMPService() override;
virtual mozilla::ipc::IPCResult RecvLoadPlugin(const uint32_t& aPluginId, nsresult* aRv,
- uint32_t* aRunID) override;
+ uint32_t* aRunID,
+ Endpoint<PPluginModuleParent>* aEndpoint) override;
virtual mozilla::ipc::IPCResult RecvConnectPluginBridge(const uint32_t& aPluginId,
- nsresult* aRv) override;
+ nsresult* aRv,
+ Endpoint<PPluginModuleParent>* aEndpoint) override;
virtual mozilla::ipc::IPCResult RecvGetBlocklistState(const uint32_t& aPluginId,
uint32_t* aIsBlocklisted) override;
virtual mozilla::ipc::IPCResult RecvFindPlugins(const uint32_t& aPluginEpoch,
nsresult* aRv,
nsTArray<PluginTag>* aPlugins,
uint32_t* aNewPluginEpoch) override;
--- a/dom/ipc/PContent.ipdl
+++ b/dom/ipc/PContent.ipdl
@@ -343,18 +343,16 @@ struct GfxInfoFeatureStatus
/**
* The PContent protocol is a top-level protocol between the UI process
* and a content process. There is exactly one PContentParent/PContentChild pair
* for each content process.
*/
nested(upto inside_cpow) sync protocol PContent
{
- parent spawns PPluginModule;
-
manages PBlob;
manages PBrowser;
manages PContentPermissionRequest;
manages PCrashReporter;
manages PCycleCollectWithLogs;
manages PDeviceStorageRequest;
manages PPSMContentDownloader;
manages PExternalHelperApp;
@@ -679,30 +677,31 @@ parent:
ProcessPriority priority,
TabId openerTabId)
returns (ContentParentId cpId, bool isForBrowser, TabId tabId);
sync BridgeToChildProcess(ContentParentId cpId);
async CreateGMPService();
/**
- * This call connects the content process to a plugin process. While this
- * call runs, a new PluginModuleParent will be created in the ContentChild
- * via bridging. The corresponding PluginModuleChild will live in the plugin
- * process.
+ * This call connects the content process to a plugin process. This call
+ * returns an endpoint for a new PluginModuleParent. The corresponding
+ * PluginModuleChild will be started up in the plugin process.
*/
- sync LoadPlugin(uint32_t aPluginId) returns (nsresult aResult, uint32_t aRunID);
+ sync LoadPlugin(uint32_t aPluginId)
+ returns (nsresult aResult, uint32_t aRunID, Endpoint<PPluginModuleParent> aEndpoint);
/**
* This call is used by asynchronous plugin instantiation to notify the
* content parent that it is now safe to initiate the plugin bridge for
- * the specified plugin id. When this call returns, the requested bridge
- * connection has been made.
+ * the specified plugin id. The endpoint for the content process part of the
+ * bridge is returned.
*/
- sync ConnectPluginBridge(uint32_t aPluginId) returns (nsresult rv);
+ sync ConnectPluginBridge(uint32_t aPluginId)
+ returns (nsresult rv, Endpoint<PPluginModuleParent> aEndpoint);
/**
* Return the current blocklist state for a particular plugin.
*/
sync GetBlocklistState(uint32_t aPluginId) returns (uint32_t aState);
/**
* This call returns the set of plugins loaded in the chrome
--- a/dom/plugins/ipc/PPluginModule.ipdl
+++ b/dom/plugins/ipc/PPluginModule.ipdl
@@ -31,18 +31,16 @@ struct PluginSettings
// These settings come from elsewhere.
nsCString userAgent;
bool nativeCursorsSupported;
};
intr protocol PPluginModule
{
- bridges PContent, PPluginModule;
-
manages PPluginInstance;
manages PCrashReporter;
both:
// Window-specific message which instructs the interrupt mechanism to enter
// a nested event loop for the current interrupt call.
async ProcessNativeEventsInInterruptCall();
@@ -104,16 +102,18 @@ child:
async StopProfiler();
async GatherProfile();
async SettingChanged(PluginSettings settings);
async NPP_SetValue_NPNVaudioDeviceChangeDetails(NPAudioDeviceChangeDetailsIPC changeDetails);
+ async InitPluginModuleChild(Endpoint<PPluginModuleChild> endpoint);
+
parent:
async NP_InitializeResult(NPError aError);
/**
* This message is only used on X11 platforms.
*
* Send a dup of the plugin process's X socket to the parent
* process. In theory, this scheme keeps the plugin's X resources
--- a/dom/plugins/ipc/PluginBridge.h
+++ b/dom/plugins/ipc/PluginBridge.h
@@ -10,21 +10,29 @@
#include "base/process.h"
namespace mozilla {
namespace dom {
class ContentParent;
} // namespace dom
+namespace ipc {
+template<class PFooSide>
+class Endpoint;
+} // namespace ipc
+
namespace plugins {
+class PPluginModuleParent;
+
bool
SetupBridge(uint32_t aPluginId, dom::ContentParent* aContentParent,
- bool aForceBridgeNow, nsresult* aResult, uint32_t* aRunID);
+ bool aForceBridgeNow, nsresult* aResult, uint32_t* aRunID,
+ ipc::Endpoint<PPluginModuleParent>* aEndpoint);
nsresult
FindPluginsForContent(uint32_t aPluginEpoch,
nsTArray<PluginTag>* aPlugins,
uint32_t* aNewPluginEpoch);
void
TakeFullMinidump(uint32_t aPluginId,
--- a/dom/plugins/ipc/PluginModuleChild.cpp
+++ b/dom/plugins/ipc/PluginModuleChild.cpp
@@ -92,27 +92,21 @@ typedef BOOL (WINAPI *GetWindowInfoPtr)(
static GetWindowInfoPtr sGetWindowInfoPtrStub = nullptr;
static HWND sBrowserHwnd = nullptr;
// sandbox process doesn't get current key states. So we need get it on chrome.
typedef SHORT (WINAPI *GetKeyStatePtr)(int);
static GetKeyStatePtr sGetKeyStatePtrStub = nullptr;
#endif
/* static */
-PluginModuleChild*
-PluginModuleChild::CreateForContentProcess(mozilla::ipc::Transport* aTransport,
- base::ProcessId aOtherPid)
+bool
+PluginModuleChild::CreateForContentProcess(Endpoint<PPluginModuleChild>&& aEndpoint)
{
auto* child = new PluginModuleChild(false);
-
- if (!child->InitForContent(aOtherPid, XRE_GetIOMessageLoop(), aTransport)) {
- return nullptr;
- }
-
- return child;
+ return child->InitForContent(Move(aEndpoint));
}
PluginModuleChild::PluginModuleChild(bool aIsChrome)
: mLibrary(0)
, mPluginFilename("")
, mQuirks(QUIRKS_NOT_INITIALIZED)
, mIsChrome(aIsChrome)
, mHasShutdown(false)
@@ -180,23 +174,21 @@ PluginModuleChild::CommonInit()
GetIPCChannel()->SetChannelFlags(MessageChannel::REQUIRE_DEFERRED_MESSAGE_PROTECTION);
memset((void*) &mFunctions, 0, sizeof(mFunctions));
mFunctions.size = sizeof(mFunctions);
mFunctions.version = (NP_VERSION_MAJOR << 8) | NP_VERSION_MINOR;
}
bool
-PluginModuleChild::InitForContent(base::ProcessId aParentPid,
- MessageLoop* aIOLoop,
- IPC::Channel* aChannel)
+PluginModuleChild::InitForContent(Endpoint<PPluginModuleChild>&& aEndpoint)
{
CommonInit();
- if (!Open(aChannel, aParentPid, aIOLoop)) {
+ if (!aEndpoint.Bind(this)) {
return false;
}
mLibrary = GetChrome()->mLibrary;
mFunctions = GetChrome()->mFunctions;
return true;
}
@@ -713,21 +705,23 @@ PluginModuleChild::RecvSetAudioSessionDa
NS_ENSURE_SUCCESS(rv, IPC_OK()); // Bail early if this fails
// Ignore failures here; we can't really do anything about them
mozilla::widget::StartAudioSession();
return IPC_OK();
#endif
}
-PPluginModuleChild*
-PluginModuleChild::AllocPPluginModuleChild(mozilla::ipc::Transport* aTransport,
- base::ProcessId aOtherPid)
+mozilla::ipc::IPCResult
+PluginModuleChild::RecvInitPluginModuleChild(Endpoint<PPluginModuleChild>&& aEndpoint)
{
- return PluginModuleChild::CreateForContentProcess(aTransport, aOtherPid);
+ if (!CreateForContentProcess(Move(aEndpoint))) {
+ return IPC_FAIL(this, "CreateForContentProcess failed");
+ }
+ return IPC_OK();
}
PCrashReporterChild*
PluginModuleChild::AllocPCrashReporterChild(mozilla::dom::NativeThreadId* id,
uint32_t* processType)
{
return new CrashReporterChild();
}
--- a/dom/plugins/ipc/PluginModuleChild.h
+++ b/dom/plugins/ipc/PluginModuleChild.h
@@ -73,19 +73,18 @@ protected:
virtual mozilla::ipc::IPCResult RecvDisableFlashProtectedMode() override;
virtual mozilla::ipc::IPCResult AnswerNP_GetEntryPoints(NPError* rv) override;
virtual mozilla::ipc::IPCResult AnswerNP_Initialize(const PluginSettings& aSettings, NPError* rv) override;
virtual mozilla::ipc::IPCResult RecvAsyncNP_Initialize(const PluginSettings& aSettings) override;
virtual mozilla::ipc::IPCResult AnswerSyncNPP_New(PPluginInstanceChild* aActor, NPError* rv)
override;
virtual mozilla::ipc::IPCResult RecvAsyncNPP_New(PPluginInstanceChild* aActor) override;
- virtual PPluginModuleChild*
- AllocPPluginModuleChild(mozilla::ipc::Transport* aTransport,
- base::ProcessId aOtherProcess) override;
+ virtual mozilla::ipc::IPCResult
+ RecvInitPluginModuleChild(Endpoint<PPluginModuleChild>&& endpoint) override;
virtual PPluginInstanceChild*
AllocPPluginInstanceChild(const nsCString& aMimeType,
const uint16_t& aMode,
const InfallibleTArray<nsCString>& aNames,
const InfallibleTArray<nsCString>& aValues)
override;
@@ -153,23 +152,20 @@ public:
void CommonInit();
// aPluginFilename is UTF8, not native-charset!
bool InitForChrome(const std::string& aPluginFilename,
base::ProcessId aParentPid,
MessageLoop* aIOLoop,
IPC::Channel* aChannel);
- bool InitForContent(base::ProcessId aParentPid,
- MessageLoop* aIOLoop,
- IPC::Channel* aChannel);
+ bool InitForContent(Endpoint<PPluginModuleChild>&& aEndpoint);
- static PluginModuleChild*
- CreateForContentProcess(mozilla::ipc::Transport* aTransport,
- base::ProcessId aOtherProcess);
+ static bool
+ CreateForContentProcess(Endpoint<PPluginModuleChild>&& aEndpoint);
void CleanUp();
NPError NP_Shutdown();
const char* GetUserAgent();
static const NPNetscapeFuncs sBrowserFuncs;
--- a/dom/plugins/ipc/PluginModuleParent.cpp
+++ b/dom/plugins/ipc/PluginModuleParent.cpp
@@ -93,17 +93,18 @@ static const char kHangUIMinDisplayPref[
#define CHILD_TIMEOUT_PREF kChildTimeoutPref
#endif
bool
mozilla::plugins::SetupBridge(uint32_t aPluginId,
dom::ContentParent* aContentParent,
bool aForceBridgeNow,
nsresult* rv,
- uint32_t* runID)
+ uint32_t* runID,
+ ipc::Endpoint<PPluginModuleParent>* aEndpoint)
{
PROFILER_LABEL_FUNC(js::ProfileEntry::Category::OTHER);
if (NS_WARN_IF(!rv) || NS_WARN_IF(!runID)) {
return false;
}
PluginModuleChromeParent::ClearInstantiationFlag();
RefPtr<nsPluginHost> host = nsPluginHost::GetInst();
@@ -125,17 +126,34 @@ mozilla::plugins::SetupBridge(uint32_t a
if (chromeParent->IsStartingAsync()) {
chromeParent->SetContentParent(aContentParent);
}
if (!aForceBridgeNow && chromeParent->IsStartingAsync() &&
PluginModuleChromeParent::DidInstantiate()) {
// We'll handle the bridging asynchronously
return true;
}
- *rv = PPluginModule::Bridge(aContentParent, chromeParent);
+
+ ipc::Endpoint<PPluginModuleParent> parent;
+ ipc::Endpoint<PPluginModuleChild> child;
+
+ *rv = PPluginModule::CreateEndpoints(aContentParent->OtherPid(),
+ chromeParent->OtherPid(),
+ &parent, &child);
+ if (NS_FAILED(*rv)) {
+ return true;
+ }
+
+ *aEndpoint = Move(parent);
+
+ if (!chromeParent->SendInitPluginModuleChild(Move(child))) {
+ *rv = NS_ERROR_BRIDGE_OPEN_CHILD;
+ return true;
+ }
+
return true;
}
#ifdef MOZ_CRASHREPORTER_INJECTOR
/**
* Use for executing CreateToolhelp32Snapshot off main thread
*/
@@ -411,21 +429,23 @@ PluginModuleContentParent::LoadModule(ui
* message. Before it sends its response, it sends a message to create
* PluginModuleParent instance. That message is handled by
* PluginModuleContentParent::Initialize, which saves the instance in
* its module mapping. We fetch it from there after LoadPlugin finishes.
*/
dom::ContentChild* cp = dom::ContentChild::GetSingleton();
nsresult rv;
uint32_t runID;
+ Endpoint<PPluginModuleParent> endpoint;
TimeStamp sendLoadPluginStart = TimeStamp::Now();
- if (!cp->SendLoadPlugin(aPluginId, &rv, &runID) ||
+ if (!cp->SendLoadPlugin(aPluginId, &rv, &runID, &endpoint) ||
NS_FAILED(rv)) {
return nullptr;
}
+ Initialize(Move(endpoint));
TimeStamp sendLoadPluginEnd = TimeStamp::Now();
PluginModuleContentParent* parent = mapping->GetModule();
MOZ_ASSERT(parent);
parent->mTimeBlocked += (sendLoadPluginEnd - sendLoadPluginStart);
if (!mapping->IsChannelOpened()) {
// mapping is linked into PluginModuleMapping::sModuleListHead and is
@@ -444,51 +464,49 @@ PluginModuleContentParent::LoadModule(ui
PluginModuleContentParent::AssociatePluginId(uint32_t aPluginId,
base::ProcessId aOtherPid)
{
DebugOnly<PluginModuleMapping*> mapping =
PluginModuleMapping::AssociateWithProcessId(aPluginId, aOtherPid);
MOZ_ASSERT(mapping);
}
-/* static */ PluginModuleContentParent*
-PluginModuleContentParent::Initialize(mozilla::ipc::Transport* aTransport,
- base::ProcessId aOtherPid)
+/* static */ void
+PluginModuleContentParent::Initialize(Endpoint<PPluginModuleParent>&& aEndpoint)
{
nsAutoPtr<PluginModuleMapping> moduleMapping(
- PluginModuleMapping::Resolve(aOtherPid));
+ PluginModuleMapping::Resolve(aEndpoint.OtherPid()));
MOZ_ASSERT(moduleMapping);
PluginModuleContentParent* parent = moduleMapping->GetModule();
MOZ_ASSERT(parent);
- DebugOnly<bool> ok = parent->Open(aTransport, aOtherPid,
- XRE_GetIOMessageLoop(),
- mozilla::ipc::ParentSide);
+ DebugOnly<bool> ok = aEndpoint.Bind(parent);
MOZ_ASSERT(ok);
moduleMapping->SetChannelOpened();
// Request Windows message deferral behavior on our channel. This
// applies to the top level and all sub plugin protocols since they
// all share the same channel.
parent->GetIPCChannel()->SetChannelFlags(MessageChannel::REQUIRE_DEFERRED_MESSAGE_PROTECTION);
TimeoutChanged(kContentTimeoutPref, parent);
// moduleMapping is linked into PluginModuleMapping::sModuleListHead and is
// needed later, so since this function is returning successfully we
// forget it here.
moduleMapping.forget();
- return parent;
}
/* static */ void
PluginModuleContentParent::OnLoadPluginResult(const uint32_t& aPluginId,
- const bool& aResult)
+ const bool& aResult,
+ Endpoint<PPluginModuleParent>&& aEndpoint)
{
+ Initialize(Move(aEndpoint));
nsAutoPtr<PluginModuleMapping> moduleMapping(
PluginModuleMapping::FindModuleByPluginId(aPluginId));
MOZ_ASSERT(moduleMapping);
PluginModuleContentParent* parent = moduleMapping->GetModule();
MOZ_ASSERT(parent);
parent->RecvNP_InitializeResult(aResult ? NPERR_NO_ERROR
: NPERR_GENERIC_ERROR);
}
--- a/dom/plugins/ipc/PluginModuleParent.h
+++ b/dom/plugins/ipc/PluginModuleParent.h
@@ -374,29 +374,31 @@ protected:
class PluginModuleContentParent : public PluginModuleParent
{
public:
explicit PluginModuleContentParent(bool aAllowAsyncInit);
static PluginLibrary* LoadModule(uint32_t aPluginId, nsPluginTag* aPluginTag);
- static PluginModuleContentParent* Initialize(mozilla::ipc::Transport* aTransport,
- base::ProcessId aOtherProcess);
+ static void OnLoadPluginResult(const uint32_t& aPluginId,
+ const bool& aResult,
+ Endpoint<PPluginModuleParent>&& aEndpoint);
- static void OnLoadPluginResult(const uint32_t& aPluginId, const bool& aResult);
static void AssociatePluginId(uint32_t aPluginId, base::ProcessId aProcessId);
virtual ~PluginModuleContentParent();
#if defined(XP_WIN) || defined(XP_MACOSX)
nsresult NP_Initialize(NPNetscapeFuncs* bFuncs, NPError* error) override;
#endif
private:
+ static void Initialize(Endpoint<PPluginModuleParent>&& aEndpoint);
+
virtual bool ShouldContinueFromReplyTimeout() override;
virtual void OnExitedSyncSend() override;
#ifdef MOZ_CRASHREPORTER_INJECTOR
void OnCrash(DWORD processID) override {}
#endif
static PluginModuleContentParent* sSavedModuleParent;