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 344839 b4cb7a1e84a85fe979bec7263c1a7c71a4a5323c
parent 344838 54cc45b7dd55efb22294d257dab2d1ada338e2d6
child 344840 a545bcb544a738353f521593643f8ed3c5924c5b
push id37969
push userkwierso@gmail.com
push dateSat, 25 Feb 2017 00:59:33 +0000
treeherderautoland@23bd8be8d881 [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()