[webkit-reviews] review denied: [Bug 101348] CustomEvent: Allow taking in a serialized value during initialization. : [Attachment 172582] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied sadrul at chromium.org's request for
review:
Bug 101348: CustomEvent: Allow taking in a serialized value during
initialization.
https://bugs.webkit.org/show_bug.cgi?id=101348

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=172582&action=review


> Source/WebCore/dom/CustomEvent.cpp:83
> +const ScriptValue& CustomEvent::detail()
> +{
> +    if (m_serializedScriptValue.get())
> +	   m_detail = ScriptValue::deserialize(ScriptState::current(),
m_serializedScriptValue.get());
> +    return m_detail;
> +}

This patch is almost right.  The one tweak is that we need to do this work in
in the bindings layer and cache the result on the JavaScript wrapper (i.e.,
JSCustomEvent or V8CustomEvent).  The way you've written this, the m_detail is
shared by all wrappers.  We want different instances of the details object for
each wrapper (i.e., for each isolated world that might receive the event).

Also, rather than using ScriptState::current(), we'll want to use the
CreationContext of the JavaScript wrapper as the context for the
deserialization.