Bug 1041232: Resolve GMP API lifetime issues and allow mid-call shutdown, etc r=cpearce
authorRandell Jesup <rjesup@jesup.org>
Thu, 24 Jul 2014 21:47:40 -0400
changeset 196083 fedd9673d4931b92e02bc7822960d07e876ad123
parent 196082 c8efe58e064cf9496c0a99e22b9783e99bb0b7d6
child 196084 f8186a3237662bbde7288fb8b5d676d33e29b2c5
push id27205
push userkwierso@gmail.com
push dateFri, 25 Jul 2014 22:59:38 +0000
treeherdermozilla-central@e07264876182 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs1041232
milestone34.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1041232: Resolve GMP API lifetime issues and allow mid-call shutdown, etc r=cpearce
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);
@@ -71,18 +72,18 @@ private:
   virtual bool RecvDrainComplete() MOZ_OVERRIDE;
   virtual bool RecvResetComplete() MOZ_OVERRIDE;
   virtual bool RecvError(const GMPErr& aError) 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);
@@ -68,17 +69,17 @@ private:
   virtual bool RecvEncoded(const GMPVideoEncodedFrameData& aEncodedFrame,
                            const nsTArray<uint8_t>& aCodecSpecificInfo) MOZ_OVERRIDE;
   virtual bool RecvError(const GMPErr& aError) 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,37 +6,50 @@
 #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;
   virtual void Error(GMPErr aError) = 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.
@@ -90,28 +92,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;
@@ -152,16 +156,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