Bug 1316215 - Merge SelectGMP and LaunchGMP into one synchronous IPC operation. r=gerald
We were seeing almost permaorange failures in the WebRTC H.264/GMP tests
due to the GMP being shutdown in the parent process in between the
content process performing an OOP select operation and then performing
an OOP launch operation.
That is, in GeckoMediaPluginServiceChild::GetContentParent() in between
the SendSelectGMP completing and the SendLaunchGMP completing, the GMP
would shutdown and so when the launch operation ran in the main process
it would fail.
The select and launch are seperate operations so that the crash handler
can be reported to the content process and an association can be made
in the content process between the plugin ID and the crash helper before
we try to launch the GMP. This is so that if the GMP crashes on startup,
we're ready to handle the crash.
However it turns out that if the GMP crashes on startup, the crash report
message comes in after another round of the event/IPC message loop. So we
actually do have time in the content process to connect the crash helper
after the launch fails.
So in order to fix the problem of the GMP shutting down in between select
and launch, we can partially revert the changes I made in
Bug 1267918 to
merge selecting and launching GMPs back into a single operation.
MozReview-Commit-ID: 5n4T1Gqlvr3
--- a/dom/media/gmp/GMPServiceChild.cpp
+++ b/dom/media/gmp/GMPServiceChild.cpp
@@ -68,38 +68,42 @@ GeckoMediaPluginServiceChild::GetContent
nsCString api(aAPI);
nsTArray<nsCString> tags(aTags);
RefPtr<GMPCrashHelper> helper(aHelper);
RefPtr<GeckoMediaPluginServiceChild> self(this);
GetServiceChild()->Then(thread, __func__,
[self, nodeId, api, tags, helper, rawHolder](GMPServiceChild* child) {
UniquePtr<MozPromiseHolder<GetGMPContentParentPromise>> holder(rawHolder);
nsresult rv;
- uint32_t pluginId = 0;
- bool ok = child->SendSelectGMP(nodeId, api, tags, &pluginId, &rv);
- if (!ok || rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN) {
- holder->Reject(rv, __func__);
- return;
- }
-
- if (helper) {
- self->ConnectCrashHelper(pluginId, helper);
- }
nsTArray<base::ProcessId> alreadyBridgedTo;
child->GetAlreadyBridgedTo(alreadyBridgedTo);
base::ProcessId otherProcess;
nsCString displayName;
- ok = child->SendLaunchGMP(pluginId,
- alreadyBridgedTo,
- &otherProcess,
- &displayName,
- &rv);
- if (!ok || rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN) {
+ uint32_t pluginId = 0;
+ bool ok = child->SendLaunchGMP(nodeId,
+ api,
+ tags,
+ alreadyBridgedTo,
+ &pluginId,
+ &otherProcess,
+ &displayName,
+ &rv);
+ if (helper && pluginId) {
+ // Note: Even if the launch failed, we need to connect the crash
+ // helper so that if the launch failed due to the plugin crashing,
+ // we can report the crash via the crash reporter. The crash
+ // handling notification will arrive shortly if the launch failed
+ // due to the plugin crashing.
+ self->ConnectCrashHelper(pluginId, helper);
+ }
+
+ if (!ok || NS_FAILED(rv)) {
+ LOGD(("GeckoMediaPluginServiceChild::GetContentParent SendLaunchGMP failed rv=%d", rv));
holder->Reject(rv, __func__);
return;
}
RefPtr<GMPContentParent> parent;
child->GetBridgedGMPContentParent(otherProcess, getter_AddRefs(parent));
if (!alreadyBridgedTo.Contains(otherProcess)) {
parent->SetDisplayName(displayName);
--- a/dom/media/gmp/GMPServiceParent.cpp
+++ b/dom/media/gmp/GMPServiceParent.cpp
@@ -1962,69 +1962,52 @@ GeckoMediaPluginServiceParent::GetById(u
GMPServiceParent::~GMPServiceParent()
{
NS_DispatchToMainThread(
NewRunnableMethod(mService.get(),
&GeckoMediaPluginServiceParent::ServiceUserDestroyed));
}
mozilla::ipc::IPCResult
-GMPServiceParent::RecvSelectGMP(const nsCString& aNodeId,
+GMPServiceParent::RecvLaunchGMP(const nsCString& aNodeId,
const nsCString& aAPI,
nsTArray<nsCString>&& aTags,
+ nsTArray<ProcessId>&& aAlreadyBridgedTo,
uint32_t* aOutPluginId,
+ ProcessId* aOutProcessId,
+ nsCString* aOutDisplayName,
nsresult* aOutRv)
{
if (mService->IsShuttingDown()) {
*aOutRv = NS_ERROR_ILLEGAL_DURING_SHUTDOWN;
return IPC_OK();
}
RefPtr<GMPParent> gmp = mService->SelectPluginForAPI(aNodeId, aAPI, aTags);
if (gmp) {
*aOutPluginId = gmp->GetPluginId();
- *aOutRv = NS_OK;
} else {
*aOutRv = NS_ERROR_FAILURE;
- }
-
- nsCString api = aTags[0];
- LOGD(("%s: %p returning %p for api %s", __FUNCTION__, (void *)this, (void *)gmp, api.get()));
-
- return IPC_OK();
-}
-
-mozilla::ipc::IPCResult
-GMPServiceParent::RecvLaunchGMP(const uint32_t& aPluginId,
- nsTArray<ProcessId>&& aAlreadyBridgedTo,
- ProcessId* aOutProcessId,
- nsCString* aOutDisplayName,
- nsresult* aOutRv)
-{
- *aOutRv = NS_OK;
- if (mService->IsShuttingDown()) {
- *aOutRv = NS_ERROR_ILLEGAL_DURING_SHUTDOWN;
- return IPC_OK();
- }
-
- RefPtr<GMPParent> gmp(mService->GetById(aPluginId));
- if (!gmp) {
- *aOutRv = NS_ERROR_FAILURE;
+ *aOutPluginId = 0;
return IPC_OK();
}
if (!gmp->EnsureProcessLoaded(aOutProcessId)) {
- return IPC_FAIL_NO_REASON(this);
+ *aOutRv = NS_ERROR_FAILURE;
+ return IPC_OK();
}
*aOutDisplayName = gmp->GetDisplayName();
if (!(aAlreadyBridgedTo.Contains(*aOutProcessId) || gmp->Bridge(this))) {
- return IPC_FAIL_NO_REASON(this);
+ *aOutRv = NS_ERROR_FAILURE;
+ return IPC_OK();
}
+
+ *aOutRv = NS_OK;
return IPC_OK();
}
mozilla::ipc::IPCResult
GMPServiceParent::RecvGetGMPNodeId(const nsString& aOrigin,
const nsString& aTopLevelOrigin,
const nsString& aGMPName,
const bool& aInPrivateBrowsing,
--- a/dom/media/gmp/GMPServiceParent.h
+++ b/dom/media/gmp/GMPServiceParent.h
@@ -251,24 +251,21 @@ public:
const nsString& aTopLevelOrigin,
const nsString& aGMPName,
const bool& aInPrivateBrowsing,
nsCString* aID) override;
void ActorDestroy(ActorDestroyReason aWhy) override;
static PGMPServiceParent* Create(Transport* aTransport, ProcessId aOtherPid);
- mozilla::ipc::IPCResult RecvSelectGMP(const nsCString& aNodeId,
+ mozilla::ipc::IPCResult RecvLaunchGMP(const nsCString& aNodeId,
const nsCString& aAPI,
nsTArray<nsCString>&& aTags,
+ nsTArray<ProcessId>&& aAlreadyBridgedTo,
uint32_t* aOutPluginId,
- nsresult* aOutRv) override;
-
- mozilla::ipc::IPCResult RecvLaunchGMP(const uint32_t& aPluginId,
- nsTArray<ProcessId>&& aAlreadyBridgedTo,
ProcessId* aOutID,
nsCString* aOutDisplayName,
nsresult* aOutRv) override;
private:
void CloseTransport(Monitor* aSyncMonitor, bool* aCompleted);
RefPtr<GeckoMediaPluginServiceParent> mService;
--- a/dom/media/gmp/PGMPService.ipdl
+++ b/dom/media/gmp/PGMPService.ipdl
@@ -11,21 +11,21 @@ namespace mozilla {
namespace gmp {
sync protocol PGMPService
{
parent spawns PGMP as child;
parent:
- sync SelectGMP(nsCString nodeId, nsCString api, nsCString[] tags)
- returns (uint32_t pluginId, nsresult aResult);
-
- sync LaunchGMP(uint32_t pluginId, ProcessId[] alreadyBridgedTo)
- returns (ProcessId id, nsCString displayName, nsresult aResult);
+ sync LaunchGMP(nsCString nodeId,
+ nsCString api,
+ nsCString[] tags,
+ ProcessId[] alreadyBridgedTo)
+ returns (uint32_t pluginId, ProcessId id, nsCString displayName, nsresult aResult);
sync GetGMPNodeId(nsString origin, nsString topLevelOrigin,
nsString gmpName,
bool inPrivateBrowsing)
returns (nsCString id);
};
} // namespace gmp