Bug 92482 – [Microdata] Remove toJs() and toV8Object() custom methods from JSHTMLElementCustom.cpp and V8HTMLElementCustom.cpp respectively.

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Arko Saha]

Remove toJs() and toV8Object() custom methods from JSHTMLElementCustom.cpp and V8HTMLElementCustom.cpp respectively.
We should use toJS() method defined in JSMicroDataItemValue.h and toV8() method defined in V8MicroDataItemValue.h in place of custom toJs() and toV8Object() methods respectively.

[Arko Saha]

Created attachment 154893 [details]
Patch

[Kentaro Hara]

Comment on attachment 154893 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154893&action=review

> Source/WebCore/ChangeLog:12
> +        No new tests. Just removed custom JS methods.

I think this patch would change the current behavior. Would you add a test or list up a couple of existing tests affected by this change?

For example, auto generated V8 code looks like this:

    inline v8::Handle<v8::Value> toV8(PassRefPtr< MicroDataItemValue > impl, v8::Isolate* isolate = 0) { return toV8(impl.get(), isolate); }

which is different from the current toV8() in custom bindings. Auto generated toJS() is also different from the current toJS() in custom bindings.

[Arko Saha]

Thanks haraken for the review.

(In reply to comment #2)
> > Source/WebCore/ChangeLog:12
> > +        No new tests. Just removed custom JS methods.
> 
> I think this patch would change the current behavior. Would you add a test or list up a couple of existing tests affected by this change?

This patch will not change the current behavior. Added list of existing test cases to ensure the same.

> For example, auto generated V8 code looks like this:
> 
>     inline v8::Handle<v8::Value> toV8(PassRefPtr< MicroDataItemValue > impl, v8::Isolate* isolate = 0) { return toV8(impl.get(), isolate); }
> 
> which is different from the current toV8() in custom bindings. Auto generated toJS() is also different from the current toJS() in custom bindings.

MicroDataItemList interface [MicroDataItemList.idl] has CustomToJSObject property specified. CustomToJSObject property allows us to write custom toJS() or toV8(). We have the same toV8() custom binding defined in V8MicroDataItemValueCustom.cpp : http://trac.webkit.org/browser/trunk/Source/WebCore/bindings/v8/custom/V8MicroDataItemValueCustom.cpp#L42.

[Arko Saha]

Created attachment 154929 [details]
Updated patch

[Kentaro Hara]

Comment on attachment 154929 [details]
Updated patch

I am sorry! I was looking at different toJS()/toV8()... You are completely right!

[WebKit Review Bot]

Comment on attachment 154929 [details]
Updated patch

Clearing flags on attachment: 154929

Committed r123880: <http://trac.webkit.org/changeset/123880>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.