Bug 44930 – [V8] Custom binding for "dataset"

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Kent Tamura]

[V8] Custom binding for "dataset"

[Kent Tamura]

Created attachment 66016 [details]
Patch

[Adam Barth]

Comment on attachment 66016 [details]
Patch

> 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.

[Kent Tamura]

(In reply to comment #2)
> > 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?

Do you mean throwing an exception for a case that prototype chain contains the name?
I'm not sure.  The behavior of the corresponding JSC binding is the same as this code.

> 
> > 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.

Unfortunately, it will make a build error.

../../WebKit/chromium/v8/include/v8.h: In constructor ‘v8::Handle<T>::Handle(v8::Handle<S>) [with S = v8::Primitive, T = v8::Boolean]’:
/Volumes/d2/WebKit/WebCore/WebCore.gyp/../bindings/v8/custom/V8DOMStringMapCustom.cpp:76:   instantiated from here
../../WebKit/chromium/v8/include/v8.h:212: error: invalid conversion from ‘v8::Primitive*’ to ‘v8::Boolean*’

[Adam Barth]

> > Are we supposed to "return throwError(ec)" ?  That would be a less error-prone pattern.
> 
> Unfortunately, it will make a build error.

Yeah, we'd have to change a bunch of code to make it work.  Something to think about in a future patch.

[Kent Tamura]

Landed: http://trac.webkit.org/changeset/66467