Bug 1224442 - null-check GMP Parent Shmem messages from the Child to handle messages after shutdown. r=cpearce, a=ritu
authorRandell Jesup <rjesup@jesup.org>
Fri, 13 Nov 2015 01:08:01 -0500
changeset 305669 c8917edc60508ce3854e2aaa585289dd06b43d07
parent 305668 7ea5152b84175b42bfe7519e450d24e546e014db
child 305670 a3d6c1d78c443d276869fbd2fc3921547632207a
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce, ritu
bugs1224442
milestone44.0a2
Bug 1224442 - null-check GMP Parent Shmem messages from the Child to handle messages after shutdown. r=cpearce, a=ritu
dom/media/gmp/GMPVideoDecoderParent.cpp
dom/media/gmp/GMPVideoEncoderParent.cpp
dom/media/gmp/GMPVideoHost.cpp
--- a/dom/media/gmp/GMPVideoDecoderParent.cpp
+++ b/dom/media/gmp/GMPVideoDecoderParent.cpp
@@ -273,17 +273,16 @@ GMPVideoDecoderParent::Shutdown()
 // Note: Keep this sync'd up with Shutdown
 void
 GMPVideoDecoderParent::ActorDestroy(ActorDestroyReason aWhy)
 {
   LOGD(("GMPVideoDecoderParent[%p]::ActorDestroy reason=%d", this, aWhy));
 
   mIsOpen = false;
   mActorDestroyed = true;
-  mVideoHost.DoneWithAPI();
 
   // Ensure if we've received a destroy while waiting for a ResetComplete
   // or DrainComplete notification, we'll unblock the caller before processing
   // the error.
   UnblockResetAndDrain();
 
   if (mCallback) {
     // May call Close() (and Shutdown()) immediately or with a delay
--- a/dom/media/gmp/GMPVideoEncoderParent.cpp
+++ b/dom/media/gmp/GMPVideoEncoderParent.cpp
@@ -224,17 +224,16 @@ GMPVideoEncoderParent::Shutdown()
   }
   mShuttingDown = true;
 
   // Notify client we're gone!  Won't occur after Close()
   if (mCallback) {
     mCallback->Terminated();
     mCallback = nullptr;
   }
-  mVideoHost.DoneWithAPI();
 
   mIsOpen = false;
   if (!mActorDestroyed) {
     unused << SendEncodingComplete();
   }
 }
 
 static void
@@ -264,17 +263,17 @@ GMPVideoEncoderParent::ActorDestroy(Acto
                      nsCOMPtr<nsIThread> >(&ShutdownEncodedThread, mEncodedThread));
     mEncodedThread = nullptr;
   }
   if (mPlugin) {
     // Ignore any return code. It is OK for this to fail without killing the process.
     mPlugin->VideoEncoderDestroyed(this);
     mPlugin = nullptr;
   }
-  mVideoHost.ActorDestroyed();
+  mVideoHost.ActorDestroyed(); // same as DoneWithAPI
 }
 
 static void
 EncodedCallback(GMPVideoEncoderCallbackProxy* aCallback,
                 GMPVideoEncodedFrame* aEncodedFrame,
                 nsTArray<uint8_t>* aCodecSpecificInfo,
                 nsCOMPtr<nsIThread> aThread)
 {
@@ -326,29 +325,39 @@ GMPVideoEncoderParent::RecvShutdown()
   Shutdown();
   return true;
 }
 
 bool
 GMPVideoEncoderParent::RecvParentShmemForPool(Shmem&& aFrameBuffer)
 {
   if (aFrameBuffer.IsWritable()) {
-    mVideoHost.SharedMemMgr()->MgrDeallocShmem(GMPSharedMem::kGMPFrameData,
-                                               aFrameBuffer);
+    // This test may be paranoia now that we don't shut down the VideoHost
+    // in ::Shutdown, but doesn't hurt
+    if (mVideoHost.SharedMemMgr()) {
+      mVideoHost.SharedMemMgr()->MgrDeallocShmem(GMPSharedMem::kGMPFrameData,
+                                                 aFrameBuffer);
+    } else {
+      LOGD(("%s::%s: %p Called in shutdown, ignoring and freeing directly", __CLASS__, __FUNCTION__, this));
+      DeallocShmem(aFrameBuffer);
+    }
   }
   return true;
 }
 
 bool
 GMPVideoEncoderParent::AnswerNeedShmem(const uint32_t& aEncodedBufferSize,
                                        Shmem* aMem)
 {
   ipc::Shmem mem;
 
-  if (!mVideoHost.SharedMemMgr()->MgrAllocShmem(GMPSharedMem::kGMPEncodedData,
+  // This test may be paranoia now that we don't shut down the VideoHost
+  // in ::Shutdown, but doesn't hurt
+  if (!mVideoHost.SharedMemMgr() ||
+      !mVideoHost.SharedMemMgr()->MgrAllocShmem(GMPSharedMem::kGMPEncodedData,
                                                 aEncodedBufferSize,
                                                 ipc::SharedMemory::TYPE_BASIC, &mem))
   {
     LOG(LogLevel::Error, ("%s::%s: Failed to get a shared mem buffer for Child! size %u",
                        __CLASS__, __FUNCTION__, aEncodedBufferSize));
     return false;
   }
   *aMem = mem;
--- a/dom/media/gmp/GMPVideoHost.cpp
+++ b/dom/media/gmp/GMPVideoHost.cpp
@@ -66,45 +66,38 @@ GMPVideoHostImpl::CreatePlane(GMPPlane**
 }
 
 GMPSharedMemManager*
 GMPVideoHostImpl::SharedMemMgr()
 {
   return mSharedMemMgr;
 }
 
+// XXX This should merge with ActorDestroyed
 void
 GMPVideoHostImpl::DoneWithAPI()
 {
+  ActorDestroyed();
+}
+
+void
+GMPVideoHostImpl::ActorDestroyed()
+{
   for (uint32_t i = mPlanes.Length(); i > 0; i--) {
     mPlanes[i - 1]->DoneWithAPI();
     mPlanes.RemoveElementAt(i - 1);
   }
   for (uint32_t i = mEncodedFrames.Length(); i > 0; i--) {
     mEncodedFrames[i - 1]->DoneWithAPI();
     mEncodedFrames.RemoveElementAt(i - 1);
   }
   mSharedMemMgr = nullptr;
 }
 
 void
-GMPVideoHostImpl::ActorDestroyed()
-{
-  for (uint32_t i = mPlanes.Length(); i > 0; i--) {
-    mPlanes[i - 1]->ActorDestroyed();
-    mPlanes.RemoveElementAt(i - 1);
-  }
-  for (uint32_t i = mEncodedFrames.Length(); i > 0; i--) {
-    mEncodedFrames[i - 1]->ActorDestroyed();
-    mEncodedFrames.RemoveElementAt(i - 1);
-  }
-  mSharedMemMgr = nullptr;
-}
-
-void
 GMPVideoHostImpl::PlaneCreated(GMPPlaneImpl* aPlane)
 {
   mPlanes.AppendElement(aPlane);
 }
 
 void
 GMPVideoHostImpl::PlaneDestroyed(GMPPlaneImpl* aPlane)
 {