[webkit-reviews] review granted: [Bug 44930] [V8] Custom binding for "dataset" : [Attachment 66016] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Kent Tamura <tkent at chromium.org>'s
request for review:
Bug 44930: [V8] Custom binding for "dataset"
https://bugs.webkit.org/show_bug.cgi?id=44930

Attachment 66016: Patch
https://bugs.webkit.org/attachment.cgi?id=66016&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
> LayoutTests/platform/chromium/test_expectations.txt:2767
> -// v8 bindings for dataset need to be implemented.
> -BUG48133 :  fast/dom/dataset-xhtml.xhtml = TEXT
> -BUG48133 :  fast/dom/dataset.html = TEXT
Yay!

> WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp:67
> +v8::Handle<v8::Boolean>
V8DOMStringMap::namedPropertyDeleter(v8::Local<v8::String> name, const
v8::AccessorInfo& info)
> +{
> +    INC_STATS("DOM.DOMStringMap.NamedPropertyDeleter");
> +    v8::Handle<v8::Value> value =
info.Holder()->GetRealNamedPropertyInPrototypeChain(name);
Can this throw an exception?

> WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp:76
> +    ExceptionCode ec = 0;
> +   
V8DOMStringMap::toNative(info.Holder())->deleteItem(toWebCoreString(name), ec);

> +    if (ec)
> +	   throwError(ec);
Are we supposed to "return throwError(ec)" ?  That would be a less error-prone
pattern.