binding: Makes Dictionary handle a possible exception in [[Get]].

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 48 matching lines @@
 
 bool Dictionary::get(const StringView& key, v8::Local<v8::Value>& value) const {
   if (!m_isolate)
     return false;
   return getInternal(v8String(m_isolate, key), value);
 }
 
 DictionaryIterator Dictionary::getIterator(
     ExecutionContext* executionContext) const {
   v8::Local<v8::Value> iteratorGetter;
-  // TODO(alancutter): Support callable objects as well as functions.
   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),
               iterator))
     return nullptr;
   if (!iterator->IsObject())
@@ skipping 18 matching lines @@
 bool Dictionary::getInternal(const v8::Local<v8::Value>& key,
                              v8::Local<v8::Value>& result) const {
   v8::Local<v8::Object> object;
   if (!toObject(object))
     return false;
 
   DCHECK(m_isolate);
   DCHECK_EQ(m_isolate, v8::Isolate::GetCurrent());
   if (!v8CallBoolean(object->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);
 }
 
 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))
     return false;
 
   v8::Local<v8::Array> properties;
   if (!object->GetOwnPropertyNames(v8Context()).ToLocal(&properties))
     return false;
+  // Swallow a possible exception in v8::Object::Get().
+  // TODO(bashi,yukishiino): Should rethrow the 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)))

[haraken, 2016/11/12 07:45:13]

Just to confirm: The line 138 and 140 never throws an exception, right?

[Yuki, 2016/11/14 06:57:25]

I expected so, but after talking with bashi@, I recognized it could throw.

IMHO, the right way to go is that
a) Stop using v8Call macros, and use ToLocal or To directly.
b) If a Maybe or MaybeLocal is empty, there must be an exception.
Then, we can correctly detect a possible exception.

Added another (short) comment in TODO.

Actually there should be no case that propertyKey() throws, so don't worry too
much for now.

[haraken, 2016/11/14 07:13:25]

Sounds reasonable. Our plan is to deprecate v8Call() and use To/ToLocal instead.

       continue;
 
     v8::Local<v8::Value> value;
-    if (!object->Get(v8Context(), key).ToLocal(&value))
+    if (!object->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 {
@@ skipping 16 matching lines @@
 
   return true;
 }
 
 bool Dictionary::toObject(v8::Local<v8::Object>& object) const {
   return !isUndefinedOrNull() &&
          m_options->ToObject(v8Context()).ToLocal(&object);
 }
 
 }  // namespace blink