Bug 1388909 - Rework MediaRecorder to tranisition to inactive upon error. r=pehrsons
authorBryce Van Dyk <bvandyk@mozilla.com>
Fri, 01 Sep 2017 11:38:52 +1200
changeset 428291 963d871f9bc03c346d749786ca6c557ea0de068b
parent 428290 94eb3eee058513037d3fa036cd7345fb9367b64b
child 428292 268d45f4eca36c9911ad7d7373197c8a06e5779a
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspehrsons
bugs1388909
milestone57.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 1388909 - Rework MediaRecorder to tranisition to inactive upon error. r=pehrsons The MediaRecorder should transition to a 'inactive' state immediately upon error. This changeset updates the recorder to do so. Previously the recorder would fire an error event before transitioning, resuling in the state still being 'recording' for handling of the the thrown error. MozReview-Commit-ID: KMkaPOnEBYx
dom/media/MediaRecorder.cpp
dom/media/MediaRecorder.h
--- a/dom/media/MediaRecorder.cpp
+++ b/dom/media/MediaRecorder.cpp
@@ -396,19 +396,17 @@ class MediaRecorder::Session: public nsI
       }
       // 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) {
-        ErrorResult result;
-        mSession->mStopIssued = true;
-        recorder->Stop(result);
+        recorder->StopForSessionDestruction();
         if (NS_FAILED(NS_DispatchToMainThread(new DestroyRunnable(mSession.forget())))) {
           MOZ_ASSERT(false, "NS_DispatchToMainThread failed");
         }
         return NS_OK;
       }
 
       // Dispatch stop event and clear MIME type.
       mSession->mMimeType = NS_LITERAL_STRING("");
@@ -813,20 +811,22 @@ private:
     // end the session.
     mNeedSessionEndTask = false;
   }
   // application should get blob and onstop event
   void DoSessionEndTask(nsresult rv)
   {
     MOZ_ASSERT(NS_IsMainThread());
     CleanupStreams();
+
     NS_DispatchToMainThread(
       new DispatchStartEventRunnable(this, NS_LITERAL_STRING("start")));
 
     if (NS_FAILED(rv)) {
+      mRecorder->ForceInactive();
       NS_DispatchToMainThread(
         NewRunnableMethod<nsresult>("dom::MediaRecorder::NotifyError",
                                     mRecorder,
                                     &MediaRecorder::NotifyError,
                                     rv));
     }
     if (NS_FAILED(NS_DispatchToMainThread(new EncoderErrorNotifierRunnable(this)))) {
       MOZ_ASSERT(false, "NS_DispatchToMainThread EncoderErrorNotifierRunnable failed");
@@ -1471,16 +1471,35 @@ MediaRecorder::GetSourceMediaStream()
   if (mDOMStream != nullptr) {
     return mDOMStream->GetPlaybackStream();
   }
   MOZ_ASSERT(mAudioNode != nullptr);
   return mPipeStream ? mPipeStream.get() : mAudioNode->GetStream();
 }
 
 void
+MediaRecorder::ForceInactive()
+{
+  LOG(LogLevel::Debug, ("MediaRecorder.ForceInactive %p", this));
+  mState = RecordingState::Inactive;
+}
+
+void
+MediaRecorder::StopForSessionDestruction()
+{
+  LOG(LogLevel::Debug, ("MediaRecorder.StopForSessionDestruction %p", this));
+  MediaRecorderReporter::RemoveMediaRecorder(this);
+  // We do not perform a mState != RecordingState::Recording) check here as
+  // we may already be inactive due to ForceInactive().
+  mState = RecordingState::Inactive;
+  MOZ_ASSERT(mSessions.Length() > 0);
+  mSessions.LastElement()->Stop();
+}
+
+void
 MediaRecorder::InitializeDomExceptions()
 {
   mSecurityDomException = DOMException::Create(NS_ERROR_DOM_SECURITY_ERR);
   mUnknownDomException = DOMException::Create(NS_ERROR_DOM_UNKNOWN_ERR);
 }
 
 size_t
 MediaRecorder::SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
--- a/dom/media/MediaRecorder.h
+++ b/dom/media/MediaRecorder.h
@@ -135,16 +135,23 @@ protected:
   // Functions for Session to query input source info.
   MediaStream* GetSourceMediaStream();
   // Create DOMExceptions capturing the JS stack for async errors. These are
   // created ahead of time rather than on demand when firing an error as the JS
   // stack of the operation that started the async behavior will not be
   // available at the time the error event is fired. Note, depending on when
   // this is called there may not be a JS stack to capture.
   void InitializeDomExceptions();
+  // Set the recorder state to inactive. This is needed to handle error states
+  // in the recorder where state must transition to inactive before full
+  // stoppage can be reached.
+  void ForceInactive();
+  // Stop the recorder and its internal session. This should be used by
+  // sessions that are in the process of being destroyed.
+  void StopForSessionDestruction();
   // DOM wrapper for source media stream. Will be null when input is audio node.
   RefPtr<DOMMediaStream> mDOMStream;
   // Source audio node. Will be null when input is a media stream.
   RefPtr<AudioNode> mAudioNode;
   // Pipe stream connecting non-destination source node and session track union
   // stream of recorder. Will be null when input is media stream or destination
   // node.
   RefPtr<AudioNodeStream> mPipeStream;