This is already looking good. Just a few comments.
https://codereview.chromium.org/11274014/diff/20003/src/runtime.cc File src/runtime.cc (right): https://codereview.chromium.org/11274014/diff/20003/src/runtime.cc#newcode13234 src/runtime.cc:13234: return ObjectHashTable::Allocate(0); As discussed offline, having this non-JS object escape into JavaScript (even though we fully control the code) seems fishy. The clean solution would be to wrap it into a JSValue, which is a full-fledged JSObject with one internal field. That would add an additional indirection though. But that would require quite some additional boilerplate and I am fine with landing this as it is. Just wanted to leave this comment so that we can remember there is a battle-plan if it ever hits us. I could very well imagine our fuzzer generating tests that trigger this. https://codereview.chromium.org/11274014/diff/20003/src/runtime.cc#newcode13241 src/runtime.cc:13241: ObjectHashTable* table = ObjectHashTable::cast(args[0]); Use the CONVERT_ARG_CHECKED macro here which will throw a proper exception upon type mismatch. Otherwise our fuzzer might spot crashes when it fuzzes this runtime function. https://codereview.chromium.org/11274014/diff/20003/src/runtime.cc#newcode13251 src/runtime.cc:13251: Handle<ObjectHashTable> table = args.at<ObjectHashTable>(0); Likewise. https://codereview.chromium.org/11274014/diff/20003/src/runtime.cc#newcode13262 src/runtime.cc:13262: ObjectHashTable* table = ObjectHashTable::cast(args[0]); Likewise. https://codereview.chromium.org/11274014/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
