Bug 1268984 - Prefer to re-use a GMPParent with the requested nodeId rather than clone. r=jesup draft
authorChris Pearce <cpearce@mozilla.com>
Thu, 05 May 2016 22:35:44 +1200
changeset 364166 6a87c41237782a0e054218176673eeb0efab1edb
parent 364165 3d84f8689458ac12b555f7cfbcefaccf294b32e4
child 520199 301604bb7449ca8285f2fe5462480a5512ba482d
push id17373
push usercpearce@mozilla.com
push dateFri, 06 May 2016 01:54:55 +0000
reviewersjesup
bugs1268984
milestone49.0a1
Bug 1268984 - Prefer to re-use a GMPParent with the requested nodeId rather than clone. r=jesup 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
dom/media/gmp/GMPParent.cpp
dom/media/gmp/GMPServiceParent.cpp
--- 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);
 }
 
 GMPParent::~GMPParent()
 {
-  LOGD("GMPParent dtor");
-
+  LOGD("GMPParent dtor id=%u", mPluginId);
   MOZ_ASSERT(!mProcess);
 }
 
 nsresult
 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.
          !mCanDecrypt;
 }
 
 bool
 GMPParent::CanBeUsedFrom(const nsACString& aNodeId) const
 {
-  return !mAsyncShutdownInProgress &&
-         ((mNodeId.IsEmpty() && State() == GMPStateNotLoaded) ||
-          mNodeId == aNodeId);
+  return !mAsyncShutdownInProgress && mNodeId == aNodeId;
 }
 
 void
 GMPParent::SetNodeId(const nsACString& aNodeId)
 {
   MOZ_ASSERT(!aNodeId.IsEmpty());
-  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
@@ -985,18 +985,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