Bug 69156 – Implicitly add toString and valueOf to prototype when convertToType callback is provided

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Hahnenberg]

Due to the fact that JSCallbackObject overrides JSCell::toString, I don't believe we can do a simple refactoring to de-virtualize toString like we did with the other conversion methods.  We'll just add it to the method table, at least for now.  The first step in doing this is to create a static version of toString in the classes that override toString and have the virtual versions call it.

[Gavin Barraclough]

I wonder if we really need to do this?

ToString applied to Object (JSObject::toString) will try looking up a default value using the object's toString property, if one exists.  For API objects that define a custom toString we could implicitly add a toString property to the object that is a host function that would call the API class's toString callback.  This would allow us to retire the ability to override toString, and make API object behave more like normal objects.

[Geoffrey Garen]

ggaren: mhahnenberg: i'd suggest a two-step process:
[9:51pm] ggaren: mhahnenberg: (1) change API classes to supply a custom toString and valueOf on the class's prototype, which forwards to classDefinition.convertToType, if classDefinition.convertToType is supplied, and add tests to testapi verifying this behavior.
[9:52pm] ggaren: mhahnenberg: (2) make toString non-virtual
[9:53pm] ggaren: mhahnenberg: internally, you can reuse JSCallbackFunction to implement (1)
[9:54pm] ggaren: mhahnenberg: you can have a shared JSObjectCallAsFunctionCallback for toString, and another for valueOf
[9:54pm] ggaren: mhahnenberg: and they just take the 'this' object supplied, check that it inherits from JSCallbackObject, cast to JSCallbackObject, grab the convertToType function out of the JSCallbackObject's class, and call it, passing the right JSType parameter

[Mark Hahnenberg]

Created attachment 109561 [details]
Patch

[Mark Hahnenberg]

Created attachment 109566 [details]
Test modification

[Geoffrey Garen]

Comment on attachment 109566 [details]
Test modification

r=me

[WebKit Review Bot]

Comment on attachment 109566 [details]
Test modification

Clearing flags on attachment: 109566

Committed r96627: <http://trac.webkit.org/changeset/96627>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.