Bug 1268984 - Prefer to re-use a GMPParent with the requested nodeId rather than clone. r=jesup a=ritu
authorChris Pearce <cpearce@mozilla.com>
Thu, 05 May 2016 22:35:44 +1200
changeset 332796 bbbcc8fc51fa0b6b58e75c4ec7c15834daddf89b
parent 332795 c74d913c3ac872cd9ceda1d3608f6aec00e08a22
child 332797 95a27b2a8a2d95e9b1c47d527b7034a438df1c39
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjesup, ritu
Bug 1268984 - Prefer to re-use a GMPParent with the requested nodeId rather than clone. r=jesup a=ritu If you request a GMPParent with a nodeId, you should get any already running instances with the same nodeId in preference to cloning an existing GMP and assigning it the nodeId. This is ensures that EME GMP actors that are same-origin run in the same GMP instance. The GMP gtests are failing because of the cross-origin checks in GeckoMediaPluginServiceParent::SelectPluginForAPI(). The loop there selects the first GMPParent that can be used from the nodeId passed in. We previously assumed a GMPParent can be used from a nodeId if the GMPParent has the same nodeId, or if it has not loaded its process and it has no nodeId. The problem with assuming that, is if an in-use GMPParent with the target nodeId lies in the GeckoMediaPluginServiceParent::mPlugins list after a GMPParent with no nodeId, we'll end up using the first GMPParent (the one with no nodeId) rather than the one with the target nodeId. The solution is to change GeckoMediaPluginServiceParent::SelectPluginForAPI() so that effectively if we have a target nodeId, we'll select the first GMPParent that has the same nodeId, or we'll clone the first which supported all the requested capabilities/tags. This means when you request a GMPParent with a given nodeId, you'll get the one with the same nodeId (origin) by preference. MozReview-Commit-ID: 4yVnrO8B1Pg
--- a/dom/media/gmp/GMPParent.cpp
+++ b/dom/media/gmp/GMPParent.cpp
@@ -68,24 +68,23 @@ GMPParent::GMPParent()
   , mIsBlockingDeletion(false)
   , mCanDecrypt(false)
   , mGMPContentChildCount(0)
   , mAsyncShutdownRequired(false)
   , mAsyncShutdownInProgress(false)
   , mChildPid(0)
   , mHoldingSelfRef(false)
-  LOGD("GMPParent ctor");
   mPluginId = GeckoChildProcessHost::GetUniqueID();
+  LOGD("GMPParent ctor id=%u", mPluginId);
-  LOGD("GMPParent dtor");
+  LOGD("GMPParent dtor id=%u", mPluginId);
 GMPParent::CloneFrom(const GMPParent* aOther)
   MOZ_ASSERT(GMPThread() == NS_GetCurrentThread());
   MOZ_ASSERT(aOther->mDirectory && aOther->mService, "null plugin directory");
@@ -980,26 +979,23 @@ GMPParent::CanBeSharedCrossNodeIds() con
          // just yet; especially not for WebRTC. Don't allow CDMs to be used
          // without a node ID.
 GMPParent::CanBeUsedFrom(const nsACString& aNodeId) const
-  return !mAsyncShutdownInProgress &&
-         ((mNodeId.IsEmpty() && State() == GMPStateNotLoaded) ||
-          mNodeId == aNodeId);
+  return !mAsyncShutdownInProgress && mNodeId == aNodeId;
 GMPParent::SetNodeId(const nsACString& aNodeId)
-  MOZ_ASSERT(CanBeUsedFrom(aNodeId));
   mNodeId = aNodeId;
 const nsCString&
 GMPParent::GetDisplayName() const
   return mDisplayName;
--- a/dom/media/gmp/GMPServiceParent.cpp
+++ b/dom/media/gmp/GMPServiceParent.cpp
@@ -1003,18 +1003,16 @@ GeckoMediaPluginServiceParent::SelectPlu
     size_t index = 0;
     RefPtr<GMPParent> gmp;
     while ((gmp = FindPluginForAPIFrom(index, aAPI, aTags, &index))) {
       if (aNodeId.IsEmpty()) {
         if (gmp->CanBeSharedCrossNodeIds()) {
           return gmp.forget();
       } else if (gmp->CanBeUsedFrom(aNodeId)) {
-        MOZ_ASSERT(!aNodeId.IsEmpty());
-        gmp->SetNodeId(aNodeId);
         return gmp.forget();
       if (!gmpToClone ||
           (gmpToClone->IsMarkedForDeletion() && !gmp->IsMarkedForDeletion())) {
         // This GMP has the correct type but has the wrong nodeId; hold on to it
         // in case we need to clone it.
         // Prefer GMPs in-use for the case where an upgraded plugin version is