binding: Lets Dictionary::getPropertyNames, etc. rethrow an exception.

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 b7e0b549d0890086718a28386f405d4659c18aee..c4ba45779a9089a3b677effb867b0f1d2f60ffb0 100644
--- a/third_party/WebKit/Source/bindings/core/v8/Dictionary.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/Dictionary.cpp
@@ -117,44 +117,52 @@ bool Dictionary::getInternal(const v8::Local<v8::Value>& key,
   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))
+// Returns a v8::String value in |value| on success, otherwise this function
+// lets an exception be thrown.  It's the caller's duty to catch the exception.

[haraken, 2016/11/24 07:22:30]

Then would it be better to make the method return a MaybeLocal handle?

[Yuki, 2016/11/24 10:19:32]

Done.

+WARN_UNUSED_RESULT static bool getStringValueInArray(
+    v8::Local<v8::Context> v8Context,
+    v8::Local<v8::Array> array,
+    uint32_t index,
+    v8::Local<v8::String>& value) {
+  v8::Local<v8::Value> v8Value;
+  if (!array->Get(v8Context, index).ToLocal(&v8Value))
     return false;
-  return property->ToString(v8Context).ToLocal(&key);
+  return v8Value->ToString(v8Context).ToLocal(&value);
 }
 
 bool Dictionary::getOwnPropertiesAsStringHashMap(
-    HashMap<String, String>& hashMap) const {
+    HashMap<String, String>& hashMap,
+    ExceptionState& exceptionState) const {
   if (m_dictionaryObject.IsEmpty())
     return false;
 
-  v8::Local<v8::Array> properties;
+  v8::TryCatch tryCatch(isolate());
+  v8::Local<v8::Array> propertyNames;
   if (!m_dictionaryObject->GetOwnPropertyNames(v8Context())
-           .ToLocal(&properties))
+           .ToLocal(&propertyNames)) {
+    exceptionState.rethrowV8Exception(tryCatch.Exception());
     return false;
-  // Swallow a possible exception in v8::Object::Get().
-  // TODO(bashi,yukishiino): Should rethrow the exception.
-  // Note that propertyKey() may throw an exception.
-  // http://crbug.com/666661
-  v8::TryCatch tryCatch(isolate());
-  for (uint32_t i = 0; i < properties->Length(); ++i) {
+  }
+
+  for (uint32_t i = 0; i < propertyNames->Length(); ++i) {
     v8::Local<v8::String> key;
-    if (!propertyKey(v8Context(), properties, i, key))
-      continue;
-    if (!v8CallBoolean(m_dictionaryObject->Has(v8Context(), key)))
-      continue;
+    if (!getStringValueInArray(v8Context(), propertyNames, i, key)) {
+      exceptionState.rethrowV8Exception(tryCatch.Exception());
+      return false;
+    }
+    V8StringResource<> stringKey(key);
+    if (!stringKey.prepare(isolate(), exceptionState))
+      return false;
 
     v8::Local<v8::Value> value;
     if (!m_dictionaryObject->Get(v8Context(), key).ToLocal(&value)) {
-      tryCatch.Reset();
-      continue;
+      exceptionState.rethrowV8Exception(tryCatch.Exception());
+      return false;
     }
-    TOSTRING_DEFAULT(V8StringResource<>, stringKey, key, false);
-    TOSTRING_DEFAULT(V8StringResource<>, stringValue, value, false);
+    V8StringResource<> stringValue(value);
+    if (!stringValue.prepare(isolate(), exceptionState))
+      return false;
+
     if (!static_cast<const String&>(stringKey).isEmpty())
       hashMap.set(stringKey, stringValue);
   }
@@ -162,20 +170,29 @@ bool Dictionary::getOwnPropertiesAsStringHashMap(
   return true;
 }
 
-bool Dictionary::getPropertyNames(Vector<String>& names) const {
+bool Dictionary::getPropertyNames(Vector<String>& names,
+                                  ExceptionState& exceptionState) const {
   if (m_dictionaryObject.IsEmpty())
     return false;
 
-  v8::Local<v8::Array> properties;
-  if (!m_dictionaryObject->GetPropertyNames(v8Context()).ToLocal(&properties))
+  v8::TryCatch tryCatch(isolate());
+  v8::Local<v8::Array> propertyNames;
+  if (!m_dictionaryObject->GetPropertyNames(v8Context())
+           .ToLocal(&propertyNames)) {
+    exceptionState.rethrowV8Exception(tryCatch.Exception());
     return false;
-  for (uint32_t i = 0; i < properties->Length(); ++i) {
+  }
+
+  for (uint32_t i = 0; i < propertyNames->Length(); ++i) {
     v8::Local<v8::String> key;
-    if (!propertyKey(v8Context(), properties, i, key))
-      continue;
-    if (!v8CallBoolean(m_dictionaryObject->Has(v8Context(), key)))
-      continue;
-    TOSTRING_DEFAULT(V8StringResource<>, stringKey, key, false);
+    if (!getStringValueInArray(v8Context(), propertyNames, i, key)) {
+      exceptionState.rethrowV8Exception(tryCatch.Exception());
+      return false;
+    }
+    V8StringResource<> stringKey(key);
+    if (!stringKey.prepare(isolate(), exceptionState))
+      return false;
+
     names.append(stringKey);
   }