Bug 1124021 - Fix dangerous UniquePtr usage pattern in GMP. r=cpearce a=lmandel
authorMatthew Gregan <kinetik@flim.org>
Tue, 20 Jan 2015 18:39:00 +1300
changeset 250174 3f463a602bea
parent 250173 589dc8554797
child 250175 bb90dd41c737
push id4521
push usercpearce@mozilla.com
push date2015-03-04 01:22 +0000
treeherdermozilla-beta@8abdbdecd2d6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce, lmandel
bugs1124021
milestone37.0
Bug 1124021 - Fix dangerous UniquePtr usage pattern in GMP. r=cpearce a=lmandel
dom/media/fmp4/eme/EMEH264Decoder.cpp
dom/media/gmp/GMPUtils.h
dom/media/gmp/GMPVideoDecoderParent.cpp
dom/media/gmp/GMPVideoDecoderParent.h
dom/media/gmp/GMPVideoDecoderProxy.h
dom/media/gmp/GMPVideoEncodedFrameImpl.h
dom/media/gmp/GMPVideoEncoderParent.cpp
dom/media/gmp/GMPVideoEncoderParent.h
dom/media/gmp/GMPVideoEncoderProxy.h
dom/media/gmp/GMPVideoi420FrameImpl.h
dom/media/gmp/moz.build
media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp
--- a/dom/media/fmp4/eme/EMEH264Decoder.cpp
+++ b/dom/media/fmp4/eme/EMEH264Decoder.cpp
@@ -307,17 +307,17 @@ EMEH264Decoder::GmpInput(MP4Sample* aSam
 
   GMPVideoFrame* ftmp = nullptr;
   GMPErr err = mHost->CreateFrame(kGMPEncodedVideoFrame, &ftmp);
   if (GMP_FAILED(err)) {
     mCallback->Error();
     return NS_ERROR_FAILURE;
   }
 
-  UniquePtr<gmp::GMPVideoEncodedFrameImpl> frame(static_cast<gmp::GMPVideoEncodedFrameImpl*>(ftmp));
+  GMPUniquePtr<gmp::GMPVideoEncodedFrameImpl> frame(static_cast<gmp::GMPVideoEncodedFrameImpl*>(ftmp));
   err = frame->CreateEmptyFrame(sample->size);
   if (GMP_FAILED(err)) {
     mCallback->Error();
     return NS_ERROR_FAILURE;
   }
 
   memcpy(frame->Buffer(), sample->data, frame->Size());
 
@@ -328,17 +328,17 @@ EMEH264Decoder::GmpInput(MP4Sample* aSam
   frame->SetDuration(sample->duration);
   if (sample->crypto.valid) {
     frame->InitCrypto(sample->crypto);
   }
   frame->SetFrameType(sample->is_sync_point ? kGMPKeyFrame : kGMPDeltaFrame);
   frame->SetBufferType(GMP_BufferLength32);
 
   nsTArray<uint8_t> info; // No codec specific per-frame info to pass.
-  nsresult rv = mGMP->Decode(UniquePtr<GMPVideoEncodedFrame>(frame.release()), false, info, 0);
+  nsresult rv = mGMP->Decode(GMPUniquePtr<GMPVideoEncodedFrame>(frame.release()), false, info, 0);
   if (NS_FAILED(rv)) {
     mCallback->Error();
     return rv;
   }
 
   return NS_OK;
 }
 
new file mode 100644
--- /dev/null
+++ b/dom/media/gmp/GMPUtils.h
@@ -0,0 +1,26 @@
+/* -*- 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 GMPUtils_h_
+#define GMPUtils_h_
+
+#include "mozilla/UniquePtr.h"
+
+namespace mozilla {
+
+template<typename T>
+struct DestroyPolicy
+{
+  void operator()(T* aGMPObject) const {
+    aGMPObject->Destroy();
+  }
+};
+
+template<typename T>
+using GMPUniquePtr = mozilla::UniquePtr<T, DestroyPolicy<T>>;
+
+} // namespace mozilla
+
+#endif
--- a/dom/media/gmp/GMPVideoDecoderParent.cpp
+++ b/dom/media/gmp/GMPVideoDecoderParent.cpp
@@ -3,16 +3,17 @@
  * 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/. */
 
 #include "GMPVideoDecoderParent.h"
 #include "prlog.h"
 #include "mozilla/unused.h"
 #include "nsAutoRef.h"
 #include "nsThreadUtils.h"
+#include "GMPUtils.h"
 #include "GMPVideoEncodedFrameImpl.h"
 #include "GMPVideoi420FrameImpl.h"
 #include "GMPParent.h"
 #include "GMPMessageUtils.h"
 #include "mozilla/gmp/GMPTypes.h"
 
 namespace mozilla {
 
@@ -103,29 +104,29 @@ GMPVideoDecoderParent::InitDecode(const 
   }
   mIsOpen = true;
 
   // Async IPC, we don't have access to a return value.
   return NS_OK;
 }
 
 nsresult
-GMPVideoDecoderParent::Decode(UniquePtr<GMPVideoEncodedFrame> aInputFrame,
+GMPVideoDecoderParent::Decode(GMPUniquePtr<GMPVideoEncodedFrame> aInputFrame,
                               bool aMissingFrames,
                               const nsTArray<uint8_t>& aCodecSpecificInfo,
                               int64_t aRenderTimeMs)
 {
   if (!mIsOpen) {
     NS_WARNING("Trying to use an dead GMP video decoder");
     return NS_ERROR_FAILURE;
   }
 
   MOZ_ASSERT(mPlugin->GMPThread() == NS_GetCurrentThread());
 
-  UniquePtr<GMPVideoEncodedFrameImpl> inputFrameImpl(
+  GMPUniquePtr<GMPVideoEncodedFrameImpl> inputFrameImpl(
     static_cast<GMPVideoEncodedFrameImpl*>(aInputFrame.release()));
 
   // 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(GMPSharedMem::kGMPFrameData) > 3*GMPSharedMem::kGMPBufLimit) ||
       (NumInUse(GMPSharedMem::kGMPEncodedData) > GMPSharedMem::kGMPBufLimit)) {
     return NS_ERROR_FAILURE;
--- a/dom/media/gmp/GMPVideoDecoderParent.h
+++ b/dom/media/gmp/GMPVideoDecoderParent.h
@@ -6,16 +6,17 @@
 #ifndef GMPVideoDecoderParent_h_
 #define GMPVideoDecoderParent_h_
 
 #include "mozilla/RefPtr.h"
 #include "gmp-video-decode.h"
 #include "mozilla/gmp/PGMPVideoDecoderParent.h"
 #include "GMPMessageUtils.h"
 #include "GMPSharedMemManager.h"
+#include "GMPUtils.h"
 #include "GMPVideoHost.h"
 #include "GMPVideoDecoderProxy.h"
 
 namespace mozilla {
 namespace gmp {
 
 class GMPParent;
 
@@ -32,17 +33,17 @@ public:
   nsresult Shutdown();
 
   // GMPVideoDecoder
   virtual void Close() MOZ_OVERRIDE;
   virtual nsresult InitDecode(const GMPVideoCodec& aCodecSettings,
                               const nsTArray<uint8_t>& aCodecSpecific,
                               GMPVideoDecoderCallbackProxy* aCallback,
                               int32_t aCoreCount) MOZ_OVERRIDE;
-  virtual nsresult Decode(UniquePtr<GMPVideoEncodedFrame> aInputFrame,
+  virtual nsresult Decode(GMPUniquePtr<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 const uint64_t ParentID() MOZ_OVERRIDE { return reinterpret_cast<uint64_t>(mPlugin.get()); }
 
   // GMPSharedMemManager
--- a/dom/media/gmp/GMPVideoDecoderProxy.h
+++ b/dom/media/gmp/GMPVideoDecoderProxy.h
@@ -7,17 +7,17 @@
 #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"
-#include "mozilla/UniquePtr.h"
+#include "GMPUtils.h"
 
 class GMPVideoDecoderCallbackProxy : public GMPCallbackBase,
                                      public GMPVideoDecoderCallback
 {
 public:
   virtual ~GMPVideoDecoderCallbackProxy() {}
 };
 
@@ -33,35 +33,22 @@ public:
 
 // 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,
                               GMPVideoDecoderCallbackProxy* aCallback,
                               int32_t aCoreCount) = 0;
-  virtual nsresult Decode(mozilla::UniquePtr<GMPVideoEncodedFrame> aInputFrame,
+  virtual nsresult Decode(mozilla::GMPUniquePtr<GMPVideoEncodedFrame> aInputFrame,
                           bool aMissingFrames,
                           const nsTArray<uint8_t>& aCodecSpecificInfo,
                           int64_t aRenderTimeMs = -1) = 0;
   virtual nsresult Reset() = 0;
   virtual nsresult Drain() = 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;
 };
 
-namespace mozilla {
-
-template<>
-struct DefaultDelete<GMPVideoEncodedFrame>
-{
-  void operator()(GMPVideoEncodedFrame* aFrame) const
-  {
-    aFrame->Destroy();
-  }
-};
-
-} // namespace mozilla
-
 #endif
--- a/dom/media/gmp/GMPVideoEncodedFrameImpl.h
+++ b/dom/media/gmp/GMPVideoEncodedFrameImpl.h
@@ -30,17 +30,16 @@
 
 #ifndef GMPVideoEncodedFrameImpl_h_
 #define GMPVideoEncodedFrameImpl_h_
 
 #include "gmp-errors.h"
 #include "gmp-video-frame.h"
 #include "gmp-video-frame-encoded.h"
 #include "gmp-decryption.h"
-#include "mozilla/UniquePtr.h"
 #include "mozilla/ipc/Shmem.h"
 #include "mp4_demuxer/DecoderData.h"
 
 namespace mozilla {
 namespace gmp {
 
 class GMPVideoHostImpl;
 class GMPVideoEncodedFrameData;
@@ -111,20 +110,11 @@ private:
   GMPVideoHostImpl* mHost;
   ipc::Shmem mBuffer;
   GMPBufferType mBufferType;
   nsAutoPtr<GMPEncryptedBufferDataImpl> mCrypto;
 };
 
 } // namespace gmp
 
-template<>
-struct DefaultDelete<mozilla::gmp::GMPVideoEncodedFrameImpl>
-{
-  void operator()(mozilla::gmp::GMPVideoEncodedFrameImpl* aFrame) const
-  {
-    aFrame->Destroy();
-  }
-};
-
 } // namespace mozilla
 
 #endif // GMPVideoEncodedFrameImpl_h_
--- a/dom/media/gmp/GMPVideoEncoderParent.cpp
+++ b/dom/media/gmp/GMPVideoEncoderParent.cpp
@@ -10,16 +10,17 @@
 #include "mozilla/unused.h"
 #include "GMPMessageUtils.h"
 #include "nsAutoRef.h"
 #include "GMPParent.h"
 #include "mozilla/gmp/GMPTypes.h"
 #include "nsThread.h"
 #include "nsThreadUtils.h"
 #include "runnable_utils.h"
+#include "GMPUtils.h"
 
 namespace mozilla {
 
 #ifdef LOG
 #undef LOG
 #endif
 
 #ifdef PR_LOGGING
@@ -120,28 +121,28 @@ GMPVideoEncoderParent::InitEncode(const 
   }
   mIsOpen = true;
 
   // Async IPC, we don't have access to a return value.
   return GMPNoErr;
 }
 
 GMPErr
-GMPVideoEncoderParent::Encode(UniquePtr<GMPVideoi420Frame> aInputFrame,
+GMPVideoEncoderParent::Encode(GMPUniquePtr<GMPVideoi420Frame> aInputFrame,
                               const nsTArray<uint8_t>& aCodecSpecificInfo,
                               const nsTArray<GMPVideoFrameType>& aFrameTypes)
 {
   if (!mIsOpen) {
     NS_WARNING("Trying to use an dead GMP video encoder");
     return GMPGenericErr;
   }
 
   MOZ_ASSERT(mPlugin->GMPThread() == NS_GetCurrentThread());
 
-  UniquePtr<GMPVideoi420FrameImpl> inputFrameImpl(
+  GMPUniquePtr<GMPVideoi420FrameImpl> inputFrameImpl(
     static_cast<GMPVideoi420FrameImpl*>(aInputFrame.release()));
 
   // 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(GMPSharedMem::kGMPFrameData) > 3*GMPSharedMem::kGMPBufLimit) ||
       (NumInUse(GMPSharedMem::kGMPEncodedData) > GMPSharedMem::kGMPBufLimit)) {
     return GMPGenericErr;
--- a/dom/media/gmp/GMPVideoEncoderParent.h
+++ b/dom/media/gmp/GMPVideoEncoderParent.h
@@ -6,16 +6,17 @@
 #ifndef GMPVideoEncoderParent_h_
 #define GMPVideoEncoderParent_h_
 
 #include "mozilla/RefPtr.h"
 #include "gmp-video-encode.h"
 #include "mozilla/gmp/PGMPVideoEncoderParent.h"
 #include "GMPMessageUtils.h"
 #include "GMPSharedMemManager.h"
+#include "GMPUtils.h"
 #include "GMPVideoHost.h"
 #include "GMPVideoEncoderProxy.h"
 
 namespace mozilla {
 namespace gmp {
 
 class GMPParent;
 
@@ -33,17 +34,17 @@ public:
 
   // 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(UniquePtr<GMPVideoi420Frame> aInputFrame,
+  virtual GMPErr Encode(GMPUniquePtr<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 const uint64_t ParentID() MOZ_OVERRIDE { return reinterpret_cast<uint64_t>(mPlugin.get()); }
 
   // GMPSharedMemManager
--- a/dom/media/gmp/GMPVideoEncoderProxy.h
+++ b/dom/media/gmp/GMPVideoEncoderProxy.h
@@ -7,17 +7,17 @@
 #define GMPVideoEncoderProxy_h_
 
 #include "nsTArray.h"
 #include "gmp-video-encode.h"
 #include "gmp-video-frame-i420.h"
 #include "gmp-video-frame-encoded.h"
 
 #include "GMPCallbackBase.h"
-#include "mozilla/UniquePtr.h"
+#include "GMPUtils.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;
 };
@@ -35,35 +35,22 @@ public:
 // 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(mozilla::UniquePtr<GMPVideoi420Frame> aInputFrame,
+  virtual GMPErr Encode(mozilla::GMPUniquePtr<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 const uint64_t ParentID() = 0;
 
   // Call to tell GMP/plugin the consumer will no longer use this
   // interface/codec.
   virtual void Close() = 0;
 };
 
-namespace mozilla {
-
-template<>
-struct DefaultDelete<GMPVideoi420Frame>
-{
-  void operator()(GMPVideoi420Frame* aFrame) const
-  {
-    aFrame->Destroy();
-  }
-};
-
-} // namespace mozilla
-
 #endif // GMPVideoEncoderProxy_h_
--- a/dom/media/gmp/GMPVideoi420FrameImpl.h
+++ b/dom/media/gmp/GMPVideoi420FrameImpl.h
@@ -2,17 +2,16 @@
 /* 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 GMPVideoi420FrameImpl_h_
 #define GMPVideoi420FrameImpl_h_
 
 #include "gmp-video-frame-i420.h"
-#include "mozilla/UniquePtr.h"
 #include "mozilla/ipc/Shmem.h"
 #include "GMPVideoPlaneImpl.h"
 
 namespace mozilla {
 namespace gmp {
 
 class GMPVideoi420FrameData;
 
@@ -75,20 +74,11 @@ private:
   int32_t mWidth;
   int32_t mHeight;
   uint64_t mTimestamp;
   uint64_t mDuration;
 };
 
 } // namespace gmp
 
-template<>
-struct DefaultDelete<mozilla::gmp::GMPVideoi420FrameImpl>
-{
-  void operator()(mozilla::gmp::GMPVideoi420FrameImpl* aFrame) const
-  {
-    aFrame->Destroy();
-  }
-};
-
 } // namespace mozilla
 
 #endif // GMPVideoi420FrameImpl_h_
--- a/dom/media/gmp/moz.build
+++ b/dom/media/gmp/moz.build
@@ -46,16 +46,17 @@ EXPORTS += [
     'GMPProcessChild.h',
     'GMPProcessParent.h',
     'GMPService.h',
     'GMPSharedMemManager.h',
     'GMPStorageChild.h',
     'GMPStorageParent.h',
     'GMPTimerChild.h',
     'GMPTimerParent.h',
+    'GMPUtils.h',
     'GMPVideoDecoderChild.h',
     'GMPVideoDecoderParent.h',
     'GMPVideoDecoderProxy.h',
     'GMPVideoEncodedFrameImpl.h',
     'GMPVideoEncoderChild.h',
     'GMPVideoEncoderParent.h',
     'GMPVideoEncoderProxy.h',
     'GMPVideoHost.h',
--- a/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp
+++ b/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp
@@ -274,17 +274,17 @@ WebrtcGmpVideoEncoder::Encode_g(const we
     }
   }
 
   GMPVideoFrame* ftmp = nullptr;
   GMPErr err = mHost->CreateFrame(kGMPI420VideoFrame, &ftmp);
   if (err != GMPNoErr) {
     return WEBRTC_VIDEO_CODEC_ERROR;
   }
-  UniquePtr<GMPVideoi420Frame> frame(static_cast<GMPVideoi420Frame*>(ftmp));
+  GMPUniquePtr<GMPVideoi420Frame> frame(static_cast<GMPVideoi420Frame*>(ftmp));
 
   err = frame->CreateFrame(aInputImage->allocated_size(webrtc::kYPlane),
                            aInputImage->buffer(webrtc::kYPlane),
                            aInputImage->allocated_size(webrtc::kUPlane),
                            aInputImage->buffer(webrtc::kUPlane),
                            aInputImage->allocated_size(webrtc::kVPlane),
                            aInputImage->buffer(webrtc::kVPlane),
                            aInputImage->width(),
@@ -630,17 +630,17 @@ WebrtcGmpVideoDecoder::Decode_g(const we
   }
 
   GMPVideoFrame* ftmp = nullptr;
   GMPErr err = mHost->CreateFrame(kGMPEncodedVideoFrame, &ftmp);
   if (err != GMPNoErr) {
     return WEBRTC_VIDEO_CODEC_ERROR;
   }
 
-  UniquePtr<GMPVideoEncodedFrame> frame(static_cast<GMPVideoEncodedFrame*>(ftmp));
+  GMPUniquePtr<GMPVideoEncodedFrame> frame(static_cast<GMPVideoEncodedFrame*>(ftmp));
   err = frame->CreateEmptyFrame(aInputImage._length);
   if (err != GMPNoErr) {
     return WEBRTC_VIDEO_CODEC_ERROR;
   }
 
   // XXX At this point, we only will get mode1 data (a single length and a buffer)
   // Session_info.cc/etc code needs to change to support mode 0.
   *(reinterpret_cast<uint32_t*>(frame->Buffer())) = frame->Size();