This has changed, there's currently only one failing test (and the failure triggers through Call, not TryCall):

imported/wpt/web-animations/interfaces/KeyframeEffect/constructor.html	

#
# Fatal error in ../../v8/src/execution.cc, line 143
# Check failed: !isolate->has_scheduled_exception().
#

==== C stack trace ===============================

 1: V8_Fatal
 2: v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*)
 3: v8::internal::Object::GetPropertyWithDefinedGetter(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::JSReceiver>)
 4: v8::internal::Object::GetPropertyWithAccessor(v8::internal::LookupIterator*)
 5: v8::internal::Object::GetProperty(v8::internal::LookupIterator*)
 6: v8::internal::Runtime::GetObjectProperty(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, bool)
 7: v8::Object::Get(v8::Local<v8::Context>, v8::Local<v8::Value>)
 8: blink::Dictionary::getInternal(v8::Local<v8::Value> const&, v8::Local<v8::Value>&) const
 9: blink::Dictionary::getKey(WTF::String const&, v8::Local<v8::Value>&) const
10: blink::Dictionary::get(WTF::String const&, v8::Local<v8::Value>&) const
11: 0x7f78f5ae2a87
12: bool blink::DictionaryHelper::get<WTF::String>(blink::Dictionary const&, WTF::String const&, WTF::String&)
13: blink::EffectInput::convertObjectForm(blink::Element&, blink::Dictionary const&, blink::ExceptionState&)
14: blink::EffectInput::convert(blink::Element*, blink::EffectModelOrDictionarySequenceOrDictionary const&, blink::ExecutionContext*, blink::ExceptionState&)
15: blink::KeyframeEffect::create(blink::ExecutionContext*, blink::Element*, blink::EffectModelOrDictionarySequenceOrDictionary const&, blink::ExceptionState&)
16: 0x7f78f5fa5e09
17: 0x7f78f5f972a7
18: 0x7f78f5f9719a
19: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&))
20: 0x7f78fb05902d
21: 0x7f78fb0572fa
22: v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*)
23: 0x3a27375063a7

Minimized testcase:

---

var target = document.getElementById("target");
new KeyframeEffect(target, { get left() { throw new Error() }})

+haraken

looks like an exception is getting ignored in bindings code?

To trigger the CHECK failure, v8 also needs to be patched as in https://codereview.chromium.org/2215663002/

I did some more digging, and the problem seems to be that the exception is thrown for the first time in  DictionaryHelper::get() at [1]. If that call fails, it is immediately reattempted three lines later, but without clearing the earlier exception. That leads to a JS call (Dictionary::get->Object::GetProperty->Execution::Call) with an existing scheduled exception.

Not sure where the proper place to handle the exception is, but I'm guessing either in blink::Dictionary::get* or blink::EffectInput::convertObjectForm?

[1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/animation/EffectInput.cpp?l=257

Thanks, your analysis looks correct. Hmm, it looks like that there're many places where we call Dictionary::get without handling/clearing exceptions...

bashi-san: Would you mind taking a look at this?

What should we do when get() returns false? What does a failure suppose to mean? Just missing a property? Or an exception thrown? How to handle failure? These seems depend on situations. I think it's callers responsibility to handle/clear exceptions.

Well, that's more or less for binding to decide, isn't it?

Either Dictionary::get is returning false if there's a pending exception, and then all callers have to check for exceptions everytime they call get() or false means just the element couldn't be retrieved for whatever reason, but then it should clear the exception internally.

Kentaro, who owns the Dictionary implementation in bindings?

bashi@ is an expert of Dictionary.

One option would be make callers always use Dictionary::get(..., ExceptionState&). If the exceptionState is set, it means that an exception has been thrown and thus the caller needs to handle it. The return value of get() means if the value is found or not. (It should return false if the exceptionState is set.)

(It will require a lot of replacement work though.)

re-assigning to bashi@ per comment #10

#10 seems a right fix to me but I'd prefer not to use Dictionary class if possible as we have IDL dictionaries now.