bindings: Add support for the record<K,V> WebIDL type.

third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h

Index: third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h
diff --git a/third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h b/third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h
index 7dfb3fb50b7d23fed67908e571dd9e1170078899..69b36e28936a73c589537d9e54fd44ea7f6511cf 100644
--- a/third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h
+++ b/third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h
@@ -305,6 +305,143 @@ struct NativeValueTraits<IDLSequence<T>>
   }
 };
 
+// Records
+template <typename K, typename V>
+struct NativeValueTraits<IDLRecord<K, V>>
+    : public NativeValueTraitsBase<IDLRecord<K, V>> {
+  // Nondependent types need to be explicitly qualified to be accessible.
+  using typename NativeValueTraitsBase<IDLRecord<K, V>>::ImplType;
+
+  // Converts a JavaScript value |O| to an IDL record<K, V> value. In C++, a
+  // record is represented as a Vector<std::pair<k, v>> (or a HeapVector if
+  // necessary). See https://heycam.github.io/webidl/#es-record.
+  static inline ImplType nativeValue(v8::Isolate* isolate,
+                                     v8::Local<v8::Value> v8Value,
+                                     ExceptionState& exceptionState) {
+    v8::Local<v8::Context> context = isolate->GetCurrentContext();
+
+    // "1. If Type(O) is not Object, throw a TypeError."
+    if (!v8Value->IsObject()) {
+      exceptionState.throwTypeError(
+          "Only objects can be converted to record<K,V> types");
+      return ImplType();
+    }
+    v8::Local<v8::Object> v8Object = v8::Local<v8::Object>::Cast(v8Value);
+    v8::TryCatch block(isolate);
+
+    // "3. Let keys be ? O.[[OwnPropertyKeys]]()."
+    v8::Local<v8::Array> keys;
+    // While we could pass v8::ONLY_ENUMERABLE below, doing so breaks
+    // web-platform-tests' headers-record.html and deviates from the spec
+    // algorithm.
+    // Symbols are being skipped due to
+    // https://github.com/heycam/webidl/issues/294.
+    if (!v8Object
+             ->GetOwnPropertyNames(context,
+                                   static_cast<v8::PropertyFilter>(
+                                       v8::PropertyFilter::ALL_PROPERTIES |
+                                       v8::PropertyFilter::SKIP_SYMBOLS))
+             .ToLocal(&keys)) {
+      exceptionState.rethrowV8Exception(block.Exception());
+      return ImplType();
+    }
+    if (keys->Length() > ImplType::maxCapacity()) {
+      exceptionState.throwRangeError("Array length exceeds supported limit.");
+      return ImplType();
+    }
+
+    // "2. Let result be a new empty instance of record<K, V>."
+    ImplType result;
+    result.reserveInitialCapacity(keys->Length());
+
+    // The conversion algorithm needs a data structure with fast insertion at
+    // the end while at the same time requiring fast checks for previous insert
+    // of a given key. |seenKeys| is a key/position in |result| map that aids in
+    // the latter part.
+    HashMap<String, size_t> seenKeys;
+
+    for (uint32_t i = 0; i < keys->Length(); ++i) {
+      // "4. Repeat, for each element key of keys in List order:"
+      v8::Local<v8::Value> key;
+      if (!keys->Get(context, i).ToLocal(&key)) {
+        exceptionState.rethrowV8Exception(block.Exception());
+        return ImplType();
+      }
+
+      // "4.1. Let desc be ? O.[[GetOwnProperty]](key)."
+      v8::Local<v8::Value> desc;
+      if (!v8Object
+               ->GetOwnPropertyDescriptor(
+                   context, key->ToString(context).ToLocalChecked())
+               .ToLocal(&desc)) {
+        exceptionState.rethrowV8Exception(block.Exception());
+        return ImplType();
+      }
+
+      // "4.2. If desc is not undefined and desc.[[Enumerable]] is true:"
+      if (!isPropertyEnumerable(isolate, desc, exceptionState)) {
+        if (exceptionState.hadException())
+          return ImplType();
+        continue;
+      }
+
+      // "4.2.1. Let typedKey be key converted to an IDL value of type K."
+      String typedKey =
+          NativeValueTraits<K>::nativeValue(isolate, key, exceptionState);
+      if (exceptionState.hadException())
+        return ImplType();
+
+      // "4.2.2. Let value be ? Get(O, key)."
+      v8::Local<v8::Value> value;
+      if (!v8Object->Get(context, key).ToLocal(&value)) {
+        exceptionState.rethrowV8Exception(block.Exception());
+        return ImplType();
+      }
+
+      // "4.2.3. Let typedValue be value converted to an IDL value of type V."
+      typename ImplType::ValueType::second_type typedValue =
+          NativeValueTraits<V>::nativeValue(isolate, value, exceptionState);
+      if (exceptionState.hadException())
+        return ImplType();
+
+      if (seenKeys.contains(typedKey)) {
+        // "4.2.4. If typedKey is already a key in result, set its value to
+        //         typedValue.
+        //         Note: This can happen when O is a proxy object."
+        const size_t pos = seenKeys.at(typedKey);
+        result[pos] = std::make_pair(typedKey, typedValue);
+      } else {
+        // "4.2.5. Otherwise, append to result a mapping (typedKey,
+        // typedValue)."
+        // Note we can take this shortcut because we are always appending.
+        const size_t pos = result.size();
+        seenKeys.set(typedKey, pos);
+        result.uncheckedAppend(std::make_pair(typedKey, typedValue));
+      }
+    }
+    // "5. Return result."
+    return result;
+  }
+
+ private:
+  static bool isPropertyEnumerable(v8::Isolate* isolate,
+                                   v8::Local<v8::Value> descriptor,
+                                   ExceptionState& exceptionState) {
+    if (!descriptor->IsObject())
+      return false;
+    v8::TryCatch block(isolate);
+    v8::Local<v8::Value> enumerable;
+    if (!v8::Local<v8::Object>::Cast(descriptor)
+             ->Get(isolate->GetCurrentContext(),
+                   v8String(isolate, "enumerable"))
+             .ToLocal(&enumerable)) {

[Yuki, 2017/03/10 06:54:32]

You can safely use ToLocalChecked() here because |descriptor| must be a property
descriptor returned by V8, i.e. |descriptor| never be an author-script-defined
object.

Thus, there must be a property named "enumerable" and whose value must be a
boolean.

Get() could fail if OOM, etc., but in that case, it's okay to crash with
ToLocalChecked.

[Raphael Kubo da Costa (rakuco), 2017/03/10 08:19:09]

Are you sure about this? One of the tests I added to NativeValueTraitsImplTests
uses a Proxy to intercept calls to Object.getOwnPropertyDescriptor and return an
object that throws an exception when its "enumerable" property is accessed (the
same could be done in a layout test).

https://tc39.github.io/ecma262/#sec-topropertydescriptor also does not seem to
make any guarantees that "enumerable" is always present, only that it is
converted to a boolean.

While my test causes the GetOwnPropertyDescriptor() call to fail instead of this
one, it could also be modified to crash here, so using ToLocalChecked() here
would likely crash Chromium as well as my test when authors are doing something
nasty.

[Yuki, 2017/03/10 09:52:21]

I didn't test anything for myself, so I can be wrong.  Please correct me
anything, and help me understand things better.

Proxy.getOwnPropertyDescriptor can throw an exception, but it should happen
during we call v8::Object::GetOwnPropertyDescriptor, right?  It's not happening
when we call v8::Object::Get(descriptor, "enumerable").  IIUC, the property
descriptor itself is created by V8 and filled with the result of
Proxy.getOwnPropertyDescriptor.  At least, the ES2017 seems saying so.

https://tc39.github.io/ecma262/#sec-proxy-object-internal-methods-and-interna...
where resultDesc is NOT created by author script.

Plus, CompletePropertyDescriptor(resultDesc) fills "enumerable" if it's not yet
set.  So, it must exist.

I didn't investigate V8's implementation, so something could be different from
the spec (though, I think that V8 follows the spec well in general).

Anyway, this is my thought.

+      exceptionState.rethrowV8Exception(block.Exception());
+      return false;
+    }
+    return toBoolean(isolate, enumerable, exceptionState);

[Yuki, 2017/03/10 06:54:32]

This is okay, but you can write
    return enumerable.BooleanValue(context).ToChecked();
given that it's guaranteed to be v8::Boolean.
And in this way, you can get rid of ExceptionState.

[Raphael Kubo da Costa (rakuco), 2017/03/10 08:19:09]

Same here: while V8 itself performs some boolean conversions in
GetOwnPropertyDescriptor(), wouldn't it still be possible for this to fail given
users can be in control of the object returned by GetOwnPropertyDescriptor()?

[Yuki, 2017/03/10 09:52:21]

IIUC, there is no way for author script to install any accessor property into a
property descriptor.

resultDesc is first created as an empty property descriptor, and gets filled
with trapResultObj.  trapResultObj may have accessor properties, but resultDesc
may not, I think.

[Raphael Kubo da Costa (rakuco), 2017/03/10 10:42:37]

You're right about those 2 points: while the proxy handler's
getOwnPropertyDescriptor might return a bogus object, the object returned by
Proxy.getOwnPropertyDescriptor is sanitized and not created by a user. I'll
upload a new patch with fewer checks.

+  }
+};
+
 }  // namespace blink
 
 #endif  // NativeValueTraitsImpl_h