Bug 1436694 - Clarify that MediaEngineSources can be double-stopped. r=padenot
☠☠ backed out by 6e0b46a7a0a0 ☠ ☠
authorAndreas Pehrson <pehrsons@mozilla.com>
Thu, 22 Feb 2018 12:23:06 +0100
changeset 405322 f5cc71d38e4a7c0e4db830242c1d454fcbdb9e48
parent 405321 6c38cc382d2114199842dddb14097be8b6d9a961
child 405323 1aff350b83b8d504dbaa3d5d8d4fe6cd99512786
push id33518
push usertoros@mozilla.com
push dateMon, 26 Feb 2018 22:20:13 +0000
treeherdermozilla-central@580d833df9c4 [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
@@ -233,16 +233,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;
 
@@ -493,19 +497,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;
   }