Bug 1413116 - Consolidate MediaRecorder::Session state and avoid "start" and "error" races. r?SingingTree draft
authorAndreas Pehrson <pehrsons@mozilla.com>
Tue, 31 Oct 2017 18:34:23 +0100
changeset 695709 986f84b72820f6b234dfb29158df3f5a9a3393b4
parent 695708 3af888fddb59ea17b5e89df0e54d2d87a933c87e
child 739680 c647f0710f943ea3a3e7a6ce5e4f570ce8072af1
push id88509
push userbmo:apehrson@mozilla.com
push dateThu, 09 Nov 2017 18:05:36 +0000
reviewersSingingTree
bugs1413116
milestone58.0a1
Bug 1413116 - Consolidate MediaRecorder::Session state and avoid "start" and "error" races. r?SingingTree MozReview-Commit-ID: L5cj4DStnUt
dom/media/MediaRecorder.cpp
--- a/dom/media/MediaRecorder.cpp
+++ b/dom/media/MediaRecorder.cpp
@@ -228,23 +228,23 @@ class MediaRecorder::Session: public Pri
                        nsresult aRv) override
     {
       RefPtr<MediaRecorder> recorder = mSession->mRecorder;
       if (!recorder) {
         return;
       }
 
       if (NS_FAILED(aRv)) {
-        recorder->NotifyError(aRv);
+        mSession->DoSessionEndTask(aRv);
         return;
       }
 
       nsresult rv = recorder->CreateAndDispatchBlobEvent(aBlob);
       if (NS_FAILED(rv)) {
-        recorder->NotifyError(aRv);
+        mSession->DoSessionEndTask(aRv);
       }
 
       if (mDestroyRunnable &&
           NS_FAILED(NS_DispatchToMainThread(mDestroyRunnable.forget()))) {
         MOZ_ASSERT(false, "NS_DispatchToMainThread failed");
       }
     }
 
@@ -320,39 +320,36 @@ class MediaRecorder::Session: public Pri
   private:
     RefPtr<Session> mSession;
   };
 
   // Fire start event and set mimeType, run in main thread task.
   class DispatchStartEventRunnable : public Runnable
   {
   public:
-    DispatchStartEventRunnable(Session* aSession, const nsAString& aEventName)
+    explicit DispatchStartEventRunnable(Session* aSession)
       : Runnable("dom::MediaRecorder::Session::DispatchStartEventRunnable")
       , mSession(aSession)
-      , mEventName(aEventName)
     { }
 
     NS_IMETHOD Run() override
     {
       LOG(LogLevel::Debug, ("Session.DispatchStartEventRunnable s=(%p)", mSession.get()));
       MOZ_ASSERT(NS_IsMainThread());
 
       NS_ENSURE_TRUE(mSession->mRecorder, NS_OK);
       RefPtr<MediaRecorder> recorder = mSession->mRecorder;
 
-      recorder->SetMimeType(mSession->mMimeType);
-      recorder->DispatchSimpleEvent(mEventName);
+      recorder->DispatchSimpleEvent(NS_LITERAL_STRING("start"));
 
       return NS_OK;
     }
 
   private:
     RefPtr<Session> mSession;
-    nsString mEventName;
   };
 
   // To ensure that MediaRecorder has tracks to record.
   class TracksAvailableCallback : public OnTracksAvailableCallback
   {
   public:
     explicit TracksAvailableCallback(Session *aSession)
      : mSession(aSession) {}
@@ -378,37 +375,43 @@ class MediaRecorder::Session: public Pri
     explicit DestroyRunnable(already_AddRefed<Session> aSession)
       : Runnable("dom::MediaRecorder::Session::DestroyRunnable")
       , mSession(aSession)
     {
     }
 
     NS_IMETHOD Run() override
     {
-      LOG(LogLevel::Debug, ("Session.DestroyRunnable session refcnt = (%d) stopIssued %d s=(%p)",
-                         (int)mSession->mRefCnt, mSession->mStopIssued, mSession.get()));
+      LOG(LogLevel::Debug, ("Session.DestroyRunnable session refcnt = (%d) s=(%p)",
+                            static_cast<int>(mSession->mRefCnt), mSession.get()));
       MOZ_ASSERT(NS_IsMainThread() && mSession);
       RefPtr<MediaRecorder> recorder = mSession->mRecorder;
       if (!recorder) {
         return NS_OK;
       }
       // SourceMediaStream is ended, and send out TRACK_EVENT_END notification.
       // Read Thread will be terminate soon.
       // We need to switch MediaRecorder to "Stop" state first to make sure
       // MediaRecorder is not associated with this Session anymore, then, it's
       // safe to delete this Session.
       // Also avoid to run if this session already call stop before
-      if (!mSession->mStopIssued) {
+      if (mSession->mRunningState.isOk() &&
+          mSession->mRunningState.unwrap() != RunningState::Stopping &&
+          mSession->mRunningState.unwrap() != RunningState::Stopped) {
         recorder->StopForSessionDestruction();
         if (NS_FAILED(NS_DispatchToMainThread(new DestroyRunnable(mSession.forget())))) {
           MOZ_ASSERT(false, "NS_DispatchToMainThread failed");
         }
         return NS_OK;
       }
 
+      if (mSession->mRunningState.isOk()) {
+        mSession->mRunningState = RunningState::Stopped;
+      }
+
       // Dispatch stop event and clear MIME type.
       mSession->mMimeType = NS_LITERAL_STRING("");
       recorder->SetMimeType(mSession->mMimeType);
       recorder->DispatchSimpleEvent(NS_LITERAL_STRING("stop"));
 
       RefPtr<Session> session = mSession.forget();
       session->Shutdown()->Then(
         GetCurrentThreadSerialEventTarget(), __func__,
@@ -486,19 +489,17 @@ class MediaRecorder::Session: public Pri
   friend class PushBlobRunnable;
   friend class DestroyRunnable;
   friend class TracksAvailableCallback;
 
 public:
   Session(MediaRecorder* aRecorder, int32_t aTimeSlice)
     : mRecorder(aRecorder)
     , mTimeSlice(aTimeSlice)
-    , mStopIssued(false)
-    , mIsStartEventFired(false)
-    , mNeedSessionEndTask(true)
+    , mRunningState(RunningState::Idling)
   {
     MOZ_ASSERT(NS_IsMainThread());
 
     mMaxMemory = Preferences::GetUint("media.recorder.max_memory",
                                       MAX_ALLOW_MEMORY_BUFFER);
     mLastBlobTimeStamp = TimeStamp::Now();
   }
 
@@ -565,26 +566,30 @@ public:
 
     MOZ_ASSERT(false, "Unknown source");
   }
 
   void Stop()
   {
     LOG(LogLevel::Debug, ("Session.Stop %p", this));
     MOZ_ASSERT(NS_IsMainThread());
-    mStopIssued = true;
 
     if (mEncoder) {
       mEncoder->Stop();
     }
 
-    if (mNeedSessionEndTask) {
-      LOG(LogLevel::Debug, ("Session.Stop mNeedSessionEndTask %p", this));
+    if (mRunningState.isOk() &&
+        mRunningState.unwrap() == RunningState::Idling) {
+      LOG(LogLevel::Debug, ("Session.Stop Explicit end task %p", this));
       // End the Session directly if there is no ExtractRunnable.
       DoSessionEndTask(NS_OK);
+    } else if (mRunningState.isOk() &&
+               (mRunningState.unwrap() == RunningState::Starting ||
+                mRunningState.unwrap() == RunningState::Running)) {
+      mRunningState = RunningState::Stopping;
     }
   }
 
   nsresult Pause()
   {
     LOG(LogLevel::Debug, ("Session.Pause"));
     MOZ_ASSERT(NS_IsMainThread());
 
@@ -716,17 +721,17 @@ private:
         MOZ_ASSERT(false, "NS_DispatchToMainThread DestroyRunnable failed");
       }
     }
   }
 
   void MediaStreamReady(DOMMediaStream* aStream) {
     MOZ_RELEASE_ASSERT(aStream);
 
-    if (mStopIssued) {
+    if (!mRunningState.isOk() || mRunningState.unwrap() != RunningState::Idling) {
       return;
     }
 
     mMediaStream = aStream;
     aStream->RegisterTrackListener(this);
 
     uint8_t trackTypes = 0;
     nsTArray<RefPtr<mozilla::dom::AudioStreamTrack>> audioTracks;
@@ -834,16 +839,21 @@ private:
     return PrincipalSubsumes(principal);
   }
 
   void InitEncoder(uint8_t aTrackTypes, TrackRate aTrackRate)
   {
     LOG(LogLevel::Debug, ("Session.InitEncoder %p", this));
     MOZ_ASSERT(NS_IsMainThread());
 
+    if (!mRunningState.isOk() || mRunningState.unwrap() != RunningState::Idling) {
+      MOZ_ASSERT_UNREACHABLE("Double-init");
+      return;
+    }
+
     // Create a TaskQueue to read encode media data from MediaEncoder.
     MOZ_RELEASE_ASSERT(!mEncoderThread);
     RefPtr<SharedThreadPool> pool =
       SharedThreadPool::Get(NS_LITERAL_CSTRING("MediaRecorderReadThread"));
     if (!pool) {
       LOG(LogLevel::Debug, ("Session.InitEncoder %p Failed to create "
                             "MediaRecorderReadThread thread pool", this));
       DoSessionEndTask(NS_ERROR_FAILURE);
@@ -946,29 +956,46 @@ private:
       mEncoder->ConnectAudioNode(mRecorder->mAudioNode,
                                  mRecorder->mAudioNodeOutput);
     }
 
     for (auto& track : mMediaStreamTracks) {
       mEncoder->ConnectMediaStreamTrack(track);
     }
 
-    // Set mNeedSessionEndTask to false because the
-    // ExtractRunnable/DestroyRunnable will take the response to
-    // end the session.
-    mNeedSessionEndTask = false;
+    // Set mRunningState to Running so that ExtractRunnable/DestroyRunnable will
+    // take the responsibility to end the session.
+    mRunningState = RunningState::Starting;
   }
 
   // application should get blob and onstop event
   void DoSessionEndTask(nsresult rv)
   {
     MOZ_ASSERT(NS_IsMainThread());
-    if (!mIsStartEventFired) {
-      NS_DispatchToMainThread(
-        new DispatchStartEventRunnable(this, NS_LITERAL_STRING("start")));
+    if (mRunningState.isErr()) {
+      // We have already ended with an error.
+      return;
+    }
+
+    if (mRunningState.isOk() &&
+        mRunningState.unwrap() == RunningState::Stopped) {
+      // We have already ended gracefully.
+      return;
+    }
+
+    if (mRunningState.isOk() &&
+        (mRunningState.unwrap() == RunningState::Idling ||
+         mRunningState.unwrap() == RunningState::Starting)) {
+      NS_DispatchToMainThread(new DispatchStartEventRunnable(this));
+    }
+
+    if (rv == NS_OK) {
+      mRunningState = RunningState::Stopped;
+    } else {
+      mRunningState = Err(rv);
     }
 
     if (NS_FAILED(rv)) {
       mRecorder->ForceInactive();
       NS_DispatchToMainThread(
         NewRunnableMethod<nsresult>("dom::MediaRecorder::NotifyError",
                                     mRecorder,
                                     &MediaRecorder::NotifyError,
@@ -982,47 +1009,66 @@ private:
       if (NS_FAILED(NS_DispatchToMainThread(new PushBlobRunnable(this, destroyRunnable)))) {
         MOZ_ASSERT(false, "NS_DispatchToMainThread PushBlobRunnable failed");
       }
     } else {
       if (NS_FAILED(NS_DispatchToMainThread(destroyRunnable))) {
         MOZ_ASSERT(false, "NS_DispatchToMainThread DestroyRunnable failed");
       }
     }
-
-    mNeedSessionEndTask = false;
   }
 
   void MediaEncoderInitialized()
   {
     MOZ_ASSERT(mEncoderThread->IsCurrentThreadIn());
 
     // Pull encoded metadata from MediaEncoder
     nsTArray<nsTArray<uint8_t> > encodedBuf;
-    nsresult rv = mEncoder->GetEncodedMetadata(&encodedBuf, mMimeType);
+    nsString mime;
+    nsresult rv = mEncoder->GetEncodedMetadata(&encodedBuf, mime);
+
     if (NS_FAILED(rv)) {
       MOZ_ASSERT(false);
       return;
     }
 
     // Append pulled data into cache buffer.
     NS_DispatchToMainThread(new StoreEncodedBufferRunnable(this,
                                                            Move(encodedBuf)));
+
+    RefPtr<Session> self = this;
+    NS_DispatchToMainThread(NewRunnableFrom([self, mime]() {
+      if (!self->mRecorder) {
+        MOZ_ASSERT_UNREACHABLE("Recorder should be live");
+        return NS_OK;
+      }
+      if (self->mRunningState.isOk()) {
+        auto state = self->mRunningState.unwrap();
+        if (state == RunningState::Starting || state == RunningState::Stopping) {
+          if (state == RunningState::Starting) {
+            // We set it to Running in the runnable since we can only assign
+            // mRunningState on main thread. We set it before running the start
+            // event runnable since that dispatches synchronously (and may cause
+            // js calls to methods depending on mRunningState).
+            self->mRunningState = RunningState::Running;
+          }
+          self->mMimeType = mime;
+          self->mRecorder->SetMimeType(self->mMimeType);
+          auto startEvent = MakeRefPtr<DispatchStartEventRunnable>(self);
+          startEvent->Run();
+        }
+      }
+      return NS_OK;
+    }));
   }
 
   void MediaEncoderDataAvailable()
   {
     MOZ_ASSERT(mEncoderThread->IsCurrentThreadIn());
 
-    if (!mIsStartEventFired) {
-      NS_DispatchToMainThread(
-        new DispatchStartEventRunnable(this, NS_LITERAL_STRING("start")));
-      mIsStartEventFired = true;
-    }
-
     Extract(false, nullptr);
   }
 
   void MediaEncoderError()
   {
     MOZ_ASSERT(mEncoderThread->IsCurrentThreadIn());
     NS_DispatchToMainThread(
       NewRunnableMethod<nsresult>(
@@ -1119,16 +1165,24 @@ private:
           return ShutdownPromise::CreateAndReject(false, __func__);
         });
     }
 
     return mShutdownPromise;
   }
 
 private:
+  enum class RunningState {
+    Idling, // Session has been created
+    Starting, // MediaEncoder started, waiting for data
+    Running, // MediaEncoder has produced data
+    Stopping, // Stop() has been called
+    Stopped, // Session has stopped without any error
+  };
+
   // Hold reference to MediaRecorder that ensure MediaRecorder is alive
   // if there is an active session. Access ONLY on main thread.
   RefPtr<MediaRecorder> mRecorder;
 
   // Stream currently recorded.
   RefPtr<DOMMediaStream> mMediaStream;
 
   // Tracks currently recorded. This should be a subset of mMediaStream's track
@@ -1151,24 +1205,20 @@ private:
   nsString mMimeType;
   // Timestamp of the last fired dataavailable event.
   TimeStamp mLastBlobTimeStamp;
   // The interval of passing encoded data from MutableBlobStorage to
   // onDataAvailable handler. "mTimeSlice < 0" means Session object does not
   // push encoded data to onDataAvailable, instead, it passive wait the client
   // side pull encoded data by calling requestData API.
   const int32_t mTimeSlice;
-  // Indicate this session's stop has been called.
-  bool mStopIssued;
-  // Indicate the session had fire start event. Encoding thread only.
-  bool mIsStartEventFired;
-  // False if the InitEncoder called successfully, ensure the
-  // ExtractRunnable/DestroyRunnable will end the session.
+  // The session's current main thread state. The error type gets setwhen ending
+  // a recording with an error. An NS_OK error is invalid.
   // Main thread only.
-  bool mNeedSessionEndTask;
+  Result<RunningState, nsresult> mRunningState;
 };
 
 NS_IMPL_ISUPPORTS_INHERITED0(MediaRecorder::Session::PushBlobRunnable, Runnable)
 
 MediaRecorder::~MediaRecorder()
 {
   LOG(LogLevel::Debug, ("~MediaRecorder (%p)", this));
   UnRegisterActivityObserver();