MediaRecorder: update to spec (2/3)

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 "config.h"
 #include "modules/mediarecorder/MediaRecorder.h"
 
+#include "bindings/core/v8/Dictionary.h"
 #include "core/dom/DOMError.h"
 #include "core/fileapi/Blob.h"
 #include "core/frame/ConsoleTypes.h"
 #include "core/inspector/ConsoleMessage.h"
 #include "modules/EventModules.h"
 #include "modules/EventTargetModules.h"
 #include "modules/mediarecorder/BlobEvent.h"
 #include "modules/mediarecorder/MediaRecorderErrorEvent.h"
 #include "platform/NotImplemented.h"
 #include "platform/blob/BlobData.h"
@@ skipping 16 matching lines @@
     }
 
     ASSERT_NOT_REACHED();
     return String();
 }
 
 } // namespace
 
 MediaRecorder* MediaRecorder::create(ExecutionContext* context, MediaStream* stream, ExceptionState& exceptionState)
 {
-    MediaRecorder* recorder = new MediaRecorder(context, stream, String(), exceptionState);
+    MediaRecorder* recorder = new MediaRecorder(context, stream, MediaRecorderOptions(), exceptionState);
     recorder->suspendIfNeeded();
 
     return recorder;
 }
 
-MediaRecorder* MediaRecorder::create(ExecutionContext* context, MediaStream* stream,  const String& mimeType, ExceptionState& exceptionState)
+MediaRecorder* MediaRecorder::create(ExecutionContext* context, MediaStream* stream,  const MediaRecorderOptions& options, ExceptionState& exceptionState)
 {
-    MediaRecorder* recorder = new MediaRecorder(context, stream, mimeType, exceptionState);
+    MediaRecorder* recorder = new MediaRecorder(context, stream, options, exceptionState);
     recorder->suspendIfNeeded();
 
     return recorder;
 }
 
-MediaRecorder::MediaRecorder(ExecutionContext* context, MediaStream* stream, const String& mimeType, ExceptionState& exceptionState)
+MediaRecorder::MediaRecorder(ExecutionContext* context, MediaStream* stream, const MediaRecorderOptions& options, ExceptionState& exceptionState)
     : ActiveDOMObject(context)
     , m_stream(stream)
-    , m_mimeType(mimeType)
+    , m_mimeType(options.mimeType())
     , m_stopped(true)
     , m_ignoreMutedMedia(true)
     , m_state(State::Inactive)
     , m_dispatchScheduledEventRunner(this, &MediaRecorder::dispatchScheduledEvent)
 {
     ASSERT(m_stream->getTracks().size());
 
     // Recording remote Audio streams is not supported: http://crbug.com/121673.
     if (!stream->getAudioTracks().isEmpty() && stream->getAudioTracks()[0]->remote()) {
         context->addConsoleMessage(ConsoleMessage::create(JSMessageSource, WarningMessageLevel, "Recording remote audio tracks is not supported, ignoring them."));
@@ skipping 79 matching lines @@
 
 void MediaRecorder::requestData(ExceptionState& exceptionState)
 {
     if (m_state != State::Recording) {
         exceptionState.throwDOMException(InvalidStateError, "The MediaRecorder's state is '" + stateToString(m_state) + "'.");
         return;
     }
     writeData(nullptr /* data */, 0 /* length */, true /* lastInSlice */);
 }
 
-String MediaRecorder::canRecordMimeType(const String& mimeType)
+bool MediaRecorder::isTypeSupported(const String& type)

[philipj_slow, 2015/12/11 08:29:03]

Can you use this in the constructor, where the "the type provided ... is
unsupported" exception is thrown? That should cause an exception to be thrown by
default, highlighting the issue of having "" as the default value of mimeType.

If there needs to be any divergence between when
MediaRecorder.isTypeSupported(x) returns false and when new MediaRecorder(ms, {
mimeType: x }) throws an exception, can you document them clearly? My guess is
that there need be no such differences.

[philipj_slow, 2015/12/18 20:18:10]

I've filed https://code.google.com/p/chromium/issues/detail?id=571107

 {
     RawPtr<WebMediaRecorderHandler> handler = Platform::current()->createMediaRecorderHandler();
     if (!handler)
-        return emptyString();
+        return false;
 
-    // MediaRecorder canRecordMimeType() MUST return 'probably' "if the UA is
-    // confident that mimeType represents a type that it can record" [1], but a
-    // number of reasons could prevent the recording from happening as expected,
-    // so 'maybe' is a better option: "Implementors are encouraged to return
-    // "maybe" unless the type can be confidently established as being supported
-    // or not.". Hence this method returns '' or 'maybe', never 'probably'.
-    // [1] http://w3c.github.io/mediacapture-record/MediaRecorder.html#methods
-    return handler->canSupportMimeType(mimeType) ? "maybe" : emptyString();
+    // If true is returned from this method, it only indicates that the
+    // MediaRecorder implementation is capable of recording Blob objects for the
+    // specified MIME type. Recording may still fail if sufficient resources are
+    // not available to support the concrete media encoding.
+    // [1] https://w3c.github.io/mediacapture-record/MediaRecorder.html#methods
+    return handler->canSupportMimeType(type);
 }
 
 const AtomicString& MediaRecorder::interfaceName() const
 {
     return EventTargetNames::MediaRecorder;
 }
 
 ExecutionContext* MediaRecorder::executionContext() const
 {
     return ActiveDOMObject::executionContext();
@@ skipping 103 matching lines @@
 
 DEFINE_TRACE(MediaRecorder)
 {
     visitor->trace(m_stream);
     visitor->trace(m_scheduledEvents);
     RefCountedGarbageCollectedEventTargetWithInlineData<MediaRecorder>::trace(visitor);
     ActiveDOMObject::trace(visitor);
 }
 
 } // namespace blink