Bug 1436694 - Clarify that MediaEngineSources can be double-stopped. r=padenot
authorAndreas Pehrson <pehrsons@mozilla.com>
Thu, 22 Feb 2018 12:23:06 +0100
changeset 461810 04b165b560de7746ab20988189619bd213c7c01b
parent 461809 0451fe123f5b9168de34b3f927bb421c8e9864ca
child 461811 07fad8b0b417d9ae8580f23d697172a3735b546b
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs1436694
milestone60.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 1436694 - Clarify that MediaEngineSources can be double-stopped. r=padenot This is already true for the audio sources. It should be for all. Crashtests showed that shutting down amidst the async init can lead to double-stops. It is impossible to completely protect yourself from them without waiting for all queued operations to resolve (results to become known) before taking action. Doing that would require a refactor in MediaManager and cause higher latency for device operations so it seems like the wrong way to go. MozReview-Commit-ID: 5Cci6whzTL7
dom/media/webrtc/MediaEngineDefault.cpp
dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
dom/media/webrtc/MediaEngineSource.h
dom/media/webrtc/MediaEngineTabVideoSource.cpp
--- a/dom/media/webrtc/MediaEngineDefault.cpp
+++ b/dom/media/webrtc/MediaEngineDefault.cpp
@@ -232,16 +232,20 @@ MediaEngineDefaultVideoSource::Start(con
   return NS_OK;
 }
 
 nsresult
 MediaEngineDefaultVideoSource::Stop(const RefPtr<const AllocationHandle>& aHandle)
 {
   AssertIsOnOwningThread();
 
+  if (mState == kStopped || mState == kAllocated) {
+    return NS_OK;
+  }
+
   MOZ_ASSERT(mState == kStarted);
   MOZ_ASSERT(mTimer);
   MOZ_ASSERT(mStream);
   MOZ_ASSERT(IsTrackIDExplicit(mTrackID));
 
   mTimer->Cancel();
   mTimer = nullptr;
 
@@ -490,19 +494,22 @@ MediaEngineDefaultAudioSource::Start(con
   return NS_OK;
 }
 
 nsresult
 MediaEngineDefaultAudioSource::Stop(const RefPtr<const AllocationHandle>& aHandle)
 {
   AssertIsOnOwningThread();
 
+  if (mState == kStopped || mState == kAllocated) {
+    return NS_OK;
+  }
+
   MOZ_ASSERT(mState == kStarted);
 
-
   MutexAutoLock lock(mMutex);
   mState = kStopped;
   return NS_OK;
 }
 
 nsresult
 MediaEngineDefaultAudioSource::Reconfigure(
     const RefPtr<AllocationHandle>& aHandle,
--- a/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
+++ b/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
@@ -319,16 +319,20 @@ MediaEngineRemoteVideoSource::Start(cons
 }
 
 nsresult
 MediaEngineRemoteVideoSource::Stop(const RefPtr<const AllocationHandle>& aHandle)
 {
   LOG((__PRETTY_FUNCTION__));
   AssertIsOnOwningThread();
 
+  if (mState == kStopped || mState == kAllocated) {
+    return NS_OK;
+  }
+
   MOZ_ASSERT(mState == kStarted);
 
   if (camera::GetChildAndCall(&camera::CamerasChild::StopCapture,
                               mCapEngine, mCaptureIndex)) {
     MOZ_DIAGNOSTIC_ASSERT(false, "Stopping a started capture failed");
   }
 
   {
--- a/dom/media/webrtc/MediaEngineSource.h
+++ b/dom/media/webrtc/MediaEngineSource.h
@@ -166,16 +166,19 @@ public:
                                const char** aOutBadConstraint) = 0;
 
   /**
    * Called by MediaEngine to stop feeding data to the track associated with
    * the given AllocationHandle.
    *
    * If this was the last AllocationHandle that had been started,
    * the underlying device will be stopped.
+   *
+   * Double-stopping a given allocation handle is allowed and will return NS_OK.
+   * This is necessary sometimes during shutdown.
    */
   virtual nsresult Stop(const RefPtr<const AllocationHandle>& aHandle) = 0;
 
   /**
    * Called by MediaEngine to deallocate a handle to this source.
    *
    * If this was the last registered AllocationHandle, the underlying device
    * will be deallocated.
--- a/dom/media/webrtc/MediaEngineTabVideoSource.cpp
+++ b/dom/media/webrtc/MediaEngineTabVideoSource.cpp
@@ -397,16 +397,21 @@ MediaEngineTabVideoSource::Draw() {
   mImage = image;
   mImageSize = size;
 }
 
 nsresult
 MediaEngineTabVideoSource::Stop(const RefPtr<const AllocationHandle>& aHandle)
 {
   AssertIsOnOwningThread();
+
+  if (mState == kStopped || mState == kAllocated) {
+    return NS_OK;
+  }
+
   MOZ_ASSERT(mState == kStarted);
 
   // If mBlackedoutWindow is true, we may be running
   // despite mWindow == nullptr.
   if (!mWindow && !mBlackedoutWindow) {
     return NS_OK;
   }