MediaRecorder: fire onError if the amount of tracks of the MediaStream varies

third_party/WebKit/Source/modules/mediarecorder/MediaRecorder.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "modules/mediarecorder/MediaRecorder.h"
 
 #include "bindings/core/v8/Dictionary.h"
 #include "core/events/Event.h"
 #include "core/fileapi/Blob.h"
 #include "modules/EventTargetModules.h"
@@ skipping 37 matching lines @@
 {
     MediaRecorder* recorder = new MediaRecorder(context, stream, options, exceptionState);
     recorder->suspendIfNeeded();
 
     return recorder;
 }
 
 MediaRecorder::MediaRecorder(ExecutionContext* context, MediaStream* stream, const MediaRecorderOptions& options, ExceptionState& exceptionState)
     : ActiveDOMObject(context)
     , m_stream(stream)
+    , m_streamAmountOfTracks(stream->getTracks().size())
     , m_mimeType(options.mimeType())
     , m_stopped(true)
     , m_ignoreMutedMedia(true)
     , m_state(State::Inactive)
     , m_dispatchScheduledEventRunner(this, &MediaRecorder::dispatchScheduledEvent)
 {
     ASSERT(m_stream->getTracks().size());
 
     m_recorderHandler = adoptPtr(Platform::current()->createMediaRecorderHandler());
     ASSERT(m_recorderHandler);
@@ skipping 130 matching lines @@
     m_stream.clear();
     m_recorderHandler.clear();
 }
 
 void MediaRecorder::writeData(const char* data, size_t length, bool lastInSlice)
 {
     if (m_stopped && !lastInSlice) {
         m_stopped = false;
         scheduleDispatchEvent(Event::create(EventTypeNames::start));
     }
+    if (m_stream && m_streamAmountOfTracks != m_stream->getTracks().size())
+        onError("Amount of tracks in MediaStream has changed.");

[Peter Beverloo, 2016/01/13 18:38:01]

This fires an `onerror` event each time writeData() is called after the number
of tracks has changed. There's also the case where a track was removed, and
another track was added - today that would mute the error again.

How useful are the repeated `onerror` events if we don't stop the recording? In
most APIs, receiving an `onerror` event means that the functionality stops (i.e.
notifications or XHR).

[Peter Beverloo, 2016/01/13 18:38:01]

Will you address the specification bug?

[mcasas, 2016/01/13 21:50:41]

Acknowledged.

[mcasas, 2016/01/13 21:50:41]

Yes, I should update m_streamAmountOfTracks after the
first error is fired. Done. 
So I was thinking about it, and wrote in the CL description,
that what would be the best way to proceed towards the
user  when a Track is added or Removed, since anyway 
the content would not be recorded/ sotp being recorded,
respectively, automatically? I think the MR should keep
recording whatever it has and let the JS decide what's the
appropriate course of action. The Spec is quite fuzzy 
about it, and it only mentions a DOMException in case
Tracks are added or removed... which can't be since is
an asynchronous condition. Like you say below, the Spec
should be updated (which falls on me again!)

So the point of this CL is to, at least, fire an Error if 
something like this happen...

 
     // TODO(mcasas): Act as |m_ignoredMutedMedia| instructs if |m_stream| track(s) is in muted() state.
 
     if (!m_blobData)
         m_blobData = BlobData::create();
     if (data)
         m_blobData->appendBytes(data, length);
 
     if (!lastInSlice)
         return;
 
     // Cache |m_blobData->length()| before release()ng it.
     const long long blobDataLength = m_blobData->length();
     createBlobEvent(Blob::create(BlobDataHandle::create(m_blobData.release(), blobDataLength)));
 }
 
 void MediaRecorder::onError(const WebString& message)
 {
     // TODO(mcasas): Beef up the Error Event and add the |message|, see https://github.com/w3c/mediacapture-record/issues/31
     scheduleDispatchEvent(Event::create(EventTypeNames::error));
-
-    if (m_state == State::Recording)
-        stopRecording();
 }
 
 void MediaRecorder::createBlobEvent(Blob* blob)
 {
     // TODO(mcasas): Consider launching an Event with a TypedArray inside, see https://github.com/w3c/mediacapture-record/issues/17.
     scheduleDispatchEvent(BlobEvent::create(EventTypeNames::dataavailable, blob));
 }
 
 void MediaRecorder::stopRecording()
 {
@@ skipping 24 matching lines @@
 
 DEFINE_TRACE(MediaRecorder)
 {
     visitor->trace(m_stream);
     visitor->trace(m_scheduledEvents);
     RefCountedGarbageCollectedEventTargetWithInlineData<MediaRecorder>::trace(visitor);
     ActiveDOMObject::trace(visitor);
 }
 
 } // namespace blink