MediaRecorder: make sure ondataavailable+onstop events are fired before onerror

content/renderer/media/recorder/media_recorder_handler.cc

Index: content/renderer/media/recorder/media_recorder_handler.cc
diff --git a/content/renderer/media/recorder/media_recorder_handler.cc b/content/renderer/media/recorder/media_recorder_handler.cc
index 3c3d2e8787e48b478f083739b4a9ed65e638d09c..c59e5b9fbb53a11da1eab381fb23fba7870d6e8c 100644
--- a/content/renderer/media/recorder/media_recorder_handler.cc
+++ b/content/renderer/media/recorder/media_recorder_handler.cc
@@ -58,6 +58,8 @@ MediaRecorderHandler::MediaRecorderHandler()
       audio_bits_per_second_(0),
       codec_id_(VideoTrackRecorder::CodecId::VP8),
       recording_(false),
+      media_stream_num_video_tracks_(0),
+      media_stream_num_audio_tracks_(0),
       client_(nullptr),
       weak_factory_(this) {}
 
@@ -146,6 +148,7 @@ bool MediaRecorderHandler::initialize(
                                   << static_cast<int>(codec_id_);
 
   media_stream_ = media_stream;
+

[emircan, 2017/02/17 18:59:25]

not: Remove empty line.

[mcasas, 2017/02/17 20:40:29]

Done.

   DCHECK(client);
   client_ = client;
 
@@ -167,6 +170,8 @@ bool MediaRecorderHandler::start(int timeslice) {
   blink::WebVector<blink::WebMediaStreamTrack> video_tracks, audio_tracks;
   media_stream_.videoTracks(video_tracks);
   media_stream_.audioTracks(audio_tracks);
+  media_stream_num_video_tracks_ = video_tracks.size();
+  media_stream_num_audio_tracks_ = audio_tracks.size();
 
   if (video_tracks.isEmpty() && audio_tracks.isEmpty()) {
     LOG(WARNING) << __func__ << ": no media tracks.";
@@ -237,6 +242,7 @@ void MediaRecorderHandler::stop() {
   DCHECK(main_render_thread_checker_.CalledOnValidThread());
   // Don't check |recording_| since we can go directly from pause() to stop().
 
+  weak_factory_.InvalidateWeakPtrs();
   recording_ = false;
   timeslice_ = TimeDelta::FromMilliseconds(0);
   video_recorders_.clear();
@@ -272,6 +278,11 @@ void MediaRecorderHandler::OnEncodedVideo(
     TimeTicks timestamp,
     bool is_key_frame) {
   DCHECK(main_render_thread_checker_.CalledOnValidThread());
+
+  if (CheckAndUpdateAmountOfTracks()) {
+    client_->onError("Amount of tracks in MediaStream has changed.");
+    return;
+  }
   if (!webm_muxer_)
     return;
   if (!webm_muxer_->OnEncodedVideo(params, std::move(encoded_data), timestamp,
@@ -286,6 +297,11 @@ void MediaRecorderHandler::OnEncodedAudio(
     std::unique_ptr<std::string> encoded_data,
     base::TimeTicks timestamp) {
   DCHECK(main_render_thread_checker_.CalledOnValidThread());
+
+  if (CheckAndUpdateAmountOfTracks()) {
+    client_->onError("Amount of tracks in MediaStream has changed.");
+    return;
+  }
   if (!webm_muxer_)
     return;
   if (!webm_muxer_->OnEncodedAudio(params, std::move(encoded_data),
@@ -313,6 +329,22 @@ void MediaRecorderHandler::WriteData(base::StringPiece data) {
                      (now - TimeTicks::UnixEpoch()).InMillisecondsF());
 }
 
+bool MediaRecorderHandler::CheckAndUpdateAmountOfTracks() {
+  DCHECK(main_render_thread_checker_.CalledOnValidThread());
+
+  blink::WebVector<blink::WebMediaStreamTrack> video_tracks, audio_tracks;
+  media_stream_.videoTracks(video_tracks);
+  media_stream_.audioTracks(audio_tracks);
+
+  if (media_stream_num_video_tracks_ == video_tracks.size() &&
+      media_stream_num_audio_tracks_ == audio_tracks.size()) {

[emircan, 2017/02/17 18:59:25]

As we discussed offline, there can be a case of one video track removed and
another added so we can compare the actual tracks here instead.

[mcasas, 2017/02/17 20:40:29]

Done.

+    return false;
+  }
+  media_stream_num_video_tracks_ = video_tracks.size();
+  media_stream_num_audio_tracks_ = audio_tracks.size();
+  return true;
+}
+
 void MediaRecorderHandler::OnVideoFrameForTesting(
     const scoped_refptr<media::VideoFrame>& frame,
     const TimeTicks& timestamp) {