Bug 1336510 - Part 6: Avoid AddRefing/Releasing CamerasChild on the wrong thread, r=jwwang
authorMichael Layzell <michael@thelayzells.com>
Thu, 16 Feb 2017 14:33:53 -0500
changeset 373842 b4cb7a1e84a85fe979bec7263c1a7c71a4a5323c
parent 373841 54cc45b7dd55efb22294d257dab2d1ada338e2d6
child 373843 a545bcb544a738353f521593643f8ed3c5924c5b
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwwang
bugs1336510
milestone54.0a1
Bug 1336510 - Part 6: Avoid AddRefing/Releasing CamerasChild on the wrong thread, r=jwwang MozReview-Commit-ID: 8b5KK7sL6wb
dom/media/ogg/OggDemuxer.cpp
dom/media/systemservices/CamerasChild.cpp
--- a/dom/media/ogg/OggDemuxer.cpp
+++ b/dom/media/ogg/OggDemuxer.cpp
@@ -123,19 +123,23 @@ OggDemuxer::~OggDemuxer()
 {
   MOZ_COUNT_DTOR(OggDemuxer);
   Reset(TrackInfo::kAudioTrack);
   Reset(TrackInfo::kVideoTrack);
   if (HasAudio() || HasVideo()) {
     // If we were able to initialize our decoders, report whether we encountered
     // a chained stream or not.
     bool isChained = mIsChained;
-    RefPtr<OggDemuxer> self = this;
-    nsCOMPtr<nsIRunnable> task = NS_NewRunnableFunction([this, self, isChained]() -> void {
-      OGG_DEBUG("Reporting telemetry MEDIA_OGG_LOADED_IS_CHAINED=%d", isChained);
+    void* ptr = this;
+    nsCOMPtr<nsIRunnable> task = NS_NewRunnableFunction([ptr, isChained]() -> void {
+      // We can't use OGG_DEBUG here because it implicitly refers to `this`,
+      // which we can't capture in this runnable.
+      MOZ_LOG(gMediaDemuxerLog, mozilla::LogLevel::Debug,
+              ("OggDemuxer(%p)::%s: Reporting telemetry MEDIA_OGG_LOADED_IS_CHAINED=%d",
+               ptr, __func__, isChained));
       Telemetry::Accumulate(Telemetry::HistogramID::MEDIA_OGG_LOADED_IS_CHAINED, isChained);
     });
     // Non-DocGroup version of AbstractThread::MainThread is fine for Telemetry.
     AbstractThread::MainThread()->Dispatch(task.forget());
   }
 }
 
 void
--- a/dom/media/systemservices/CamerasChild.cpp
+++ b/dom/media/systemservices/CamerasChild.cpp
@@ -279,41 +279,31 @@ CamerasChild::DispatchToParent(nsIRunnab
 
 int
 CamerasChild::NumberOfCapabilities(CaptureEngine aCapEngine,
                                    const char* deviceUniqueIdUTF8)
 {
   LOG((__PRETTY_FUNCTION__));
   LOG(("NumberOfCapabilities for %s", deviceUniqueIdUTF8));
   nsCString unique_id(deviceUniqueIdUTF8);
-  RefPtr<CamerasChild> self = this;
   nsCOMPtr<nsIRunnable> runnable =
-    media::NewRunnableFrom([self, aCapEngine, unique_id]() -> nsresult {
-      if (self->SendNumberOfCapabilities(aCapEngine, unique_id)) {
-        return NS_OK;
-      }
-      return NS_ERROR_FAILURE;
-    });
+    mozilla::NewNonOwningRunnableMethod<CaptureEngine, nsCString>
+    (this, &CamerasChild::SendNumberOfCapabilities, aCapEngine, unique_id);
   LockAndDispatch<> dispatcher(this, __func__, runnable, 0, mReplyInteger);
   LOG(("Capture capability count: %d", dispatcher.ReturnValue()));
   return dispatcher.ReturnValue();
 }
 
 int
 CamerasChild::NumberOfCaptureDevices(CaptureEngine aCapEngine)
 {
   LOG((__PRETTY_FUNCTION__));
-  RefPtr<CamerasChild> self = this;
   nsCOMPtr<nsIRunnable> runnable =
-    media::NewRunnableFrom([self, aCapEngine]() -> nsresult {
-      if (self->SendNumberOfCaptureDevices(aCapEngine)) {
-        return NS_OK;
-      }
-      return NS_ERROR_FAILURE;
-    });
+    mozilla::NewNonOwningRunnableMethod<CaptureEngine>
+    (this, &CamerasChild::SendNumberOfCaptureDevices, aCapEngine);
   LockAndDispatch<> dispatcher(this, __func__, runnable, 0, mReplyInteger);
   LOG(("Capture Devices: %d", dispatcher.ReturnValue()));
   return dispatcher.ReturnValue();
 }
 
 mozilla::ipc::IPCResult
 CamerasChild::RecvReplyNumberOfCaptureDevices(const int& numdev)
 {
@@ -325,45 +315,35 @@ CamerasChild::RecvReplyNumberOfCaptureDe
   monitor.Notify();
   return IPC_OK();
 }
 
 int
 CamerasChild::EnsureInitialized(CaptureEngine aCapEngine)
 {
   LOG((__PRETTY_FUNCTION__));
-  RefPtr<CamerasChild> self = this;
   nsCOMPtr<nsIRunnable> runnable =
-    media::NewRunnableFrom([self, aCapEngine]() -> nsresult {
-      if (self->SendEnsureInitialized(aCapEngine)) {
-        return NS_OK;
-      }
-      return NS_ERROR_FAILURE;
-    });
+    mozilla::NewNonOwningRunnableMethod<CaptureEngine>
+    (this, &CamerasChild::SendEnsureInitialized, aCapEngine);
   LockAndDispatch<> dispatcher(this, __func__, runnable, 0, mReplyInteger);
   LOG(("Capture Devices: %d", dispatcher.ReturnValue()));
   return dispatcher.ReturnValue();
 }
 
 int
 CamerasChild::GetCaptureCapability(CaptureEngine aCapEngine,
                                    const char* unique_idUTF8,
                                    const unsigned int capability_number,
                                    webrtc::VideoCaptureCapability& capability)
 {
   LOG(("GetCaptureCapability: %s %d", unique_idUTF8, capability_number));
   nsCString unique_id(unique_idUTF8);
-  RefPtr<CamerasChild> self = this;
   nsCOMPtr<nsIRunnable> runnable =
-    media::NewRunnableFrom([self, aCapEngine, unique_id, capability_number]() -> nsresult {
-      if (self->SendGetCaptureCapability(aCapEngine, unique_id, capability_number)) {
-        return NS_OK;
-      }
-      return NS_ERROR_FAILURE;
-    });
+    mozilla::NewNonOwningRunnableMethod<CaptureEngine, nsCString, unsigned int>
+    (this, &CamerasChild::SendGetCaptureCapability, aCapEngine, unique_id, capability_number);
   LockAndDispatch<> dispatcher(this, __func__, runnable);
   if (dispatcher.Success()) {
     capability = mReplyCapability;
   }
   return dispatcher.ReturnValue();
 }
 
 mozilla::ipc::IPCResult
@@ -388,24 +368,19 @@ int
 CamerasChild::GetCaptureDevice(CaptureEngine aCapEngine,
                                unsigned int list_number, char* device_nameUTF8,
                                const unsigned int device_nameUTF8Length,
                                char* unique_idUTF8,
                                const unsigned int unique_idUTF8Length,
                                bool* scary)
 {
   LOG((__PRETTY_FUNCTION__));
-  RefPtr<CamerasChild> self = this;
   nsCOMPtr<nsIRunnable> runnable =
-    media::NewRunnableFrom([self, aCapEngine, list_number]() -> nsresult {
-      if (self->SendGetCaptureDevice(aCapEngine, list_number)) {
-        return NS_OK;
-      }
-      return NS_ERROR_FAILURE;
-    });
+    mozilla::NewNonOwningRunnableMethod<CaptureEngine, unsigned int>
+    (this, &CamerasChild::SendGetCaptureDevice, aCapEngine, list_number);
   LockAndDispatch<> dispatcher(this, __func__, runnable);
   if (dispatcher.Success()) {
     base::strlcpy(device_nameUTF8, mReplyDeviceName.get(), device_nameUTF8Length);
     base::strlcpy(unique_idUTF8, mReplyDeviceID.get(), unique_idUTF8Length);
     if (scary) {
       *scary = mReplyScary;
     }
     LOG(("Got %s name %s id", device_nameUTF8, unique_idUTF8));
@@ -433,24 +408,19 @@ int
 CamerasChild::AllocateCaptureDevice(CaptureEngine aCapEngine,
                                     const char* unique_idUTF8,
                                     const unsigned int unique_idUTF8Length,
                                     int& aStreamId,
                                     const mozilla::ipc::PrincipalInfo& aPrincipalInfo)
 {
   LOG((__PRETTY_FUNCTION__));
   nsCString unique_id(unique_idUTF8);
-  RefPtr<CamerasChild> self = this;
   nsCOMPtr<nsIRunnable> runnable =
-    media::NewRunnableFrom([self, aCapEngine, unique_id, aPrincipalInfo]() -> nsresult {
-      if (self->SendAllocateCaptureDevice(aCapEngine, unique_id, aPrincipalInfo)) {
-        return NS_OK;
-      }
-      return NS_ERROR_FAILURE;
-    });
+    mozilla::NewNonOwningRunnableMethod<CaptureEngine, nsCString, const mozilla::ipc::PrincipalInfo&>
+    (this, &CamerasChild::SendAllocateCaptureDevice, aCapEngine, unique_id, aPrincipalInfo);
   LockAndDispatch<> dispatcher(this, __func__, runnable);
   if (dispatcher.Success()) {
     LOG(("Capture Device allocated: %d", mReplyInteger));
     aStreamId = mReplyInteger;
   }
   return dispatcher.ReturnValue();
 }
 
@@ -467,24 +437,19 @@ CamerasChild::RecvReplyAllocateCaptureDe
   return IPC_OK();
 }
 
 int
 CamerasChild::ReleaseCaptureDevice(CaptureEngine aCapEngine,
                                    const int capture_id)
 {
   LOG((__PRETTY_FUNCTION__));
-  RefPtr<CamerasChild> self = this;
   nsCOMPtr<nsIRunnable> runnable =
-    media::NewRunnableFrom([self, aCapEngine, capture_id]() -> nsresult {
-      if (self->SendReleaseCaptureDevice(aCapEngine, capture_id)) {
-        return NS_OK;
-      }
-      return NS_ERROR_FAILURE;
-    });
+    mozilla::NewNonOwningRunnableMethod<CaptureEngine, int>
+    (this, &CamerasChild::SendReleaseCaptureDevice, aCapEngine, capture_id);
   LockAndDispatch<> dispatcher(this, __func__, runnable);
   return dispatcher.ReturnValue();
 }
 
 void
 CamerasChild::AddCallback(const CaptureEngine aCapEngine, const int capture_id,
                           FrameRelay* render)
 {
@@ -519,40 +484,30 @@ CamerasChild::StartCapture(CaptureEngine
   AddCallback(aCapEngine, capture_id, cb);
   VideoCaptureCapability capCap(webrtcCaps.width,
                            webrtcCaps.height,
                            webrtcCaps.maxFPS,
                            webrtcCaps.expectedCaptureDelay,
                            webrtcCaps.rawType,
                            webrtcCaps.codecType,
                            webrtcCaps.interlaced);
-  RefPtr<CamerasChild> self = this;
   nsCOMPtr<nsIRunnable> runnable =
-    media::NewRunnableFrom([self, aCapEngine, capture_id, capCap]() -> nsresult {
-      if (self->SendStartCapture(aCapEngine, capture_id, capCap)) {
-        return NS_OK;
-      }
-      return NS_ERROR_FAILURE;
-    });
+    mozilla::NewNonOwningRunnableMethod<CaptureEngine, int, VideoCaptureCapability>
+    (this, &CamerasChild::SendStartCapture, aCapEngine, capture_id, capCap);
   LockAndDispatch<> dispatcher(this, __func__, runnable);
   return dispatcher.ReturnValue();
 }
 
 int
 CamerasChild::StopCapture(CaptureEngine aCapEngine, const int capture_id)
 {
   LOG((__PRETTY_FUNCTION__));
-  RefPtr<CamerasChild> self = this;
   nsCOMPtr<nsIRunnable> runnable =
-    media::NewRunnableFrom([self, aCapEngine, capture_id]() -> nsresult {
-      if (self->SendStopCapture(aCapEngine, capture_id)) {
-        return NS_OK;
-      }
-      return NS_ERROR_FAILURE;
-    });
+    mozilla::NewNonOwningRunnableMethod<CaptureEngine, int>
+    (this, &CamerasChild::SendStopCapture, aCapEngine, capture_id);
   LockAndDispatch<> dispatcher(this, __func__, runnable);
   if (dispatcher.Success()) {
     RemoveCallback(aCapEngine, capture_id);
   }
   return dispatcher.ReturnValue();
 }
 
 void
@@ -603,24 +558,20 @@ CamerasChild::ShutdownParent()
   {
     MonitorAutoLock monitor(mReplyMonitor);
     mIPCIsAlive = false;
     monitor.NotifyAll();
   }
   if (CamerasSingleton::Thread()) {
     LOG(("Dispatching actor deletion"));
     // Delete the parent actor.
-    RefPtr<CamerasChild> self = this;
-    RefPtr<Runnable> deleteRunnable =
-      // CamerasChild (this) will remain alive and is only deleted by the
-      // IPC layer when SendAllDone returns.
-      media::NewRunnableFrom([self]() -> nsresult {
-        Unused << self->SendAllDone();
-        return NS_OK;
-      });
+    // CamerasChild (this) will remain alive and is only deleted by the
+    // IPC layer when SendAllDone returns.
+    nsCOMPtr<nsIRunnable> deleteRunnable =
+      mozilla::NewNonOwningRunnableMethod(this, &CamerasChild::SendAllDone);
     CamerasSingleton::Thread()->Dispatch(deleteRunnable, NS_DISPATCH_NORMAL);
   } else {
     LOG(("ShutdownParent called without PBackground thread"));
   }
 }
 
 void
 CamerasChild::ShutdownChild()