Bug 1336510 - Part 6: Avoid AddRefing/Releasing CamerasChild on the wrong thread, r=jwwang
☠☠ backed out by 3fee518242f6 ☠ ☠
authorMichael Layzell <michael@thelayzells.com>
Thu, 16 Feb 2017 14:33:53 -0500
changeset 344617 422e63b872b5284a2f31cfaf36c1853a8f636a75
parent 344616 21869174dfd3e1548324f590245ea06d215a5d6a
child 344618 c8aeead6fe83a62d86c15decc9445a089ab2b717
push id31414
push usercbook@mozilla.com
push dateFri, 24 Feb 2017 10:47:41 +0000
treeherdermozilla-central@be661bae6cb9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwwang
bugs1336510
milestone54.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 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()