Refactor "options" argument to getUserMedia to be a dictionary

Source/modules/mediastream/UserMediaRequest.cpp

Index: Source/modules/mediastream/UserMediaRequest.cpp
diff --git a/Source/modules/mediastream/UserMediaRequest.cpp b/Source/modules/mediastream/UserMediaRequest.cpp
index 4262a2cbd7adc1d9c9882b688c419f17047d03ba..e4bb565d0764fd884a12797ab503b9084e045776 100644
--- a/Source/modules/mediastream/UserMediaRequest.cpp
+++ b/Source/modules/mediastream/UserMediaRequest.cpp
@@ -41,37 +41,36 @@
 #include "core/dom/SpaceSplitString.h"
 #include "modules/mediastream/MediaConstraintsImpl.h"
 #include "modules/mediastream/MediaStream.h"
+#include "modules/mediastream/MediaStreamConstraints.h"
 #include "modules/mediastream/UserMediaController.h"
 #include "platform/mediastream/MediaStreamCenter.h"
 #include "platform/mediastream/MediaStreamDescriptor.h"
 
 namespace blink {
 
-static WebMediaConstraints parseOptions(const Dictionary& options, const String& mediaType, ExceptionState& exceptionState)
+static WebMediaConstraints parseOptions(const BooleanOrDictionary& options, ExceptionState& exceptionState)
 {
     WebMediaConstraints constraints;
 
     Dictionary constraintsDictionary;
-    bool ok = options.get(mediaType, constraintsDictionary);
-    if (ok && !constraintsDictionary.isUndefinedOrNull())
-        constraints = MediaConstraintsImpl::create(constraintsDictionary, exceptionState);
-    else {
-        bool mediaRequested = false;
-        DictionaryHelper::get(options, mediaType, mediaRequested);
-        if (mediaRequested)
+    if (options.isDictionary()) {
+        constraints = MediaConstraintsImpl::create(options.getAsDictionary(), exceptionState);
+    } else {
+        if (options.getAsBoolean()) {

[tommi (sloooow) - chröme, 2015/08/20 08:05:28]

} else if (options.getAsBoolean()) {

             constraints = MediaConstraintsImpl::create();
+        }
     }

[tommi (sloooow) - chröme, 2015/08/20 08:05:28]

} else {
  ASSERT(false);
}

(assuming options will always be boolean or a dictionary)

[hta - Chromium, 2015/08/20 08:20:32]

options.getAsBoolean actually calls ASSERT(isBoolean()) already.

I find this a little hard to read, because the hidden inner "else" for
getAsBoolean returning false is "leave constraints alone, don't touch it", which
is logical, but not highly visible.

The argument for using nested rather than chained ifs is that the outer if is a
type switch, while the inner if is a switch on the value of getAsBoolean; I
don't like if/else chains that check on completely different properties.

 
     return constraints;
 }
 
-UserMediaRequest* UserMediaRequest::create(ExecutionContext* context, UserMediaController* controller, const Dictionary& options, NavigatorUserMediaSuccessCallback* successCallback, NavigatorUserMediaErrorCallback* errorCallback, ExceptionState& exceptionState)
+UserMediaRequest* UserMediaRequest::create(ExecutionContext* context, UserMediaController* controller, const MediaStreamConstraints& options, NavigatorUserMediaSuccessCallback* successCallback, NavigatorUserMediaErrorCallback* errorCallback, ExceptionState& exceptionState)
 {
-    WebMediaConstraints audio = parseOptions(options, "audio", exceptionState);
+    WebMediaConstraints audio = parseOptions(options.audio(), exceptionState);
     if (exceptionState.hadException())
         return nullptr;
 
-    WebMediaConstraints video = parseOptions(options, "video", exceptionState);
+    WebMediaConstraints video = parseOptions(options.video(), exceptionState);
     if (exceptionState.hadException())
         return nullptr;