Bug 92482 - [Microdata] Remove toJs() and toV8Object() custom methods from JSHTMLElementCustom.cpp and V8HTMLElementCustom.cpp respectively.
Summary: [Microdata] Remove toJs() and toV8Object() custom methods from JSHTMLElementC...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Arko Saha
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-27 03:09 PDT by Arko Saha
Modified: 2012-07-27 10:17 PDT (History)
6 users (show)

See Also:


Attachments
Patch (3.33 KB, patch)
2012-07-27 03:26 PDT, Arko Saha
haraken: review-
Details | Formatted Diff | Diff
Updated patch (4.02 KB, patch)
2012-07-27 06:38 PDT, Arko Saha
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Arko Saha 2012-07-27 03:09:59 PDT
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.
Comment 1 Arko Saha 2012-07-27 03:26:44 PDT
Created attachment 154893 [details]
Patch
Comment 2 Kentaro Hara 2012-07-27 03:35:41 PDT
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.
Comment 3 Arko Saha 2012-07-27 06:36:55 PDT
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.
Comment 4 Arko Saha 2012-07-27 06:38:07 PDT
Created attachment 154929 [details]
Updated patch
Comment 5 Kentaro Hara 2012-07-27 06:46:49 PDT
Comment on attachment 154929 [details]
Updated patch

I am sorry! I was looking at different toJS()/toV8()... You are completely right!
Comment 6 WebKit Review Bot 2012-07-27 10:17:16 PDT
Comment on attachment 154929 [details]
Updated patch

Clearing flags on attachment: 154929

Committed r123880: <http://trac.webkit.org/changeset/123880>
Comment 7 WebKit Review Bot 2012-07-27 10:17:20 PDT
All reviewed patches have been landed.  Closing bug.