[webkit-reviews] review denied: [Bug 33592] Use SerializedScriptValue for passing values between inspector front-end and the inspected page : [Attachment 48221] patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Yury Semikhatsky
<yurys at chromium.org>'s request for review:
Bug 33592: Use SerializedScriptValue for passing values between inspector
front-end and the inspected page
https://bugs.webkit.org/show_bug.cgi?id=33592

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

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
I like this a lot!

> +PassRefPtr<SerializedScriptValue> serialize(ScriptState* scriptState, const
ScriptValue& value)
> +{
> +    return SerializedScriptValue::create(scriptState, value.jsValue());
> +}

Should this be defined in ScriptValue instead?

> +
> +ScriptValue deserialize(ScriptState* scriptState, SerializedScriptValue*
value)
> +{
> +    return ScriptValue(value->deserialize(scriptState,
scriptState->lexicalGlobalObject()));
> +}
> +

ditto.

> -    var argsArray = InjectedScript.JSON.parse(args);
> +    var argsArray = JSON.parse(args);

Overriding JSON would bring harm us here. I realize that parse is much more
standard than stringify, but still. I'd suggest that you either inline copy of
parse method here or simply use eval. This method is being called from the
trusted source, it has only valid symbols, etc. r- for this.