Bug 1353030: document use of WrapRunnable(this) r=cpearce
authorRandell Jesup <rjesup@jesup.org>
Fri, 02 Jun 2017 16:36:34 -0400
changeset 362171 39f8ae0ea97f639da72f335ce644e7fc7a55331f
parent 362170 0cec7ba89ffd1266684b3b974c439a0e0b60d26b
child 362172 8aee9eb0b3495ec3aa3ee4894cd52a7ebaf9896b
push id31961
push userarchaeopteryx@coole-files.de
push dateSat, 03 Jun 2017 18:15:59 +0000
treeherdermozilla-central@130efc657df7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs1353030
milestone55.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 1353030: document use of WrapRunnable(this) r=cpearce MozReview-Commit-ID: Fb3KjsI9tE3
dom/media/gmp/GMPServiceParent.cpp
media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
media/webrtc/signaling/src/peerconnection/WebrtcGlobalInformation.cpp
--- a/dom/media/gmp/GMPServiceParent.cpp
+++ b/dom/media/gmp/GMPServiceParent.cpp
@@ -268,17 +268,21 @@ GeckoMediaPluginServiceParent::Observe(n
       }
       if (crashNow) {
         nsCOMPtr<nsIThread> gmpThread;
         {
           MutexAutoLock lock(mMutex);
           gmpThread = mGMPThread;
         }
         if (gmpThread) {
-          gmpThread->Dispatch(WrapRunnable(this,
+          // Note: the GeckoMediaPluginServiceParent singleton is kept alive by a
+          // static refptr that is only cleared in the final stage of
+          // shutdown after everything else is shutdown, so this RefPtr<> is not
+          // strictly necessary so long as that is true, but it's safer.
+          gmpThread->Dispatch(WrapRunnable(RefPtr<GeckoMediaPluginServiceParent>(this),
                                            &GeckoMediaPluginServiceParent::CrashPlugins),
                               NS_DISPATCH_NORMAL);
         }
       }
     }
   } else if (!strcmp("profile-change-teardown", aTopic)) {
     mWaitingForPluginsSyncShutdown = true;
 
--- a/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp
+++ b/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp
@@ -304,16 +304,17 @@ WebrtcGmpVideoEncoder::InitEncoderForSiz
 int32_t
 WebrtcGmpVideoEncoder::Encode(const webrtc::VideoFrame& aInputImage,
                               const webrtc::CodecSpecificInfo* aCodecSpecificInfo,
                               const std::vector<webrtc::FrameType>* aFrameTypes)
 {
   MOZ_ASSERT(aInputImage.width() >= 0 && aInputImage.height() >= 0);
   // Would be really nice to avoid this sync dispatch, but it would require a
   // copy of the frame, since it doesn't appear to actually have a refcount.
+  // Passing 'this' is safe since this is synchronous.
   mGMPThread->Dispatch(
       WrapRunnable(this,
                    &WebrtcGmpVideoEncoder::Encode_g,
                    &aInputImage,
                    aCodecSpecificInfo,
                    aFrameTypes),
       NS_DISPATCH_SYNC);
 
@@ -771,16 +772,17 @@ WebrtcGmpVideoDecoder::Decode(const webr
                               const webrtc::CodecSpecificInfo* aCodecSpecificInfo,
                               int64_t aRenderTimeMs)
 {
   int32_t ret;
   MOZ_ASSERT(mGMPThread);
   MOZ_ASSERT(!NS_IsMainThread());
   // Would be really nice to avoid this sync dispatch, but it would require a
   // copy of the frame, since it doesn't appear to actually have a refcount.
+  // Passing 'this' is safe since this is synchronous.
   mozilla::SyncRunnable::DispatchToThread(mGMPThread,
                 WrapRunnableRet(&ret, this,
                                 &WebrtcGmpVideoDecoder::Decode_g,
                                 aInputImage,
                                 aMissingFrames,
                                 aFragmentation,
                                 aCodecSpecificInfo,
                                 aRenderTimeMs));
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
@@ -1132,16 +1132,17 @@ PeerConnectionMedia::ShutdownMediaTransp
   Telemetry::ScalarAdd(Telemetry::ScalarID::WEBRTC_NICER_TURN_403S,
                        stats.turn_403s);
   Telemetry::ScalarAdd(Telemetry::ScalarID::WEBRTC_NICER_TURN_438S,
                        stats.turn_438s);
 #endif
 
   mIceCtxHdlr = nullptr;
 
+  // we're holding a ref to 'this' that's released by SelfDestruct_m
   mMainThread->Dispatch(WrapRunnable(this, &PeerConnectionMedia::SelfDestruct_m),
                         NS_DISPATCH_NORMAL);
 }
 
 LocalSourceStreamInfo*
 PeerConnectionMedia::GetLocalStreamByIndex(int aIndex)
 {
   ASSERT_ON_THREAD(mMainThread);
--- a/media/webrtc/signaling/src/peerconnection/WebrtcGlobalInformation.cpp
+++ b/media/webrtc/signaling/src/peerconnection/WebrtcGlobalInformation.cpp
@@ -868,16 +868,18 @@ WebrtcGlobalChild::RecvGetLogRequest(con
     return IPC_OK();
   }
 
   nsresult rv;
   nsCOMPtr<nsIEventTarget> stsThread =
     do_GetService(NS_SOCKETTRANSPORTSERVICE_CONTRACTID, &rv);
 
   if (NS_SUCCEEDED(rv) && stsThread) {
+    // this is a singleton, so we shouldn't need to hold a ref for the
+    // request (and can't just add a ref here anyways)
     rv = RUN_ON_THREAD(stsThread,
                        WrapRunnableNM(&GetLogging_s, this, aRequestId, aPattern.get()),
                        NS_DISPATCH_NORMAL);
 
     if (NS_SUCCEEDED(rv)) {
       return IPC_OK();
     }
   }