Refactor "options" argument to getUserMedia to be a dictionary

Source/modules/mediastream/UserMediaRequest.cpp

 /*
  * Copyright (C) 2011 Ericsson AB. All rights reserved.
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
@@ skipping 23 matching lines @@
 #include "modules/mediastream/UserMediaRequest.h"
 
 #include "bindings/core/v8/Dictionary.h"
 #include "bindings/core/v8/ExceptionMessages.h"
 #include "bindings/core/v8/ExceptionState.h"
 #include "core/dom/Document.h"
 #include "core/dom/ExceptionCode.h"
 #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;
 
     if (audio.isNull() && video.isNull()) {
         exceptionState.throwDOMException(SyntaxError, "At least one of audio and video must be requested");
         return nullptr;
     }
 
     return new UserMediaRequest(context, controller, audio, video, successCallback, errorCallback);
 }
@@ skipping 102 matching lines @@
 
 DEFINE_TRACE(UserMediaRequest)
 {
     visitor->trace(m_controller);
     visitor->trace(m_successCallback);
     visitor->trace(m_errorCallback);
     ContextLifecycleObserver::trace(visitor);
 }
 
 } // namespace blink