binding: Makes Dictionary throw an exception (constructor).

third_party/WebKit/Source/bindings/core/v8/Dictionary.cpp

Index: third_party/WebKit/Source/bindings/core/v8/Dictionary.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/Dictionary.cpp b/third_party/WebKit/Source/bindings/core/v8/Dictionary.cpp
index 7d7442cdb7e359fe66f0a42011df553b14519e25..1b9ebfc66f590b928f2c599c5bbbe56aac624eb7 100644
--- a/third_party/WebKit/Source/bindings/core/v8/Dictionary.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/Dictionary.cpp
@@ -31,36 +31,40 @@
 
 namespace blink {
 
-Dictionary& Dictionary::operator=(const Dictionary& optionsObject) {
-  m_options = optionsObject.m_options;
-  m_isolate = optionsObject.m_isolate;
-  return *this;
-}
-
-bool Dictionary::isObject() const {
-  return !isUndefinedOrNull() && m_options->IsObject();
-}
+Dictionary::Dictionary(v8::Isolate* isolate,
+                       v8::Local<v8::Value> dictionaryObject,
+                       ExceptionState& exceptionState)
+    : m_isolate(isolate) {
+  DCHECK(isolate);
+
+  // https://heycam.github.io/webidl/#es-dictionary

[bashi, 2016/11/18 15:03:55]

I'm not sure "Dictionary" class is a "WebIDL dictionary". I think "Dictionary"
class may not conform "WebIDL dictionary" in its nature (e.g. we call [[Get]]
multiple time)

I'd prefer not adding this link. But I'll defer to you.

[Yuki, 2016/11/21 12:40:27]

Well, ... but you said that the call sites of Dictionary should call get() only
once.

I understand that this Dictionary class does NOT perfectly conform to the spec,
but this is the closest one, isn't it?  As we don't have a plan to add
IDLDictionary class so far, and as we're not perfect at many places / things, we
don't need to be so nervous about such a violation.

The reason why we check the type here and why only undefined, null and object
are allowed is just because Web IDL accepts these things for IDL Dictionary. 
Thus, the link is very informative, I think.

Don't be too nervous.  ;)

+  // Type of an ECMAScript value must be Undefined, Null or Object.
+  if (dictionaryObject.IsEmpty() || dictionaryObject->IsUndefined()) {
+    m_valueType = ValueType::Undefined;
+    return;
+  }
+  if (dictionaryObject->IsNull()) {
+    m_valueType = ValueType::Null;
+    return;
+  }
+  if (dictionaryObject->IsObject()) {
+    m_valueType = ValueType::Object;
+    m_dictionaryObject = dictionaryObject.As<v8::Object>();
+    return;
+  }
 
-bool Dictionary::isUndefinedOrNull() const {
-  if (m_options.IsEmpty())
-    return true;
-  return blink::isUndefinedOrNull(m_options);
+  exceptionState.throwTypeError(
+      "The dictionary provided is neither undefined, null nor an Object.");
 }
 
 bool Dictionary::hasProperty(const StringView& key) const {
-  v8::Local<v8::Object> object;
-  if (!toObject(object))
+  if (m_dictionaryObject.IsEmpty())
     return false;
 
-  DCHECK(m_isolate);
-  DCHECK_EQ(m_isolate, v8::Isolate::GetCurrent());
-  return v8CallBoolean(object->Has(v8Context(), v8String(m_isolate, key)));
-}
-
-bool Dictionary::get(const StringView& key, v8::Local<v8::Value>& value) const {
-  if (!m_isolate)
-    return false;
-  return getInternal(v8String(m_isolate, key), value);
+  // TODO(bashi,yukishiino): Should rethrow the exception.
+  // Has() on a revoked proxy will throw an exception.
+  return v8CallBoolean(
+      m_dictionaryObject->Has(v8Context(), v8String(m_isolate, key)));
 }
 
 DictionaryIterator Dictionary::getIterator(
@@ -72,7 +76,7 @@ DictionaryIterator Dictionary::getIterator(
   v8::Local<v8::Value> iterator;
   if (!v8Call(V8ScriptRunner::callFunction(
                   v8::Local<v8::Function>::Cast(iteratorGetter),
-                  executionContext, m_options, 0, nullptr, m_isolate),
+                  executionContext, m_dictionaryObject, 0, nullptr, m_isolate),
               iterator))
     return nullptr;
   if (!iterator->IsObject())
@@ -88,7 +92,9 @@ bool Dictionary::get(const StringView& key, Dictionary& value) const {
   if (v8Value->IsObject()) {
     ASSERT(m_isolate);
     ASSERT(m_isolate == v8::Isolate::GetCurrent());
-    value = Dictionary(m_isolate, v8Value);
+    // TODO(bashi,yukishiino): Should rethrow the exception.

[bashi, 2016/11/18 15:03:55]

nit: wouldn't it worth referring a bug entry ?

[Yuki, 2016/11/21 12:40:27]

Done.

+    TrackExceptionState exceptionState;
+    value = Dictionary(m_isolate, v8Value, exceptionState);
   }
 
   return true;
@@ -96,19 +102,16 @@ bool Dictionary::get(const StringView& key, Dictionary& value) const {
 
 bool Dictionary::getInternal(const v8::Local<v8::Value>& key,
                              v8::Local<v8::Value>& result) const {
-  v8::Local<v8::Object> object;
-  if (!toObject(object))
+  if (m_dictionaryObject.IsEmpty())
     return false;
 
-  DCHECK(m_isolate);
-  DCHECK_EQ(m_isolate, v8::Isolate::GetCurrent());
-  if (!v8CallBoolean(object->Has(v8Context(), key)))
+  if (!v8CallBoolean(m_dictionaryObject->Has(v8Context(), key)))
     return false;
 
   // Swallow a possible exception in v8::Object::Get().
   // TODO(bashi,yukishiino): Should rethrow the exception.
   v8::TryCatch tryCatch(isolate());
-  return object->Get(v8Context(), key).ToLocal(&result);
+  return m_dictionaryObject->Get(v8Context(), key).ToLocal(&result);
 }
 
 static inline bool propertyKey(v8::Local<v8::Context> v8Context,
@@ -123,12 +126,12 @@ static inline bool propertyKey(v8::Local<v8::Context> v8Context,
 
 bool Dictionary::getOwnPropertiesAsStringHashMap(
     HashMap<String, String>& hashMap) const {
-  v8::Local<v8::Object> object;
-  if (!toObject(object))
+  if (m_dictionaryObject.IsEmpty())
     return false;
 
   v8::Local<v8::Array> properties;
-  if (!object->GetOwnPropertyNames(v8Context()).ToLocal(&properties))
+  if (!m_dictionaryObject->GetOwnPropertyNames(v8Context())
+           .ToLocal(&properties))
     return false;
   // Swallow a possible exception in v8::Object::Get().
   // TODO(bashi,yukishiino): Should rethrow the exception.
@@ -138,11 +141,11 @@ bool Dictionary::getOwnPropertiesAsStringHashMap(
     v8::Local<v8::String> key;
     if (!propertyKey(v8Context(), properties, i, key))
       continue;
-    if (!v8CallBoolean(object->Has(v8Context(), key)))
+    if (!v8CallBoolean(m_dictionaryObject->Has(v8Context(), key)))
       continue;
 
     v8::Local<v8::Value> value;
-    if (!object->Get(v8Context(), key).ToLocal(&value)) {
+    if (!m_dictionaryObject->Get(v8Context(), key).ToLocal(&value)) {
       tryCatch.Reset();
       continue;
     }
@@ -156,18 +159,17 @@ bool Dictionary::getOwnPropertiesAsStringHashMap(
 }
 
 bool Dictionary::getPropertyNames(Vector<String>& names) const {
-  v8::Local<v8::Object> object;
-  if (!toObject(object))
+  if (m_dictionaryObject.IsEmpty())
     return false;
 
   v8::Local<v8::Array> properties;
-  if (!object->GetPropertyNames(v8Context()).ToLocal(&properties))
+  if (!m_dictionaryObject->GetPropertyNames(v8Context()).ToLocal(&properties))
     return false;
   for (uint32_t i = 0; i < properties->Length(); ++i) {
     v8::Local<v8::String> key;
     if (!propertyKey(v8Context(), properties, i, key))
       continue;
-    if (!v8CallBoolean(object->Has(v8Context(), key)))
+    if (!v8CallBoolean(m_dictionaryObject->Has(v8Context(), key)))
       continue;
     TOSTRING_DEFAULT(V8StringResource<>, stringKey, key, false);
     names.append(stringKey);
@@ -176,9 +178,4 @@ bool Dictionary::getPropertyNames(Vector<String>& names) const {
   return true;
 }
 
-bool Dictionary::toObject(v8::Local<v8::Object>& object) const {
-  return !isUndefinedOrNull() &&
-         m_options->ToObject(v8Context()).ToLocal(&object);
-}
-
 }  // namespace blink