[webkit-reviews] review denied: [Bug 76035] Add state attribute to history's dom interface. : [Attachment 121987] proposed patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Pablo Flouret <pablof at motorola.com>'s
request for review:
Bug 76035: Add state attribute to history's dom interface.
https://bugs.webkit.org/show_bug.cgi?id=76035

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

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


Thanks for working on this bug.  One issue below:

> Source/WebCore/bindings/js/JSHistoryCustom.cpp:170
> +    History* history = static_cast<History*>(impl());
> +    return history->stateObject()->deserialize(exec, globalObject(), 0);

I think this isn't quite right.  Every time we call this getter, we're going to
deserialize the state object again.  That means

history.state !== history.state

which doesn't seem to match the spec.  If that's what other browsers do, then
we can change the spec, but it's more likely that we should cache the result of
deserializing the SerializedScriptValue and aways return the same value.

Would you be willing to test Firefox and/or IE to see whether they create a new
deserialization every time you call the getter?