Bug 1530305 - handle failure to alloc Shmem in RemoteVideoDecoderParent. r=jya
authorMichael Froman <mfroman@mozilla.com>
Tue, 19 Mar 2019 15:56:52 +0000
changeset 465058 beaf8e80224d73c9aa5058c579073dd99f834357
parent 465057 85bd45178007470651a23a54a416ac9208b1d531
child 465059 ce4539262b1bc2357300080f866457b6fa274748
push id35730
push userrmaries@mozilla.com
push dateTue, 19 Mar 2019 21:51:47 +0000
treeherdermozilla-central@4f6d8ed9e948 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjya
bugs1530305
milestone68.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 1530305 - handle failure to alloc Shmem in RemoteVideoDecoderParent. r=jya - Modify ProcessDecodedData to return a MediaResult. - RemoteDecoderParent::RecvInput and RemoteDecoderParent::RecvDrain both use error returned from ProcessDecodedData to call SendError. - RemoteVideoDecoderParent and RemoteAudioDecoderParent both return MediaResult with NS_ERROR_OUT_OF_MEMORY if AllocShmem fails in ProcessDecodedData (or if the returned buffer size is less than the requested size). Differential Revision: https://phabricator.services.mozilla.com/D23988
dom/media/ipc/RemoteAudioDecoder.cpp
dom/media/ipc/RemoteAudioDecoder.h
dom/media/ipc/RemoteDecoderParent.cpp
dom/media/ipc/RemoteDecoderParent.h
dom/media/ipc/RemoteVideoDecoder.cpp
dom/media/ipc/RemoteVideoDecoder.h
--- a/dom/media/ipc/RemoteAudioDecoder.cpp
+++ b/dom/media/ipc/RemoteAudioDecoder.cpp
@@ -85,39 +85,49 @@ RemoteAudioDecoderParent::RemoteAudioDec
   if (NS_FAILED(error)) {
     MOZ_ASSERT(aErrorDescription);
     *aErrorDescription = error.Description();
   }
 
   *aSuccess = !!mDecoder;
 }
 
-void RemoteAudioDecoderParent::ProcessDecodedData(
+MediaResult RemoteAudioDecoderParent::ProcessDecodedData(
     const MediaDataDecoder::DecodedData& aData) {
   MOZ_ASSERT(OnManagerThread());
 
   for (const auto& data : aData) {
     MOZ_ASSERT(data->mType == MediaData::Type::AUDIO_DATA,
                "Can only decode audio using RemoteAudioDecoderParent!");
     AudioData* audio = static_cast<AudioData*>(data.get());
 
     MOZ_ASSERT(audio->Data().Elements(),
                "Decoded audio must output an AlignedAudioBuffer "
                "to be used with RemoteAudioDecoderParent");
 
     Shmem buffer;
-    if (AllocShmem(audio->Data().Length() * sizeof(AudioDataValue),
-                   Shmem::SharedMemory::TYPE_BASIC, &buffer) &&
-        audio->Data().Length() == buffer.Size<AudioDataValue>()) {
-      PodCopy(buffer.get<AudioDataValue>(), audio->Data().Elements(),
-              audio->Data().Length());
+    if (!AllocShmem(audio->Data().Length() * sizeof(AudioDataValue),
+                    Shmem::SharedMemory::TYPE_BASIC, &buffer)) {
+      return MediaResult(NS_ERROR_OUT_OF_MEMORY,
+                         "AllocShmem failed in "
+                         "RemoteAudioDecoderParent::ProcessDecodedData");
     }
+    if (audio->Data().Length() > buffer.Size<AudioDataValue>()) {
+      return MediaResult(NS_ERROR_OUT_OF_MEMORY,
+                         "AllocShmem returned less than requested in "
+                         "RemoteAudioDecoderParent::ProcessDecodedData");
+    }
+
+    PodCopy(buffer.get<AudioDataValue>(), audio->Data().Elements(),
+            audio->Data().Length());
 
     RemoteAudioDataIPDL output(
         MediaDataIPDL(data->mOffset, data->mTime, data->mTimecode,
                       data->mDuration, data->mKeyframe),
         audio->mChannels, audio->mRate, audio->mChannelMap, std::move(buffer));
 
     Unused << SendOutput(output);
   }
+
+  return NS_OK;
 }
 
 }  // namespace mozilla
--- a/dom/media/ipc/RemoteAudioDecoder.h
+++ b/dom/media/ipc/RemoteAudioDecoder.h
@@ -28,17 +28,18 @@ class RemoteAudioDecoderParent final : p
   RemoteAudioDecoderParent(RemoteDecoderManagerParent* aParent,
                            const AudioInfo& aAudioInfo,
                            const CreateDecoderParams::OptionSet& aOptions,
                            TaskQueue* aManagerTaskQueue,
                            TaskQueue* aDecodeTaskQueue, bool* aSuccess,
                            nsCString* aErrorDescription);
 
  protected:
-  void ProcessDecodedData(const MediaDataDecoder::DecodedData& aData) override;
+  MediaResult ProcessDecodedData(
+      const MediaDataDecoder::DecodedData& aData) override;
 
  private:
   // Can only be accessed from the manager thread
   // Note: we can't keep a reference to the original AudioInfo here
   // because unlike in typical MediaDataDecoder situations, we're being
   // passed a deserialized AudioInfo from RecvPRemoteDecoderConstructor
   // which is destroyed when RecvPRemoteDecoderConstructor returns.
   const AudioInfo mAudioInfo;
--- a/dom/media/ipc/RemoteDecoderParent.cpp
+++ b/dom/media/ipc/RemoteDecoderParent.cpp
@@ -81,17 +81,21 @@ mozilla::ipc::IPCResult RemoteDecoderPar
 
   RefPtr<RemoteDecoderParent> self = this;
   mDecoder->Decode(data)->Then(
       mManagerTaskQueue, __func__,
       [self, this](const MediaDataDecoder::DecodedData& aResults) {
         if (mDestroyed) {
           return;
         }
-        ProcessDecodedData(aResults);
+        MediaResult res = ProcessDecodedData(aResults);
+        if (res != NS_OK) {
+          self->Error(res);
+          return;
+        }
         Unused << SendInputExhausted();
       },
       [self](const MediaResult& aError) { self->Error(aError); });
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult RemoteDecoderParent::RecvFlush() {
   MOZ_ASSERT(!mDestroyed);
@@ -112,17 +116,21 @@ mozilla::ipc::IPCResult RemoteDecoderPar
 mozilla::ipc::IPCResult RemoteDecoderParent::RecvDrain() {
   MOZ_ASSERT(!mDestroyed);
   MOZ_ASSERT(OnManagerThread());
   RefPtr<RemoteDecoderParent> self = this;
   mDecoder->Drain()->Then(
       mManagerTaskQueue, __func__,
       [self, this](const MediaDataDecoder::DecodedData& aResults) {
         if (!mDestroyed) {
-          ProcessDecodedData(aResults);
+          MediaResult res = ProcessDecodedData(aResults);
+          if (res != NS_OK) {
+            self->Error(res);
+            return;
+          }
           Unused << SendDrainComplete();
         }
       },
       [self](const MediaResult& aError) { self->Error(aError); });
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult RemoteDecoderParent::RecvShutdown() {
--- a/dom/media/ipc/RemoteDecoderParent.h
+++ b/dom/media/ipc/RemoteDecoderParent.h
@@ -37,17 +37,17 @@ class RemoteDecoderParent : public PRemo
   void ActorDestroy(ActorDestroyReason aWhy) override;
 
  protected:
   virtual ~RemoteDecoderParent();
 
   bool OnManagerThread();
   void Error(const MediaResult& aError);
 
-  virtual void ProcessDecodedData(
+  virtual MediaResult ProcessDecodedData(
       const MediaDataDecoder::DecodedData& aData) = 0;
 
   RefPtr<RemoteDecoderManagerParent> mParent;
   RefPtr<RemoteDecoderParent> mIPDLSelfRef;
   RefPtr<TaskQueue> mManagerTaskQueue;
   RefPtr<TaskQueue> mDecodeTaskQueue;
   RefPtr<MediaDataDecoder> mDecoder;
 
--- a/dom/media/ipc/RemoteVideoDecoder.cpp
+++ b/dom/media/ipc/RemoteVideoDecoder.cpp
@@ -158,17 +158,17 @@ RemoteVideoDecoderParent::RemoteVideoDec
   if (NS_FAILED(error)) {
     MOZ_ASSERT(aErrorDescription);
     *aErrorDescription = error.Description();
   }
 
   *aSuccess = !!mDecoder;
 }
 
-void RemoteVideoDecoderParent::ProcessDecodedData(
+MediaResult RemoteVideoDecoderParent::ProcessDecodedData(
     const MediaDataDecoder::DecodedData& aData) {
   MOZ_ASSERT(OnManagerThread());
 
   for (const auto& data : aData) {
     MOZ_ASSERT(data->mType == MediaData::Type::VIDEO_DATA,
                "Can only decode videos using RemoteDecoderParent!");
     VideoData* video = static_cast<VideoData*>(data.get());
 
@@ -176,24 +176,34 @@ void RemoteVideoDecoderParent::ProcessDe
                "Decoded video must output a layer::Image to "
                "be used with RemoteDecoderParent");
 
     PlanarYCbCrImage* image =
         static_cast<PlanarYCbCrImage*>(video->mImage.get());
 
     SurfaceDescriptorBuffer sdBuffer;
     Shmem buffer;
-    if (AllocShmem(image->GetDataSize(), Shmem::SharedMemory::TYPE_BASIC,
-                   &buffer) &&
-        image->GetDataSize() == buffer.Size<uint8_t>()) {
-      sdBuffer.data() = std::move(buffer);
-      image->BuildSurfaceDescriptorBuffer(sdBuffer);
+    if (!AllocShmem(image->GetDataSize(), Shmem::SharedMemory::TYPE_BASIC,
+                    &buffer)) {
+      return MediaResult(NS_ERROR_OUT_OF_MEMORY,
+                         "AllocShmem failed in "
+                         "RemoteVideoDecoderParent::ProcessDecodedData");
     }
+    if (image->GetDataSize() > buffer.Size<uint8_t>()) {
+      return MediaResult(NS_ERROR_OUT_OF_MEMORY,
+                         "AllocShmem returned less than requested in "
+                         "RemoteVideoDecoderParent::ProcessDecodedData");
+    }
+
+    sdBuffer.data() = std::move(buffer);
+    image->BuildSurfaceDescriptorBuffer(sdBuffer);
 
     RemoteVideoDataIPDL output(
         MediaDataIPDL(data->mOffset, data->mTime, data->mTimecode,
                       data->mDuration, data->mKeyframe),
         video->mDisplay, image->GetSize(), sdBuffer, video->mFrameID);
     Unused << SendOutput(output);
   }
+
+  return NS_OK;
 }
 
 }  // namespace mozilla
--- a/dom/media/ipc/RemoteVideoDecoder.h
+++ b/dom/media/ipc/RemoteVideoDecoder.h
@@ -40,17 +40,18 @@ class RemoteVideoDecoderParent final : p
   RemoteVideoDecoderParent(RemoteDecoderManagerParent* aParent,
                            const VideoInfo& aVideoInfo, float aFramerate,
                            const CreateDecoderParams::OptionSet& aOptions,
                            TaskQueue* aManagerTaskQueue,
                            TaskQueue* aDecodeTaskQueue, bool* aSuccess,
                            nsCString* aErrorDescription);
 
  protected:
-  void ProcessDecodedData(const MediaDataDecoder::DecodedData& aData) override;
+  MediaResult ProcessDecodedData(
+      const MediaDataDecoder::DecodedData& aData) override;
 
  private:
   // Can only be accessed from the manager thread
   // Note: we can't keep a reference to the original VideoInfo here
   // because unlike in typical MediaDataDecoder situations, we're being
   // passed a deserialized VideoInfo from RecvPRemoteDecoderConstructor
   // which is destroyed when RecvPRemoteDecoderConstructor returns.
   const VideoInfo mVideoInfo;