Bug 69156 - Implicitly add toString and valueOf to prototype when convertToType callback is provided
Summary: Implicitly add toString and valueOf to prototype when convertToType callback ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Hahnenberg
URL:
Keywords:
Depends on:
Blocks: 67690 69677 69679
  Show dependency treegraph
 
Reported: 2011-09-30 10:50 PDT by Mark Hahnenberg
Modified: 2011-10-07 16:51 PDT (History)
3 users (show)

See Also:


Attachments
Patch (6.24 KB, patch)
2011-10-03 17:32 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Test modification (6.25 KB, patch)
2011-10-03 18:22 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hahnenberg 2011-09-30 10:50:37 PDT
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.
Comment 1 Gavin Barraclough 2011-09-30 15:07:42 PDT
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.
Comment 2 Geoffrey Garen 2011-10-02 15:08:15 PDT
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
Comment 3 Mark Hahnenberg 2011-10-03 17:32:57 PDT
Created attachment 109561 [details]
Patch
Comment 4 Mark Hahnenberg 2011-10-03 18:22:45 PDT
Created attachment 109566 [details]
Test modification
Comment 5 Geoffrey Garen 2011-10-03 18:24:50 PDT
Comment on attachment 109566 [details]
Test modification

r=me
Comment 6 WebKit Review Bot 2011-10-04 12:01:13 PDT
Comment on attachment 109566 [details]
Test modification

Clearing flags on attachment: 109566

Committed r96627: <http://trac.webkit.org/changeset/96627>
Comment 7 WebKit Review Bot 2011-10-04 12:01:18 PDT
All reviewed patches have been landed.  Closing bug.