Improve handling of dictionary conversions.

Source/bindings/v8/Dictionary.cpp

Index: Source/bindings/v8/Dictionary.cpp
diff --git a/Source/bindings/v8/Dictionary.cpp b/Source/bindings/v8/Dictionary.cpp
index 9b1050c389a5c5d881233bc7e43057fd3d084bbc..998a9ccf85783cdecb1472e56ea839fcd47bf8d8 100644
--- a/Source/bindings/v8/Dictionary.cpp
+++ b/Source/bindings/v8/Dictionary.cpp
@@ -91,6 +91,22 @@ bool Dictionary::isUndefinedOrNull() const
     return WebCore::isUndefinedOrNull(m_options);
 }
 
+bool Dictionary::hasProperty(const String& key) const
+{
+    if (isUndefinedOrNull())
+        return false;
+    v8::Local<v8::Object> options = m_options->ToObject();
+    ASSERT(!options.IsEmpty());
+
+    ASSERT(m_isolate);
+    ASSERT(m_isolate == v8::Isolate::GetCurrent());
+    v8::Handle<v8::String> v8Key = v8String(key, m_isolate);
+    if (!options->Has(v8Key))
+        return false;
+
+    return true;
+}
+
 bool Dictionary::getKey(const String& key, v8::Local<v8::Value>& value) const
 {
     if (isUndefinedOrNull())
@@ -127,6 +143,12 @@ bool Dictionary::get(const String& key, bool& value) const
     return true;
 }
 
+bool Dictionary::convert(const String& key, bool& value, ExceptionState&) const
+{
+    get(key, value);
+    return true;
+}
+
 bool Dictionary::get(const String& key, int32_t& value) const
 {
     v8::Local<v8::Value> v8Value;
@@ -149,7 +171,7 @@ bool Dictionary::get(const String& key, double& value, bool& hasValue) const
     }
 
     hasValue = true;
-    v8::Local<v8::Number> v8Number = v8Value->ToNumber();
+    V8TRYCATCH_RETURN(v8::Local<v8::Number>, v8Number, v8Value->ToNumber(), false);
     if (v8Number.IsEmpty())
         return false;
     value = v8Number->Value();
@@ -162,6 +184,16 @@ bool Dictionary::get(const String& key, double& value) const
     return get(key, value, unused);
 }
 
+bool Dictionary::convert(const String& key, double& value, ExceptionState& exceptionState) const
+{
+    bool hasValue = false;
+    if (!get(key, value, hasValue) && hasValue) {
+        exceptionState.throwTypeError(ExceptionMessages::illTypedProperty(key, "is not of type 'double'."));

[Mike West, 2013/11/25 08:22:48]

So, as I noted in an earlier comment, I'd like to see something like
`ExceptionMessages::failedToConstruct('xxx',
ExceptionMessages::illTypedProperty(key, ...));`

It's not clear to me when these conversion methods are called, however. If it's
difficult to distinguish between construction and other types of conversion,
then that nicety might not be worth it.

[sof, 2013/11/25 11:57:23]

I like the suggestion, we should do it. I'm reasonably certain that these
conversions are currently only called upon for event constructors, but that
might well change.

I could extend the method signatures to pass in the context of use, and emit
error messages based on that, but this makes the case for a single constant
context parameter recording this & other settings, even more compelling. Will
give that a go.

[sof, 2013/11/25 23:24:05]

Done.

+        return false;
+    }
+    return true;
+}
+
 bool Dictionary::get(const String& key, String& value) const
 {
     v8::Local<v8::Value> v8Value;
@@ -173,6 +205,17 @@ bool Dictionary::get(const String& key, String& value) const
     return true;
 }
 
+bool Dictionary::convert(const String& key, String& value, ExceptionState& exceptionState) const
+{
+    v8::Local<v8::Value> v8Value;
+    if (!getKey(key, v8Value))
+        return true;
+
+    V8TRYCATCH_FOR_V8STRINGRESOURCE_RETURN(V8StringResource<>, stringValue, v8Value, false);
+    value = stringValue;
+    return true;
+}
+
 bool Dictionary::get(const String& key, ScriptValue& value) const
 {
     v8::Local<v8::Value> v8Value;
@@ -183,6 +226,12 @@ bool Dictionary::get(const String& key, ScriptValue& value) const
     return true;
 }
 
+bool Dictionary::convert(const String& key, ScriptValue& value, ExceptionState&) const
+{
+    get(key, value);
+    return true;
+}
+
 bool Dictionary::get(const String& key, unsigned short& value) const
 {
     v8::Local<v8::Value> v8Value;
@@ -241,7 +290,7 @@ bool Dictionary::get(const String& key, unsigned long long& value) const
     if (!getKey(key, v8Value))
         return false;
 
-    v8::Local<v8::Number> v8Number = v8Value->ToNumber();
+    V8TRYCATCH_RETURN(v8::Local<v8::Number>, v8Number, v8Value->ToNumber(), false);
     if (v8Number.IsEmpty())
         return false;
     double d = v8Number->Value();
@@ -290,6 +339,15 @@ bool Dictionary::get(const String& key, MessagePortArray& value) const
     return getMessagePortArray(v8Value, key, value, m_isolate);
 }
 
+bool Dictionary::convert(const String& key, const String&, bool isNullable, MessagePortArray& value, ExceptionState& exceptionState) const
+{
+    v8::Local<v8::Value> v8Value;
+    if (!getKey(key, v8Value))
+        return true;
+
+    return get(key, value);
+}
+
 bool Dictionary::get(const String& key, HashSet<AtomicString>& value) const
 {
     v8::Local<v8::Value> v8Value;
@@ -312,17 +370,29 @@ bool Dictionary::get(const String& key, HashSet<AtomicString>& value) const
     return true;
 }
 
-bool Dictionary::getWithUndefinedOrNullCheck(const String& key, String& value) const
+bool Dictionary::convert(const String& key, const String&, bool isNullable, HashSet<AtomicString>& value, ExceptionState& exceptionState) const
 {
     v8::Local<v8::Value> v8Value;
-    if (!getKey(key, v8Value) || v8Value->IsNull() || v8Value->IsUndefined())
-        return false;
+    if (!getKey(key, v8Value))
+        return true;
 
-    if (WebCore::isUndefinedOrNull(v8Value)) {
-        value = String();
+    if (isNullable && WebCore::isUndefinedOrNull(v8Value))
         return true;
+
+    if (!v8Value->IsArray()) {
+        exceptionState.throwTypeError(ExceptionMessages::notASequenceTypeProperty(key));
+        return false;
     }
 
+    return get(key, value);
+}
+
+bool Dictionary::getWithUndefinedOrNullCheck(const String& key, String& value) const
+{
+    v8::Local<v8::Value> v8Value;
+    if (!getKey(key, v8Value) || WebCore::isUndefinedOrNull(v8Value))
+        return false;
+
     V8TRYCATCH_FOR_V8STRINGRESOURCE_RETURN(V8StringResource<>, stringValue, v8Value, false);
     value = stringValue;
     return true;
@@ -484,6 +554,22 @@ bool Dictionary::get(const String& key, Dictionary& value) const
     return true;
 }
 
+bool Dictionary::convert(const String& key, const String&, bool isNullable, Dictionary& value, ExceptionState& exceptionState) const
+{
+    v8::Local<v8::Value> v8Value;
+    if (!getKey(key, v8Value))
+        return true;
+
+    if (v8Value->IsObject())
+        return get(key, value);
+
+    if (isNullable && WebCore::isUndefinedOrNull(v8Value))
+        return true;
+
+    exceptionState.throwTypeError(ExceptionMessages::illTypedProperty(key, "does not have a Dictionary type."));
+    return false;
+}
+
 bool Dictionary::get(const String& key, Vector<String>& value) const
 {
     v8::Local<v8::Value> v8Value;
@@ -503,6 +589,23 @@ bool Dictionary::get(const String& key, Vector<String>& value) const
     return true;
 }
 
+bool Dictionary::convert(const String& key, const String&, bool isNullable, Vector<String>& value, ExceptionState& exceptionState) const
+{
+    v8::Local<v8::Value> v8Value;
+    if (!getKey(key, v8Value))
+        return true;
+
+    if (isNullable && WebCore::isUndefinedOrNull(v8Value))
+        return true;
+
+    if (!v8Value->IsArray()) {
+        exceptionState.throwTypeError(ExceptionMessages::notASequenceTypeProperty(key));
+        return false;
+    }
+
+    return get(key, value);
+}
+
 bool Dictionary::get(const String& key, ArrayValue& value) const
 {
     v8::Local<v8::Value> v8Value;
@@ -518,6 +621,23 @@ bool Dictionary::get(const String& key, ArrayValue& value) const
     return true;
 }
 
+bool Dictionary::convert(const String& key, const String&, bool isNullable, ArrayValue& value, ExceptionState& exceptionState) const
+{
+    v8::Local<v8::Value> v8Value;
+    if (!getKey(key, v8Value))
+        return true;
+
+    if (isNullable && WebCore::isUndefinedOrNull(v8Value))
+        return true;
+
+    if (!v8Value->IsArray()) {
+        exceptionState.throwTypeError(ExceptionMessages::notASequenceTypeProperty(key));
+        return false;
+    }
+
+    return get(key, value);
+}
+
 bool Dictionary::get(const String& key, RefPtr<DOMError>& value) const
 {
     v8::Local<v8::Value> v8Value;