Refactor "track options" to be a dictionary.

third_party/WebKit/Source/modules/mediastream/MediaConstraintsImpl.cpp

 /*
  * 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.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 13 matching lines @@
  * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
  * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
  * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "config.h"
 
 #include "modules/mediastream/MediaConstraintsImpl.h"
+#include "modules/mediastream/MediaTrackConstraintSet.h"
 
 #include "bindings/core/v8/ArrayValue.h"
 #include "bindings/core/v8/Dictionary.h"
 #include "bindings/core/v8/ExceptionState.h"
 #include "core/dom/ExceptionCode.h"
 #include "wtf/HashMap.h"
 #include "wtf/Vector.h"
 #include "wtf/text/StringHash.h"
 
 namespace blink {
@@ skipping 61 matching lines @@
                 return false;
             optionalConstraintsVector.append(WebMediaConstraint(key, value));
         }
     }
 
     optional.assign(optionalConstraintsVector);
     mandatory.assign(mandatoryConstraintsVector);
     return true;
 }
 
+static bool parse(const MediaTrackConstraintSet& constraintsIn, WebVector<WebMediaConstraint>& optional, WebVector<WebMediaConstraint>& mandatory)
+{
+    Vector<WebMediaConstraint> mandatoryConstraintsVector;
+    if (constraintsIn.hasMandatory()) {
+        Dictionary mandatoryConstraintsDictionary = constraintsIn.mandatory();

[Peter Beverloo, 2015/10/05 13:49:09]

Parsing of |mandatoryConstraintsDictionary| and |optionalConstraintsVector| in
this method are identical to the parse() function above.

Rather than duplicating the code, could we perhaps two static (or anonymous)
functions for parsing the "mandatory" and "_optional" members, both of which are
called directly by the create() methods below?

[hta - Chromium, 2015/10/05 21:28:06]

There are subtle differences in the control flow, but significant chunks can be
extracted. Does this look better?

+
+        HashMap<String, String> mandatoryConstraintsHashMap;
+        bool ok = mandatoryConstraintsDictionary.getOwnPropertiesAsStringHashMap(mandatoryConstraintsHashMap);
+        if (!ok)
+            return false;
+
+        HashMap<String, String>::const_iterator iter = mandatoryConstraintsHashMap.begin();
+        for (; iter != mandatoryConstraintsHashMap.end(); ++iter)

[Guido Urdaneta, 2015/10/05 12:34:12]

Can you use range for here?

[hta - Chromium, 2015/10/05 12:54:01]

can you point to an examle of range for? I think that's a C++ 11 feature I've
never used.
This is copy/pasted code (I intend to remove the code I pasted from, but it
turned out to have one more user).

[Guido Urdaneta, 2015/10/05 13:00:25]

Since this is not an STL container I don't know if it range for actually
applies.
http://en.cppreference.com/w/cpp/language/range-for

+            mandatoryConstraintsVector.append(WebMediaConstraint(iter->key, iter->value));
+    }
+
+    Vector<WebMediaConstraint> optionalConstraintsVector;
+    if (constraintsIn.hasOptional()) {
+        const Vector<Dictionary> optionalConstraints = constraintsIn.optional();
+
+        for (size_t i = 0; i < optionalConstraints.size(); ++i) {

[Guido Urdaneta, 2015/10/05 12:34:12]

Range for?

+            Dictionary constraint = optionalConstraints[i];
+            if (constraint.isUndefinedOrNull())
+                return false;
+            Vector<String> localNames;
+            constraint.getPropertyNames(localNames);
+            if (localNames.size() != 1)
+                return false;
+            String key = localNames[0];
+            String value;
+            bool ok = DictionaryHelper::get(constraint, key, value);
+            if (!ok)
+                return false;
+            optionalConstraintsVector.append(WebMediaConstraint(key, value));
+        }
+    }
+
+    optional.assign(optionalConstraintsVector);
+    mandatory.assign(mandatoryConstraintsVector);
+    return true;
+}
+
 
 WebMediaConstraints create(const Dictionary& constraintsDictionary, ExceptionState& exceptionState)
 {
     WebVector<WebMediaConstraint> optional;
     WebVector<WebMediaConstraint> mandatory;
     if (!parse(constraintsDictionary, optional, mandatory)) {
         exceptionState.throwTypeError("Malformed constraints object.");
         return WebMediaConstraints();
     }
 
     WebMediaConstraints constraints;
     constraints.initialize(optional, mandatory);
     return constraints;
 }
 
+WebMediaConstraints create(const MediaTrackConstraintSet& constraintsIn, ExceptionState& exceptionState)
+{
+    WebVector<WebMediaConstraint> optional;
+    WebVector<WebMediaConstraint> mandatory;
+    if (!parse(constraintsIn, optional, mandatory)) {
+        exceptionState.throwTypeError("Malformed constraints object.");
+        return WebMediaConstraints();
+    }
+    WebMediaConstraints constraints;
+    constraints.initialize(optional, mandatory);
+    return constraints;
+}
+
 WebMediaConstraints create()
 {
     WebMediaConstraints constraints;
     constraints.initialize();
     return constraints;
 }
 
 } // namespace MediaConstraintsImpl
 } // namespace blink