Bug 1550737 - Make sure we keed strong reference until the end of the method. r=jya.
authorAlex Chronopoulos <achronop@gmail.com>
Thu, 16 May 2019 14:07:42 +0000
changeset 474067 d4bbc018fe902044fb90f08b6dc2c0a0d172423d
parent 474066 41f28d23024ebecf5445896cd3b6b39a6b650a9d
child 474068 5ba6d20ec80ad5dcabead733c7bd77df18aa05d7
push id36022
push userncsoregi@mozilla.com
push dateThu, 16 May 2019 21:55:16 +0000
treeherdermozilla-central@96802be91766 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjya
bugs1550737
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 1550737 - Make sure we keed strong reference until the end of the method. r=jya. Instead of deleteing a RefPtr directly copy it in a local variable in order to ensure that the pointer will be alive till the end of the method. In addition to that, on RemoteMediaDataDecpder::Shutdown promise use a reference of the child object instead of the whole `self` object since this is the only one needed. Finally, one style change. Differential Revision: https://phabricator.services.mozilla.com/D30637
dom/media/ipc/RemoteDecoderChild.cpp
dom/media/ipc/VideoDecoderChild.cpp
--- a/dom/media/ipc/RemoteDecoderChild.cpp
+++ b/dom/media/ipc/RemoteDecoderChild.cpp
@@ -32,18 +32,19 @@ mozilla::ipc::IPCResult RemoteDecoderChi
 }
 
 mozilla::ipc::IPCResult RemoteDecoderChild::RecvError(const nsresult& aError) {
   AssertOnManagerThread();
   mDecodedData = MediaDataDecoder::DecodedData();
   mDecodePromise.RejectIfExists(aError, __func__);
   mDrainPromise.RejectIfExists(aError, __func__);
   mFlushPromise.RejectIfExists(aError, __func__);
-  mShutdownSelfRef = nullptr;
   mShutdownPromise.ResolveIfExists(true, __func__);
+  RefPtr<RemoteDecoderChild> kungFuDeathGrip = mShutdownSelfRef.forget();
+  Unused << kungFuDeathGrip;
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult RemoteDecoderChild::RecvInitComplete(
     const TrackInfo::TrackType& trackType, const nsCString& aDecoderDescription,
     const ConversionRequired& aConversion) {
   AssertOnManagerThread();
   mInitPromise.ResolveIfExists(trackType, __func__);
@@ -64,31 +65,33 @@ mozilla::ipc::IPCResult RemoteDecoderChi
   AssertOnManagerThread();
   mFlushPromise.ResolveIfExists(true, __func__);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult RemoteDecoderChild::RecvShutdownComplete() {
   AssertOnManagerThread();
   MOZ_ASSERT(mShutdownSelfRef);
-  mShutdownSelfRef = nullptr;
   mShutdownPromise.ResolveIfExists(true, __func__);
+  RefPtr<RemoteDecoderChild> kungFuDeathGrip = mShutdownSelfRef.forget();
+  Unused << kungFuDeathGrip;
   return IPC_OK();
 }
 
 void RemoteDecoderChild::ActorDestroy(ActorDestroyReason aWhy) {
   MOZ_ASSERT(mCanSend);
   // If the IPC channel is gone pending promises need to be resolved/rejected.
   if (aWhy == AbnormalShutdown) {
     MediaResult error(NS_ERROR_DOM_MEDIA_DECODE_ERR);
     mDecodePromise.RejectIfExists(error, __func__);
     mDrainPromise.RejectIfExists(error, __func__);
     mFlushPromise.RejectIfExists(error, __func__);
-    mShutdownSelfRef = nullptr;
     mShutdownPromise.ResolveIfExists(true, __func__);
+    RefPtr<RemoteDecoderChild> kungFuDeathGrip = mShutdownSelfRef.forget();
+    Unused << kungFuDeathGrip;
   }
   mCanSend = false;
 }
 
 void RemoteDecoderChild::DestroyIPDL() {
   AssertOnManagerThread();
   if (mCanSend) {
     PRemoteDecoderChild::Send__delete__(this);
@@ -130,18 +133,17 @@ RefPtr<MediaDataDecoder::DecodePromise> 
         NS_ERROR_DOM_MEDIA_DECODE_ERR, __func__);
   }
 
   memcpy(buffer.get<uint8_t>(), aSample->Data(), aSample->Size());
 
   MediaRawDataIPDL sample(
       MediaDataIPDL(aSample->mOffset, aSample->mTime, aSample->mTimecode,
                     aSample->mDuration, aSample->mKeyframe),
-      aSample->mEOS,
-      std::move(buffer));
+      aSample->mEOS, std::move(buffer));
   SendInput(sample);
 
   return mDecodePromise.Ensure(__func__);
 }
 
 RefPtr<MediaDataDecoder::FlushPromise> RemoteDecoderChild::Flush() {
   AssertOnManagerThread();
   mDecodePromise.RejectIfExists(NS_ERROR_DOM_MEDIA_CANCELED, __func__);
--- a/dom/media/ipc/VideoDecoderChild.cpp
+++ b/dom/media/ipc/VideoDecoderChild.cpp
@@ -81,18 +81,19 @@ mozilla::ipc::IPCResult VideoDecoderChil
 }
 
 mozilla::ipc::IPCResult VideoDecoderChild::RecvError(const nsresult& aError) {
   AssertOnManagerThread();
   mDecodedData = MediaDataDecoder::DecodedData();
   mDecodePromise.RejectIfExists(aError, __func__);
   mDrainPromise.RejectIfExists(aError, __func__);
   mFlushPromise.RejectIfExists(aError, __func__);
-  mShutdownSelfRef = nullptr;
   mShutdownPromise.ResolveIfExists(true, __func__);
+  RefPtr<VideoDecoderChild> kungFuDeathGrip = mShutdownSelfRef.forget();
+  Unused << kungFuDeathGrip;
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult VideoDecoderChild::RecvInitComplete(
     const nsCString& aDecoderDescription, const bool& aHardware,
     const nsCString& aHardwareReason, const uint32_t& aConversion) {
   AssertOnManagerThread();
   mInitPromise.ResolveIfExists(TrackInfo::kVideoTrack, __func__);
@@ -115,39 +116,42 @@ mozilla::ipc::IPCResult VideoDecoderChil
   AssertOnManagerThread();
   mFlushPromise.ResolveIfExists(true, __func__);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult VideoDecoderChild::RecvShutdownComplete() {
   AssertOnManagerThread();
   MOZ_ASSERT(mShutdownSelfRef);
-  mShutdownSelfRef = nullptr;
   mShutdownPromise.ResolveIfExists(true, __func__);
+  RefPtr<VideoDecoderChild> kungFuDeathGrip = mShutdownSelfRef.forget();
+  Unused << kungFuDeathGrip;
   return IPC_OK();
 }
 
 void VideoDecoderChild::ActorDestroy(ActorDestroyReason aWhy) {
   if (aWhy == AbnormalShutdown) {
     // GPU process crashed, record the time and send back to MFR for telemetry.
     mGPUCrashTime = TimeStamp::Now();
 
     // Defer reporting an error until we've recreated the manager so that
     // it'll be safe for MediaFormatReader to recreate decoders
     RefPtr<VideoDecoderChild> ref = this;
+    // Make sure shutdown self reference is null. Since ref is captured by the
+    // lambda it is not necessary to keep it any longer.
+    mShutdownSelfRef = nullptr;
     GetManager()->RunWhenRecreated(
         NS_NewRunnableFunction("VideoDecoderChild::ActorDestroy", [=]() {
           MediaResult error(NS_ERROR_DOM_MEDIA_NEED_NEW_DECODER);
           error.SetGPUCrashTimeStamp(ref->mGPUCrashTime);
           if (ref->mInitialized) {
             mDecodedData = MediaDataDecoder::DecodedData();
             mDecodePromise.RejectIfExists(error, __func__);
             mDrainPromise.RejectIfExists(error, __func__);
             mFlushPromise.RejectIfExists(error, __func__);
-            mShutdownSelfRef = nullptr;
             mShutdownPromise.ResolveIfExists(true, __func__);
             // Make sure the next request will be rejected accordingly if ever
             // called.
             mNeedNewDecoder = true;
           } else {
             ref->mInitPromise.RejectIfExists(error, __func__);
           }
         }));
@@ -251,18 +255,17 @@ RefPtr<MediaDataDecoder::DecodePromise> 
         NS_ERROR_DOM_MEDIA_DECODE_ERR, __func__);
   }
 
   memcpy(buffer.get<uint8_t>(), aSample->Data(), aSample->Size());
 
   MediaRawDataIPDL sample(
       MediaDataIPDL(aSample->mOffset, aSample->mTime, aSample->mTimecode,
                     aSample->mDuration, aSample->mKeyframe),
-      aSample->mEOS,
-      std::move(buffer));
+      aSample->mEOS, std::move(buffer));
   SendInput(sample);
   return mDecodePromise.Ensure(__func__);
 }
 
 RefPtr<MediaDataDecoder::FlushPromise> VideoDecoderChild::Flush() {
   AssertOnManagerThread();
   mDecodePromise.RejectIfExists(NS_ERROR_DOM_MEDIA_CANCELED, __func__);
   mDrainPromise.RejectIfExists(NS_ERROR_DOM_MEDIA_CANCELED, __func__);