Bug 1041232: Resolve GMP API lifetime issues and allow mid-call shutdown, etc r=cpearce a=sylvestre
authorRandell Jesup <rjesup@jesup.org>
Thu, 24 Jul 2014 21:47:40 -0400
changeset 209180 3fb159eeaad747ec7e2b1722e91117625240e335
parent 209179 debfa7a18dff47f37570de6cd239ccec7030ecaa
child 209181 0cc303c8d18ad599b07d909b9cd9bee139583299
push id6588
push userrjesup@wgate.com
push dateWed, 30 Jul 2014 15:49:16 +0000
treeherdermozilla-aurora@683f69c347f1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce, sylvestre
bugs1041232
milestone33.0a2
Bug 1041232: Resolve GMP API lifetime issues and allow mid-call shutdown, etc r=cpearce a=sylvestre
content/media/gmp/GMPCallbackBase.h
content/media/gmp/GMPParent.cpp
content/media/gmp/GMPVideoDecoderParent.cpp
content/media/gmp/GMPVideoDecoderParent.h
content/media/gmp/GMPVideoDecoderProxy.h
content/media/gmp/GMPVideoEncoderParent.cpp
content/media/gmp/GMPVideoEncoderParent.h
content/media/gmp/GMPVideoEncoderProxy.h
content/media/gmp/moz.build
media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp
media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.h
new file mode 100644
--- /dev/null
+++ b/content/media/gmp/GMPCallbackBase.h
@@ -0,0 +1,21 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef GMPCallbackBase_h_
+#define GMPCallbackBase_h_
+
+class GMPCallbackBase
+{
+public:
+  virtual ~GMPCallbackBase() {}
+
+  // The GMP code will call this if the codec crashes or shuts down.  It's
+  // expected that the consumer (destination of this callback) will respond
+  // by dropping their reference to the proxy, allowing the proxy/parent to
+  // be destroyed.
+  virtual void Terminated() = 0;
+};
+
+#endif
--- a/content/media/gmp/GMPParent.cpp
+++ b/content/media/gmp/GMPParent.cpp
@@ -106,22 +106,22 @@ void
 GMPParent::CloseActive()
 {
   if (mState == GMPStateLoaded) {
     mState = GMPStateUnloading;
   }
 
   // Invalidate and remove any remaining API objects.
   for (uint32_t i = mVideoDecoders.Length(); i > 0; i--) {
-    mVideoDecoders[i - 1]->DecodingComplete();
+    mVideoDecoders[i - 1]->Shutdown();
   }
 
   // Invalidate and remove any remaining API objects.
   for (uint32_t i = mVideoEncoders.Length(); i > 0; i--) {
-    mVideoEncoders[i - 1]->EncodingComplete();
+    mVideoEncoders[i - 1]->Shutdown();
   }
 
   // Note: the shutdown of the codecs is async!  don't kill
   // the plugin-container until they're all safely shut down via
   // CloseIfUnused();
   CloseIfUnused();
 }
 
@@ -256,16 +256,19 @@ GMPParent::GetGMPVideoDecoder(GMPVideoDe
   }
 
   // returned with one anonymous AddRef that locks it until Destroy
   PGMPVideoDecoderParent* pvdp = SendPGMPVideoDecoderConstructor();
   if (!pvdp) {
     return NS_ERROR_FAILURE;
   }
   GMPVideoDecoderParent *vdp = static_cast<GMPVideoDecoderParent*>(pvdp);
+  // This addref corresponds to the Proxy pointer the consumer is returned.
+  // It's dropped by calling Close() on the interface.
+  NS_ADDREF(vdp);
   *aGMPVD = vdp;
   mVideoDecoders.AppendElement(vdp);
 
   return NS_OK;
 }
 
 nsresult
 GMPParent::GetGMPVideoEncoder(GMPVideoEncoderParent** aGMPVE)
@@ -277,16 +280,19 @@ GMPParent::GetGMPVideoEncoder(GMPVideoEn
   }
 
   // returned with one anonymous AddRef that locks it until Destroy
   PGMPVideoEncoderParent* pvep = SendPGMPVideoEncoderConstructor();
   if (!pvep) {
     return NS_ERROR_FAILURE;
   }
   GMPVideoEncoderParent *vep = static_cast<GMPVideoEncoderParent*>(pvep);
+  // This addref corresponds to the Proxy pointer the consumer is returned.
+  // It's dropped by calling Close() on the interface.
+  NS_ADDREF(vep);
   *aGMPVE = vep;
   mVideoEncoders.AppendElement(vep);
 
   return NS_OK;
 }
 
 #ifdef MOZ_CRASHREPORTER
 void
--- a/content/media/gmp/GMPVideoDecoderParent.cpp
+++ b/content/media/gmp/GMPVideoDecoderParent.cpp
@@ -19,18 +19,28 @@ class nsAutoRefTraits<GMPVideoEncodedFra
 {
 public:
   static void Release(GMPVideoEncodedFrame* aFrame) { aFrame->Destroy(); }
 };
 
 namespace mozilla {
 namespace gmp {
 
+// States:
+// Initial: mIsOpen == false
+//    on InitDecode success -> Open
+//    on Shutdown -> Dead
+// Open: mIsOpen == true
+//    on Close -> Dead
+//    on ActorDestroy -> Dead
+//    on Shutdown -> Dead
+// Dead: mIsOpen == false
+
 GMPVideoDecoderParent::GMPVideoDecoderParent(GMPParent* aPlugin)
-  : mCanSendMessages(true)
+  : mIsOpen(false)
   , mPlugin(aPlugin)
   , mCallback(nullptr)
   , mVideoHost(MOZ_THIS_IN_INITIALIZER_LIST())
 {
   MOZ_ASSERT(mPlugin);
 }
 
 GMPVideoDecoderParent::~GMPVideoDecoderParent()
@@ -38,150 +48,171 @@ GMPVideoDecoderParent::~GMPVideoDecoderP
 }
 
 GMPVideoHostImpl&
 GMPVideoDecoderParent::Host()
 {
   return mVideoHost;
 }
 
+// Note: may be called via Terminated()
+void
+GMPVideoDecoderParent::Close()
+{
+  MOZ_ASSERT(mPlugin->GMPThread() == NS_GetCurrentThread());
+  // Consumer is done with us; we can shut down.  No more callbacks should
+  // be made to mCallback.  Note: do this before Shutdown()!
+  mCallback = nullptr;
+  // Let Shutdown mark us as dead so it knows if we had been alive
+
+  // In case this is the last reference
+  nsRefPtr<GMPVideoDecoderParent> kungfudeathgrip(this);
+  NS_RELEASE(kungfudeathgrip);
+  Shutdown();
+}
+
 nsresult
 GMPVideoDecoderParent::InitDecode(const GMPVideoCodec& aCodecSettings,
                                   const nsTArray<uint8_t>& aCodecSpecific,
-                                  GMPVideoDecoderCallback* aCallback,
+                                  GMPVideoDecoderCallbackProxy* aCallback,
                                   int32_t aCoreCount)
 {
-  if (!mCanSendMessages) {
-    NS_WARNING("Trying to use an invalid GMP video decoder!");
+  if (mIsOpen) {
+    NS_WARNING("Trying to re-init an in-use GMP video decoder!");
     return NS_ERROR_FAILURE;
   }
 
   MOZ_ASSERT(mPlugin->GMPThread() == NS_GetCurrentThread());
 
   if (!aCallback) {
     return NS_ERROR_FAILURE;
   }
   mCallback = aCallback;
 
   if (!SendInitDecode(aCodecSettings, aCodecSpecific, aCoreCount)) {
     return NS_ERROR_FAILURE;
   }
+  mIsOpen = true;
 
   // Async IPC, we don't have access to a return value.
   return NS_OK;
 }
 
 nsresult
 GMPVideoDecoderParent::Decode(GMPVideoEncodedFrame* aInputFrame,
                               bool aMissingFrames,
                               const nsTArray<uint8_t>& aCodecSpecificInfo,
                               int64_t aRenderTimeMs)
 {
   nsAutoRef<GMPVideoEncodedFrame> autoDestroy(aInputFrame);
 
-  if (!mCanSendMessages) {
-    NS_WARNING("Trying to use an invalid GMP video decoder!");
+  if (!mIsOpen) {
+    NS_WARNING("Trying to use an dead GMP video decoder");
     return NS_ERROR_FAILURE;
   }
 
   MOZ_ASSERT(mPlugin->GMPThread() == NS_GetCurrentThread());
 
   auto inputFrameImpl = static_cast<GMPVideoEncodedFrameImpl*>(aInputFrame);
 
-  GMPVideoEncodedFrameData frameData;
-  inputFrameImpl->RelinquishFrameData(frameData);
-
   // Very rough kill-switch if the plugin stops processing.  If it's merely
   // hung and continues, we'll come back to life eventually.
   // 3* is because we're using 3 buffers per frame for i420 data for now.
   if (NumInUse(kGMPFrameData) > 3*GMPSharedMemManager::kGMPBufLimit ||
       NumInUse(kGMPEncodedData) > GMPSharedMemManager::kGMPBufLimit) {
     return NS_ERROR_FAILURE;
   }
 
+  GMPVideoEncodedFrameData frameData;
+  inputFrameImpl->RelinquishFrameData(frameData);
+
   if (!SendDecode(frameData,
                   aMissingFrames,
                   aCodecSpecificInfo,
                   aRenderTimeMs)) {
     return NS_ERROR_FAILURE;
   }
 
   // Async IPC, we don't have access to a return value.
   return NS_OK;
 }
 
 nsresult
 GMPVideoDecoderParent::Reset()
 {
-  if (!mCanSendMessages) {
-    NS_WARNING("Trying to use an invalid GMP video decoder!");
+  if (!mIsOpen) {
+    NS_WARNING("Trying to use an dead GMP video decoder");
     return NS_ERROR_FAILURE;
   }
 
   MOZ_ASSERT(mPlugin->GMPThread() == NS_GetCurrentThread());
 
   if (!SendReset()) {
     return NS_ERROR_FAILURE;
   }
 
   // Async IPC, we don't have access to a return value.
   return NS_OK;
 }
 
 nsresult
 GMPVideoDecoderParent::Drain()
 {
-  if (!mCanSendMessages) {
-    NS_WARNING("Trying to use an invalid GMP video decoder!");
+  if (!mIsOpen) {
+    NS_WARNING("Trying to use an dead GMP video decoder");
     return NS_ERROR_FAILURE;
   }
 
   MOZ_ASSERT(mPlugin->GMPThread() == NS_GetCurrentThread());
 
   if (!SendDrain()) {
     return NS_ERROR_FAILURE;
   }
 
   // Async IPC, we don't have access to a return value.
   return NS_OK;
 }
 
 // Note: Consider keeping ActorDestroy sync'd up when making changes here.
 nsresult
-GMPVideoDecoderParent::DecodingComplete()
+GMPVideoDecoderParent::Shutdown()
 {
-  if (!mCanSendMessages) {
-    NS_WARNING("Trying to use an invalid GMP video decoder!");
-    return NS_ERROR_FAILURE;
-  }
-
   MOZ_ASSERT(mPlugin->GMPThread() == NS_GetCurrentThread());
 
-  mCanSendMessages = false;
-
-  mCallback = nullptr;
-
+  // Notify client we're gone!  Won't occur after Close()
+  if (mCallback) {
+    mCallback->Terminated();
+    mCallback = nullptr;
+  }
   mVideoHost.DoneWithAPI();
 
-  unused << SendDecodingComplete();
+  if (mIsOpen) {
+    // Don't send DecodingComplete if we died
+    mIsOpen = false;
+    unused << SendDecodingComplete();
+  }
 
   return NS_OK;
 }
 
-// Note: Keep this sync'd up with DecodingComplete
+// Note: Keep this sync'd up with Shutdown
 void
 GMPVideoDecoderParent::ActorDestroy(ActorDestroyReason aWhy)
 {
+  mIsOpen = false;
+  if (mCallback) {
+    // May call Close() (and Shutdown()) immediately or with a delay
+    mCallback->Terminated();
+    mCallback = nullptr;
+  }
   if (mPlugin) {
     // Ignore any return code. It is OK for this to fail without killing the process.
     mPlugin->VideoDecoderDestroyed(this);
     mPlugin = nullptr;
   }
-  mCanSendMessages = false;
-  mCallback = nullptr;
   mVideoHost.ActorDestroyed();
 }
 
 void
 GMPVideoDecoderParent::CheckThread()
 {
   MOZ_ASSERT(mPlugin->GMPThread() == NS_GetCurrentThread());
 }
--- a/content/media/gmp/GMPVideoDecoderParent.h
+++ b/content/media/gmp/GMPVideoDecoderParent.h
@@ -24,29 +24,30 @@ class GMPVideoDecoderParent MOZ_FINAL : 
                                       , public GMPVideoDecoderProxy
 {
 public:
   NS_INLINE_DECL_REFCOUNTING(GMPVideoDecoderParent)
 
   GMPVideoDecoderParent(GMPParent *aPlugin);
 
   GMPVideoHostImpl& Host();
+  nsresult Shutdown();
 
   // GMPVideoDecoder
+  virtual void Close() MOZ_OVERRIDE;
   virtual nsresult InitDecode(const GMPVideoCodec& aCodecSettings,
                               const nsTArray<uint8_t>& aCodecSpecific,
-                              GMPVideoDecoderCallback* aCallback,
+                              GMPVideoDecoderCallbackProxy* aCallback,
                               int32_t aCoreCount) MOZ_OVERRIDE;
   virtual nsresult Decode(GMPVideoEncodedFrame* aInputFrame,
                           bool aMissingFrames,
                           const nsTArray<uint8_t>& aCodecSpecificInfo,
                           int64_t aRenderTimeMs = -1) MOZ_OVERRIDE;
   virtual nsresult Reset() MOZ_OVERRIDE;
   virtual nsresult Drain() MOZ_OVERRIDE;
-  virtual nsresult DecodingComplete() MOZ_OVERRIDE;
   virtual const uint64_t ParentID() MOZ_OVERRIDE { return reinterpret_cast<uint64_t>(mPlugin.get()); }
 
   // GMPSharedMemManager
   virtual void CheckThread();
   virtual bool Alloc(size_t aSize, Shmem::SharedMemory::SharedMemoryType aType, Shmem* aMem)
   {
 #ifdef GMP_SAFE_SHMEM
     return AllocShmem(aSize, aType, aMem);
@@ -70,18 +71,18 @@ private:
   virtual bool RecvInputDataExhausted() MOZ_OVERRIDE;
   virtual bool RecvDrainComplete() MOZ_OVERRIDE;
   virtual bool RecvResetComplete() MOZ_OVERRIDE;
   virtual bool RecvParentShmemForPool(Shmem& aEncodedBuffer) MOZ_OVERRIDE;
   virtual bool AnswerNeedShmem(const uint32_t& aFrameBufferSize,
                                Shmem* aMem) MOZ_OVERRIDE;
   virtual bool Recv__delete__() MOZ_OVERRIDE;
 
-  bool mCanSendMessages;
+  bool mIsOpen;
   nsRefPtr<GMPParent> mPlugin;
-  GMPVideoDecoderCallback* mCallback;
+  GMPVideoDecoderCallbackProxy* mCallback;
   GMPVideoHostImpl mVideoHost;
 };
 
 } // namespace gmp
 } // namespace mozilla
 
 #endif // GMPVideoDecoderParent_h_
--- a/content/media/gmp/GMPVideoDecoderProxy.h
+++ b/content/media/gmp/GMPVideoDecoderProxy.h
@@ -6,29 +6,48 @@
 #ifndef GMPVideoDecoderProxy_h_
 #define GMPVideoDecoderProxy_h_
 
 #include "nsTArray.h"
 #include "gmp-video-decode.h"
 #include "gmp-video-frame-i420.h"
 #include "gmp-video-frame-encoded.h"
 
+#include "GMPCallbackBase.h"
+
+class GMPVideoDecoderCallbackProxy : public GMPCallbackBase,
+                                     public GMPVideoDecoderCallback
+{
+public:
+  virtual ~GMPVideoDecoderCallbackProxy() {}
+};
+
 // A proxy to GMPVideoDecoder in the child process.
 // GMPVideoDecoderParent exposes this to users the GMP.
 // This enables Gecko to pass nsTArrays to the child GMP and avoid
 // an extra copy when doing so.
+
+// The consumer must call Close() when done with the codec, or when
+// Terminated() is called by the GMP plugin indicating an abnormal shutdown
+// of the underlying plugin.  After calling Close(), the consumer must
+// not access this again.
+
+// This interface is not thread-safe and must only be used from GMPThread.
 class GMPVideoDecoderProxy {
 public:
   virtual nsresult InitDecode(const GMPVideoCodec& aCodecSettings,
                               const nsTArray<uint8_t>& aCodecSpecific,
-                              GMPVideoDecoderCallback* aCallback,
+                              GMPVideoDecoderCallbackProxy* aCallback,
                               int32_t aCoreCount) = 0;
   virtual nsresult Decode(GMPVideoEncodedFrame* aInputFrame,
                           bool aMissingFrames,
                           const nsTArray<uint8_t>& aCodecSpecificInfo,
                           int64_t aRenderTimeMs = -1) = 0;
   virtual nsresult Reset() = 0;
   virtual nsresult Drain() = 0;
-  virtual nsresult DecodingComplete() = 0;
   virtual const uint64_t ParentID() = 0;
+
+  // Call to tell GMP/plugin the consumer will no longer use this
+  // interface/codec.
+  virtual void Close() = 0;
 };
 
 #endif
--- a/content/media/gmp/GMPVideoEncoderParent.cpp
+++ b/content/media/gmp/GMPVideoEncoderParent.cpp
@@ -19,18 +19,28 @@ class nsAutoRefTraits<GMPVideoi420Frame>
 {
 public:
   static void Release(GMPVideoi420Frame* aFrame) { aFrame->Destroy(); }
 };
 
 namespace mozilla {
 namespace gmp {
 
+// States:
+// Initial: mIsOpen == false
+//    on InitDecode success -> Open
+//    on Shutdown -> Dead
+// Open: mIsOpen == true
+//    on Close -> Dead
+//    on ActorDestroy -> Dead
+//    on Shutdown -> Dead
+// Dead: mIsOpen == false
+
 GMPVideoEncoderParent::GMPVideoEncoderParent(GMPParent *aPlugin)
-: mCanSendMessages(true),
+: mIsOpen(false),
   mPlugin(aPlugin),
   mCallback(nullptr),
   mVideoHost(MOZ_THIS_IN_INITIALIZER_LIST())
 {
   MOZ_ASSERT(mPlugin);
 }
 
 GMPVideoEncoderParent::~GMPVideoEncoderParent()
@@ -38,84 +48,101 @@ GMPVideoEncoderParent::~GMPVideoEncoderP
 }
 
 GMPVideoHostImpl&
 GMPVideoEncoderParent::Host()
 {
   return mVideoHost;
 }
 
+// Note: may be called via Terminated()
+void
+GMPVideoEncoderParent::Close()
+{
+  MOZ_ASSERT(mPlugin->GMPThread() == NS_GetCurrentThread());
+  // Consumer is done with us; we can shut down.  No more callbacks should
+  // be made to mCallback.  Note: do this before Shutdown()!
+  mCallback = nullptr;
+  // Let Shutdown mark us as dead so it knows if we had been alive
+
+  // In case this is the last reference
+  nsRefPtr<GMPVideoEncoderParent> kungfudeathgrip(this);
+  NS_RELEASE(kungfudeathgrip);
+  Shutdown();
+}
+
 GMPErr
 GMPVideoEncoderParent::InitEncode(const GMPVideoCodec& aCodecSettings,
                                   const nsTArray<uint8_t>& aCodecSpecific,
                                   GMPVideoEncoderCallbackProxy* aCallback,
                                   int32_t aNumberOfCores,
                                   uint32_t aMaxPayloadSize)
 {
-  if (!mCanSendMessages) {
-    NS_WARNING("Trying to use an invalid GMP video encoder!");
-    return GMPGenericErr;
+  if (mIsOpen) {
+    NS_WARNING("Trying to re-init an in-use GMP video encoder!");
+    return GMPGenericErr;;
   }
 
   MOZ_ASSERT(mPlugin->GMPThread() == NS_GetCurrentThread());
 
   if (!aCallback) {
     return GMPGenericErr;
   }
   mCallback = aCallback;
 
   if (!SendInitEncode(aCodecSettings, aCodecSpecific, aNumberOfCores, aMaxPayloadSize)) {
     return GMPGenericErr;
   }
+  mIsOpen = true;
 
   // Async IPC, we don't have access to a return value.
   return GMPNoErr;
 }
 
 GMPErr
 GMPVideoEncoderParent::Encode(GMPVideoi420Frame* aInputFrame,
                               const nsTArray<uint8_t>& aCodecSpecificInfo,
                               const nsTArray<GMPVideoFrameType>& aFrameTypes)
 {
   nsAutoRef<GMPVideoi420Frame> frameRef(aInputFrame);
 
-  if (!mCanSendMessages) {
-    NS_WARNING("Trying to use an invalid GMP video encoder!");
+  if (!mIsOpen) {
+    NS_WARNING("Trying to use an dead GMP video encoder");
     return GMPGenericErr;
   }
 
   MOZ_ASSERT(mPlugin->GMPThread() == NS_GetCurrentThread());
 
   auto inputFrameImpl = static_cast<GMPVideoi420FrameImpl*>(aInputFrame);
 
-  GMPVideoi420FrameData frameData;
-  inputFrameImpl->InitFrameData(frameData);
-
   // Very rough kill-switch if the plugin stops processing.  If it's merely
   // hung and continues, we'll come back to life eventually.
   // 3* is because we're using 3 buffers per frame for i420 data for now.
   if (NumInUse(kGMPFrameData) > 3*GMPSharedMemManager::kGMPBufLimit ||
       NumInUse(kGMPEncodedData) > GMPSharedMemManager::kGMPBufLimit) {
     return GMPGenericErr;
   }
 
+  GMPVideoi420FrameData frameData;
+  inputFrameImpl->InitFrameData(frameData);
+
   if (!SendEncode(frameData,
                   aCodecSpecificInfo,
                   aFrameTypes)) {
     return GMPGenericErr;
   }
 
   // Async IPC, we don't have access to a return value.
   return GMPNoErr;
 }
 
 GMPErr
 GMPVideoEncoderParent::SetChannelParameters(uint32_t aPacketLoss, uint32_t aRTT)
 {
-  if (!mCanSendMessages) {
+  if (!mIsOpen) {
     NS_WARNING("Trying to use an invalid GMP video encoder!");
     return GMPGenericErr;
   }
 
   MOZ_ASSERT(mPlugin->GMPThread() == NS_GetCurrentThread());
 
   if (!SendSetChannelParameters(aPacketLoss, aRTT)) {
     return GMPGenericErr;
@@ -123,80 +150,83 @@ GMPVideoEncoderParent::SetChannelParamet
 
   // Async IPC, we don't have access to a return value.
   return GMPNoErr;
 }
 
 GMPErr
 GMPVideoEncoderParent::SetRates(uint32_t aNewBitRate, uint32_t aFrameRate)
 {
-  if (!mCanSendMessages) {
-    NS_WARNING("Trying to use an invalid GMP video encoder!");
+  if (!mIsOpen) {
+    NS_WARNING("Trying to use an dead GMP video decoder");
     return GMPGenericErr;
   }
 
   MOZ_ASSERT(mPlugin->GMPThread() == NS_GetCurrentThread());
 
   if (!SendSetRates(aNewBitRate, aFrameRate)) {
     return GMPGenericErr;
   }
 
   // Async IPC, we don't have access to a return value.
   return GMPNoErr;
 }
 
 GMPErr
 GMPVideoEncoderParent::SetPeriodicKeyFrames(bool aEnable)
 {
-  if (!mCanSendMessages) {
+  if (!mIsOpen) {
     NS_WARNING("Trying to use an invalid GMP video encoder!");
     return GMPGenericErr;
   }
 
   MOZ_ASSERT(mPlugin->GMPThread() == NS_GetCurrentThread());
 
   if (!SendSetPeriodicKeyFrames(aEnable)) {
     return GMPGenericErr;
   }
 
   // Async IPC, we don't have access to a return value.
   return GMPNoErr;
 }
 
 // Note: Consider keeping ActorDestroy sync'd up when making changes here.
 void
-GMPVideoEncoderParent::EncodingComplete()
+GMPVideoEncoderParent::Shutdown()
 {
-  if (!mCanSendMessages) {
-    NS_WARNING("Trying to use an invalid GMP video encoder!");
-    return;
-  }
-
   MOZ_ASSERT(mPlugin->GMPThread() == NS_GetCurrentThread());
 
-  mCanSendMessages = false;
-
-  mCallback = nullptr;
-
+  // Notify client we're gone!  Won't occur after Close()
+  if (mCallback) {
+    mCallback->Terminated();
+    mCallback = nullptr;
+  }
   mVideoHost.DoneWithAPI();
-
-  unused << SendEncodingComplete();
+  if (mIsOpen) {
+    // Don't send EncodingComplete if we died
+    mIsOpen = false;
+    unused << SendEncodingComplete();
+  }
 }
 
-// Note: Keep this sync'd up with DecodingComplete
+// Note: Keep this sync'd up with Shutdown
 void
 GMPVideoEncoderParent::ActorDestroy(ActorDestroyReason aWhy)
 {
+  mIsOpen = false;
+  if (mCallback) {
+    // May call Close() (and Shutdown()) immediately or with a delay
+    mCallback->Terminated();
+    mCallback = nullptr;
+  }
   if (mPlugin) {
     // Ignore any return code. It is OK for this to fail without killing the process.
     mPlugin->VideoEncoderDestroyed(this);
     mPlugin = nullptr;
   }
-  mCanSendMessages = false;
-  mCallback = nullptr;
   mVideoHost.ActorDestroyed();
 }
 
 void
 GMPVideoEncoderParent::CheckThread()
 {
   MOZ_ASSERT(mPlugin->GMPThread() == NS_GetCurrentThread());
 }
--- a/content/media/gmp/GMPVideoEncoderParent.h
+++ b/content/media/gmp/GMPVideoEncoderParent.h
@@ -24,30 +24,31 @@ class GMPVideoEncoderParent : public GMP
                               public GMPSharedMemManager
 {
 public:
   NS_INLINE_DECL_REFCOUNTING(GMPVideoEncoderParent)
 
   GMPVideoEncoderParent(GMPParent *aPlugin);
 
   GMPVideoHostImpl& Host();
+  void Shutdown();
 
   // GMPVideoEncoderProxy
+  virtual void Close() MOZ_OVERRIDE;
   virtual GMPErr InitEncode(const GMPVideoCodec& aCodecSettings,
                             const nsTArray<uint8_t>& aCodecSpecific,
                             GMPVideoEncoderCallbackProxy* aCallback,
                             int32_t aNumberOfCores,
                             uint32_t aMaxPayloadSize) MOZ_OVERRIDE;
   virtual GMPErr Encode(GMPVideoi420Frame* aInputFrame,
                         const nsTArray<uint8_t>& aCodecSpecificInfo,
                         const nsTArray<GMPVideoFrameType>& aFrameTypes) MOZ_OVERRIDE;
   virtual GMPErr SetChannelParameters(uint32_t aPacketLoss, uint32_t aRTT) MOZ_OVERRIDE;
   virtual GMPErr SetRates(uint32_t aNewBitRate, uint32_t aFrameRate) MOZ_OVERRIDE;
   virtual GMPErr SetPeriodicKeyFrames(bool aEnable) MOZ_OVERRIDE;
-  virtual void EncodingComplete() MOZ_OVERRIDE;
   virtual const uint64_t ParentID() MOZ_OVERRIDE { return reinterpret_cast<uint64_t>(mPlugin.get()); }
 
   // GMPSharedMemManager
   virtual void CheckThread();
   virtual bool Alloc(size_t aSize, Shmem::SharedMemory::SharedMemoryType aType, Shmem* aMem)
   {
 #ifdef GMP_SAFE_SHMEM
     return AllocShmem(aSize, aType, aMem);
@@ -67,17 +68,17 @@ private:
   virtual void ActorDestroy(ActorDestroyReason aWhy) MOZ_OVERRIDE;
   virtual bool RecvEncoded(const GMPVideoEncodedFrameData& aEncodedFrame,
                            const nsTArray<uint8_t>& aCodecSpecificInfo) MOZ_OVERRIDE;
   virtual bool RecvParentShmemForPool(Shmem& aFrameBuffer) MOZ_OVERRIDE;
   virtual bool AnswerNeedShmem(const uint32_t& aEncodedBufferSize,
                                Shmem* aMem) MOZ_OVERRIDE;
   virtual bool Recv__delete__() MOZ_OVERRIDE;
 
-  bool mCanSendMessages;
+  bool mIsOpen;
   nsRefPtr<GMPParent> mPlugin;
   GMPVideoEncoderCallbackProxy* mCallback;
   GMPVideoHostImpl mVideoHost;
 };
 
 } // namespace gmp
 } // namespace mozilla
 
--- a/content/media/gmp/GMPVideoEncoderProxy.h
+++ b/content/media/gmp/GMPVideoEncoderProxy.h
@@ -6,36 +6,49 @@
 #ifndef GMPVideoEncoderProxy_h_
 #define GMPVideoEncoderProxy_h_
 
 #include "nsTArray.h"
 #include "gmp-video-encode.h"
 #include "gmp-video-frame-i420.h"
 #include "gmp-video-frame-encoded.h"
 
-class GMPVideoEncoderCallbackProxy {
+#include "GMPCallbackBase.h"
+
+class GMPVideoEncoderCallbackProxy : public GMPCallbackBase {
 public:
+  virtual ~GMPVideoEncoderCallbackProxy() {}
   virtual void Encoded(GMPVideoEncodedFrame* aEncodedFrame,
                        const nsTArray<uint8_t>& aCodecSpecificInfo) = 0;
 };
 
 // A proxy to GMPVideoEncoder in the child process.
 // GMPVideoEncoderParent exposes this to users the GMP.
 // This enables Gecko to pass nsTArrays to the child GMP and avoid
 // an extra copy when doing so.
+
+// The consumer must call Close() when done with the codec, or when
+// Terminated() is called by the GMP plugin indicating an abnormal shutdown
+// of the underlying plugin.  After calling Close(), the consumer must
+// not access this again.
+
+// This interface is not thread-safe and must only be used from GMPThread.
 class GMPVideoEncoderProxy {
 public:
   virtual GMPErr InitEncode(const GMPVideoCodec& aCodecSettings,
                             const nsTArray<uint8_t>& aCodecSpecific,
                             GMPVideoEncoderCallbackProxy* aCallback,
                             int32_t aNumberOfCores,
                             uint32_t aMaxPayloadSize) = 0;
   virtual GMPErr Encode(GMPVideoi420Frame* aInputFrame,
                         const nsTArray<uint8_t>& aCodecSpecificInfo,
                         const nsTArray<GMPVideoFrameType>& aFrameTypes) = 0;
   virtual GMPErr SetChannelParameters(uint32_t aPacketLoss, uint32_t aRTT) = 0;
   virtual GMPErr SetRates(uint32_t aNewBitRate, uint32_t aFrameRate) = 0;
   virtual GMPErr SetPeriodicKeyFrames(bool aEnable) = 0;
-  virtual void EncodingComplete() = 0;
   virtual const uint64_t ParentID() = 0;
+
+  // Call to tell GMP/plugin the consumer will no longer use this
+  // interface/codec.
+  virtual void Close() = 0;
 };
 
 #endif // GMPVideoEncoderProxy_h_
--- a/content/media/gmp/moz.build
+++ b/content/media/gmp/moz.build
@@ -24,16 +24,17 @@ EXPORTS += [
     'gmp-api/gmp-video-codec.h',
     'gmp-api/gmp-video-decode.h',
     'gmp-api/gmp-video-encode.h',
     'gmp-api/gmp-video-frame-encoded.h',
     'gmp-api/gmp-video-frame-i420.h',
     'gmp-api/gmp-video-frame.h',
     'gmp-api/gmp-video-host.h',
     'gmp-api/gmp-video-plane.h',
+    'GMPCallbackBase.h',
     'GMPChild.h',
     'GMPMessageUtils.h',
     'GMPParent.h',
     'GMPPlatform.h',
     'GMPProcessChild.h',
     'GMPProcessParent.h',
     'GMPService.h',
     'GMPSharedMemManager.h',
--- a/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp
+++ b/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp
@@ -48,16 +48,35 @@ GetGMPLog()
 
 // Encoder.
 WebrtcGmpVideoEncoder::WebrtcGmpVideoEncoder()
   : mGMP(nullptr)
   , mHost(nullptr)
   , mCallback(nullptr)
 {}
 
+static void
+Encoder_Close_g(GMPVideoEncoderProxy* aGMP)
+{
+  aGMP->Close();
+}
+
+WebrtcGmpVideoEncoder::~WebrtcGmpVideoEncoder()
+{
+  // Note: we only use SyncRunnables to access mGMP
+  // Callbacks may occur at any time until we call Close (or receive
+  // Terminated()), so call Close here synchronously.
+  // Do NOT touch the refcount of 'this'!
+  if (mGMPThread && mGMP) {
+    mozilla::SyncRunnable::DispatchToThread(mGMPThread,
+                                            WrapRunnableNM(&Encoder_Close_g, mGMP));
+    mGMP = nullptr;
+  }
+}
+
 static int
 WebrtcFrameTypeToGmpFrameType(webrtc::VideoFrameType aIn,
                               GMPVideoFrameType *aOut)
 {
   MOZ_ASSERT(aOut);
   switch(aIn) {
     case webrtc::kKeyFrame:
       *aOut = kGMPKeyFrame;
@@ -123,23 +142,23 @@ WebrtcGmpVideoEncoder::InitEncode(const 
   if (!mGMPThread) {
     if (NS_WARN_IF(NS_FAILED(mMPS->GetThread(getter_AddRefs(mGMPThread))))) {
       mMPS = nullptr;
       return WEBRTC_VIDEO_CODEC_ERROR;
     }
   }
 
   int32_t ret;
-  mozilla::SyncRunnable::DispatchToThread(mGMPThread,
-                WrapRunnableRet(this,
-                                &WebrtcGmpVideoEncoder::InitEncode_g,
-                                aCodecSettings,
-                                aNumberOfCores,
-                                aMaxPayloadSize,
-                                &ret));
+  mGMPThread->Dispatch(WrapRunnableRet(this,
+                                       &WebrtcGmpVideoEncoder::InitEncode_g,
+                                       aCodecSettings,
+                                       aNumberOfCores,
+                                       aMaxPayloadSize,
+                                       &ret),
+                       NS_DISPATCH_SYNC);
 
   return ret;
 }
 
 int32_t
 WebrtcGmpVideoEncoder::InitEncode_g(const webrtc::VideoCodec* aCodecSettings,
                                     int32_t aNumberOfCores,
                                     uint32_t aMaxPayloadSize)
@@ -171,17 +190,17 @@ WebrtcGmpVideoEncoder::InitEncode_g(cons
   codec.mHeight = aCodecSettings->height;
   codec.mStartBitrate = aCodecSettings->startBitrate;
   codec.mMinBitrate = aCodecSettings->minBitrate;
   codec.mMaxBitrate = aCodecSettings->maxBitrate;
   codec.mMaxFramerate = aCodecSettings->maxFramerate;
 
   // Pass dummy codecSpecific data for now...
   nsTArray<uint8_t> codecSpecific;
- 
+
   // H.264 mode 1 only supported so far
   GMPErr err = mGMP->InitEncode(codec, codecSpecific, this, 1, 1024*1024 /*aMaxPayloadSize*/);
   if (err != GMPNoErr) {
     return WEBRTC_VIDEO_CODEC_ERROR;
   }
 
   return WEBRTC_VIDEO_CODEC_OK;
 }
@@ -207,17 +226,20 @@ WebrtcGmpVideoEncoder::Encode(const webr
 
 
 int32_t
 WebrtcGmpVideoEncoder::Encode_g(const webrtc::I420VideoFrame* aInputImage,
                                 const webrtc::CodecSpecificInfo* aCodecSpecificInfo,
                                 const std::vector<webrtc::VideoFrameType>* aFrameTypes)
 {
   MOZ_ASSERT(mHost);
-  MOZ_ASSERT(mGMP);
+  if (!mGMP) {
+    // destroyed via Terminate()
+    return WEBRTC_VIDEO_CODEC_ERROR;
+  }
 
   GMPVideoFrame* ftmp = nullptr;
   GMPErr err = mHost->CreateFrame(kGMPI420VideoFrame, &ftmp);
   if (err != GMPNoErr) {
     return WEBRTC_VIDEO_CODEC_ERROR;
   }
   GMPVideoi420Frame* frame = static_cast<GMPVideoi420Frame*>(ftmp);
 
@@ -274,16 +296,24 @@ WebrtcGmpVideoEncoder::RegisterEncodeCom
   mCallback = aCallback;
 
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
 int32_t
 WebrtcGmpVideoEncoder::Release()
 {
+  // Note: we only use SyncRunnables to access mGMP
+  // Callbacks may occur at any time until we call Close (or receive
+  // Terminated()), so call Close here synchronously.
+  if (mGMPThread && mGMP) {
+    mozilla::SyncRunnable::DispatchToThread(mGMPThread,
+                                            WrapRunnableNM(&Encoder_Close_g, mGMP));
+  }
+  // Now safe to forget things
   mMPS = nullptr;
   mGMP = nullptr;
   mHost = nullptr;
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
 int32_t
 WebrtcGmpVideoEncoder::SetChannelParameters(uint32_t aPacketLoss, int aRTT)
@@ -303,27 +333,40 @@ WebrtcGmpVideoEncoder::SetRates(uint32_t
                                 &ret));
 
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
 int32_t
 WebrtcGmpVideoEncoder::SetRates_g(uint32_t aNewBitRate, uint32_t aFrameRate)
 {
-  MOZ_ASSERT(mGMP);
+  if (!mGMP) {
+    // destroyed via Terminate()
+    return WEBRTC_VIDEO_CODEC_ERROR;
+  }
+
   GMPErr err = mGMP->SetRates(aNewBitRate, aFrameRate);
   if (err != GMPNoErr) {
     return WEBRTC_VIDEO_CODEC_ERROR;
   }
 
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
 // GMPVideoEncoderCallback virtual functions.
 void
+WebrtcGmpVideoEncoder::Terminated()
+{
+  // We need to drop our reference to this
+  mGMP->Close();
+  mGMP = nullptr;
+  // Could now notify that it's dead
+}
+
+void
 WebrtcGmpVideoEncoder::Encoded(GMPVideoEncodedFrame* aEncodedFrame,
                                const nsTArray<uint8_t>& aCodecSpecificInfo)
 {
   if (mCallback) { // paranoia
     webrtc::VideoFrameType ft;
     GmpFrameTypeToWebrtcFrameType(aEncodedFrame->FrameType(), &ft);
     uint32_t timestamp = (aEncodedFrame->TimeStamp() * 90ll + 999)/1000;
 
@@ -384,36 +427,55 @@ WebrtcGmpVideoEncoder::Encoded(GMPVideoE
 }
 
 // Decoder.
 WebrtcGmpVideoDecoder::WebrtcGmpVideoDecoder() :
   mGMP(nullptr),
   mHost(nullptr),
   mCallback(nullptr) {}
 
+static void
+Decoder_Close_g(GMPVideoDecoderProxy* aGMP)
+{
+  aGMP->Close();
+}
+
+WebrtcGmpVideoDecoder::~WebrtcGmpVideoDecoder()
+{
+  // Note: we only use SyncRunnables to access mGMP
+  // Callbacks may occur at any time until we call Close (or receive
+  // Terminated()), so call Close here synchronously.
+  // Do NOT touch the refcount of 'this'!
+  if (mGMPThread && mGMP) {
+    mozilla::SyncRunnable::DispatchToThread(mGMPThread,
+                                            WrapRunnableNM(&Decoder_Close_g, mGMP));
+    mGMP = nullptr;
+  }
+}
+
 int32_t
 WebrtcGmpVideoDecoder::InitDecode(const webrtc::VideoCodec* aCodecSettings,
                                   int32_t aNumberOfCores)
 {
   mMPS = do_GetService("@mozilla.org/gecko-media-plugin-service;1");
   MOZ_ASSERT(mMPS);
 
   if (!mGMPThread) {
     if (NS_WARN_IF(NS_FAILED(mMPS->GetThread(getter_AddRefs(mGMPThread))))) {
       return WEBRTC_VIDEO_CODEC_ERROR;
     }
   }
 
   int32_t ret;
-  mozilla::SyncRunnable::DispatchToThread(mGMPThread,
-                WrapRunnableRet(this,
-                                &WebrtcGmpVideoDecoder::InitDecode_g,
-                                aCodecSettings,
-                                aNumberOfCores,
-                                &ret));
+  mGMPThread->Dispatch(WrapRunnableRet(this,
+                                       &WebrtcGmpVideoDecoder::InitDecode_g,
+                                       aCodecSettings,
+                                       aNumberOfCores,
+                                       &ret),
+                       NS_DISPATCH_SYNC);
 
   return ret;
 }
 
 int32_t
 WebrtcGmpVideoDecoder::InitDecode_g(const webrtc::VideoCodec* aCodecSettings,
                                     int32_t aNumberOfCores)
 {
@@ -477,17 +539,20 @@ WebrtcGmpVideoDecoder::Decode(const webr
 int32_t
 WebrtcGmpVideoDecoder::Decode_g(const webrtc::EncodedImage& aInputImage,
                                 bool aMissingFrames,
                                 const webrtc::RTPFragmentationHeader* aFragmentation,
                                 const webrtc::CodecSpecificInfo* aCodecSpecificInfo,
                                 int64_t aRenderTimeMs)
 {
   MOZ_ASSERT(mHost);
-  MOZ_ASSERT(mGMP);
+  if (!mGMP) {
+    // destroyed via Terminate()
+    return WEBRTC_VIDEO_CODEC_ERROR;
+  }
 
   GMPVideoFrame* ftmp = nullptr;
   GMPErr err = mHost->CreateFrame(kGMPEncodedVideoFrame, &ftmp);
   if (err != GMPNoErr) {
     return WEBRTC_VIDEO_CODEC_ERROR;
   }
 
   GMPVideoEncodedFrame* frame = static_cast<GMPVideoEncodedFrame*>(ftmp);
@@ -542,31 +607,47 @@ WebrtcGmpVideoDecoder::RegisterDecodeCom
 
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
 
 int32_t
 WebrtcGmpVideoDecoder::Release()
 {
+  // Note: we only use SyncRunnables to access mGMP
+  // Callbacks may occur at any time until we call Close (or receive
+  // Terminated()), so call Close here synchronously.
+  if (mGMPThread && mGMP) {
+    mozilla::SyncRunnable::DispatchToThread(mGMPThread,
+                                            WrapRunnableNM(&Decoder_Close_g, mGMP));
+  }
+  // Now safe to forget things
   mMPS = nullptr;
   mGMP = nullptr;
   mGMPThread = nullptr;
   mHost = nullptr;
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
 int32_t
 WebrtcGmpVideoDecoder::Reset()
 {
   // XXX ?
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
 void
+WebrtcGmpVideoDecoder::Terminated()
+{
+  mGMP->Close();
+  mGMP = nullptr;
+  // Could now notify that it's dead
+}
+
+void
 WebrtcGmpVideoDecoder::Decoded(GMPVideoi420Frame* aDecodedFrame)
 {
   if (mCallback) { // paranioa
     webrtc::I420VideoFrame image;
     int ret = image.CreateFrame(aDecodedFrame->AllocatedSize(kGMPYPlane),
                                 aDecodedFrame->Buffer(kGMPYPlane),
                                 aDecodedFrame->AllocatedSize(kGMPUPlane),
                                 aDecodedFrame->Buffer(kGMPUPlane),
--- a/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.h
+++ b/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.h
@@ -35,36 +35,38 @@
 
 namespace mozilla {
 
 class WebrtcGmpVideoEncoder : public WebrtcVideoEncoder,
                               public GMPVideoEncoderCallbackProxy
 {
 public:
   WebrtcGmpVideoEncoder();
-  virtual ~WebrtcGmpVideoEncoder() {}
+  virtual ~WebrtcGmpVideoEncoder();
 
   // Implement VideoEncoder interface.
   virtual const uint64_t PluginID() MOZ_OVERRIDE
   {
     return mGMP ? mGMP->ParentID() : 0;
   }
 
+  virtual void Terminated() MOZ_OVERRIDE;
+
   virtual int32_t InitEncode(const webrtc::VideoCodec* aCodecSettings,
                              int32_t aNumberOfCores,
-                             uint32_t aMaxPayloadSize);
+                             uint32_t aMaxPayloadSize) MOZ_OVERRIDE;
 
   virtual int32_t Encode(const webrtc::I420VideoFrame& aInputImage,
                          const webrtc::CodecSpecificInfo* aCodecSpecificInfo,
-                         const std::vector<webrtc::VideoFrameType>* aFrameTypes);
+                         const std::vector<webrtc::VideoFrameType>* aFrameTypes) MOZ_OVERRIDE;
 
   virtual int32_t RegisterEncodeCompleteCallback(
     webrtc::EncodedImageCallback* aCallback) MOZ_OVERRIDE;
 
-  virtual int32_t Release();
+  virtual int32_t Release() MOZ_OVERRIDE;
 
   virtual int32_t SetChannelParameters(uint32_t aPacketLoss,
                                        int aRTT) MOZ_OVERRIDE;
 
   virtual int32_t SetRates(uint32_t aNewBitRate,
                            uint32_t aFrameRate) MOZ_OVERRIDE;
 
   // GMPVideoEncoderCallback virtual functions.
@@ -88,28 +90,30 @@ private:
   nsCOMPtr<nsIThread> mGMPThread;
   GMPVideoEncoderProxy* mGMP;
   GMPVideoHost* mHost;
   webrtc::EncodedImageCallback* mCallback;
 };
 
 
 class WebrtcGmpVideoDecoder : public WebrtcVideoDecoder,
-                              public GMPVideoDecoderCallback
+                              public GMPVideoDecoderCallbackProxy
 {
 public:
   WebrtcGmpVideoDecoder();
-  virtual ~WebrtcGmpVideoDecoder() {}
+  virtual ~WebrtcGmpVideoDecoder();
 
   // Implement VideoDecoder interface.
   virtual const uint64_t PluginID() MOZ_OVERRIDE
   {
     return mGMP ? mGMP->ParentID() : 0;
   }
 
+  virtual void Terminated();
+
   virtual int32_t InitDecode(const webrtc::VideoCodec* aCodecSettings,
                              int32_t aNumberOfCores);
   virtual int32_t Decode(const webrtc::EncodedImage& aInputImage,
                          bool aMissingFrames,
                          const webrtc::RTPFragmentationHeader* aFragmentation,
                          const webrtc::CodecSpecificInfo* aCodecSpecificInfo = nullptr,
                          int64_t aRenderTimeMs = -1) MOZ_OVERRIDE;
   virtual int32_t RegisterDecodeCompleteCallback(webrtc::DecodedImageCallback* aCallback) MOZ_OVERRIDE;
@@ -147,16 +151,16 @@ private:
   virtual int32_t Decode_g(const webrtc::EncodedImage& aInputImage,
                            bool aMissingFrames,
                            const webrtc::RTPFragmentationHeader* aFragmentation,
                            const webrtc::CodecSpecificInfo* aCodecSpecificInfo,
                            int64_t aRenderTimeMs);
 
   nsCOMPtr<mozIGeckoMediaPluginService> mMPS;
   nsCOMPtr<nsIThread> mGMPThread;
-  GMPVideoDecoderProxy*  mGMP;
+  GMPVideoDecoderProxy* mGMP; // Addref is held for us
   GMPVideoHost* mHost;
   webrtc::DecodedImageCallback* mCallback;
 };
 
 }
 
 #endif