binding: Makes Dictionary throw an exception (constructor).

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

 /*
  * Copyright (C) 2010 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 @@
  */
 
 #include "bindings/core/v8/Dictionary.h"
 
 #include "bindings/core/v8/V8ScriptRunner.h"
 #include "bindings/core/v8/V8StringResource.h"
 #include "core/dom/ExecutionContext.h"
 
 namespace blink {
 
-Dictionary& Dictionary::operator=(const Dictionary& optionsObject) {
-  m_options = optionsObject.m_options;
-  m_isolate = optionsObject.m_isolate;
-  return *this;
-}
+Dictionary::Dictionary(v8::Isolate* isolate,
+                       v8::Local<v8::Value> dictionaryObject,
+                       ExceptionState& exceptionState)
+    : m_isolate(isolate) {
+  DCHECK(isolate);
 
-bool Dictionary::isObject() const {
-  return !isUndefinedOrNull() && m_options->IsObject();
-}
+  // 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(
     ExecutionContext* executionContext) const {
   v8::Local<v8::Value> iteratorGetter;
   if (!getInternal(v8::Symbol::GetIterator(m_isolate), iteratorGetter) ||
       !iteratorGetter->IsFunction())
     return nullptr;
   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())
     return nullptr;
   return DictionaryIterator(v8::Local<v8::Object>::Cast(iterator), m_isolate);
 }
 
 bool Dictionary::get(const StringView& key, Dictionary& value) const {
   v8::Local<v8::Value> v8Value;
   if (!get(key, v8Value))
     return false;
 
   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;
 }
 
 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,
                                v8::Local<v8::Array> properties,
                                uint32_t index,
                                v8::Local<v8::String>& key) {
   v8::Local<v8::Value> property;
   if (!properties->Get(v8Context, index).ToLocal(&property))
     return false;
   return property->ToString(v8Context).ToLocal(&key);
 }
 
 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.
   // Note that propertyKey() may throw an exception.
   v8::TryCatch tryCatch(isolate());
   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;
 
     v8::Local<v8::Value> value;
-    if (!object->Get(v8Context(), key).ToLocal(&value)) {
+    if (!m_dictionaryObject->Get(v8Context(), key).ToLocal(&value)) {
       tryCatch.Reset();
       continue;
     }
     TOSTRING_DEFAULT(V8StringResource<>, stringKey, key, false);
     TOSTRING_DEFAULT(V8StringResource<>, stringValue, value, false);
     if (!static_cast<const String&>(stringKey).isEmpty())
       hashMap.set(stringKey, stringValue);
   }
 
   return true;
 }
 
 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);
   }
 
   return true;
 }
 
-bool Dictionary::toObject(v8::Local<v8::Object>& object) const {
-  return !isUndefinedOrNull() &&
-         m_options->ToObject(v8Context()).ToLocal(&object);
-}
-
 }  // namespace blink