[webkit-reviews] review granted: [Bug 66877] [Chromium] Add ability to do static SerializedScriptValue deserialization : [Attachment 105493] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 29 10:45:14 PDT 2011


Darin Fisher (:fishd, Google) <fishd at chromium.org> has granted Greg Billock
<gbillock at google.com>'s request for review:
Bug 66877: [Chromium] Add ability to do static SerializedScriptValue
deserialization
https://bugs.webkit.org/show_bug.cgi?id=66877

Attachment 105493: Patch
https://bugs.webkit.org/attachment.cgi?id=105493&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
Hmm, I'm kinda torn.  There are arguments in favor of the following:

WebSerializedScriptValue(v8::Handle<v8::Value>);
static WebSerializedScriptValue create(v8::Handle<v8::Value>);
static WebSerializedScriptValue serialize(v8::Handle<v8::Value>);

The first is kind of natural given that that's the point of constructors.
The second is consistent with createInvalid.  The third is consistent with
the deserialize method.

The odd ball is fromString, which could be a constructor too, or it could
be named createFromString.

Note: in the WebKit API create* is usually reserved for methods that return
heap allocated objects that the caller is then responsible for deleting.
This argues that createInvalid should be renamed.

I wonder... perhaps the default constructor for WebSerializedScriptValue
should return the same thing as createInvalid.	It seems that they are not
implemented the same, which suggests that there is yet another state being
represented.  I wonder if we need that.

Sigh, this is not a well engineered interface :(


More information about the webkit-reviews mailing list