[Webkit-unassigned] [Bug 76035] Add state attribute to history's dom interface.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=76035





--- Comment #21 from Adam Barth <abarth at webkit.org>  2012-01-13 16:34:32 PST ---
(From update of attachment 122518)
View in context: https://bugs.webkit.org/attachment.cgi?id=122518&action=review

> Source/WebCore/page/History.cpp:112
> +ScriptValue History::state(ScriptState* exec) const

exec -> scriptState

> Source/WebCore/page/History.cpp:114
> +    if (m_frame) {

Prefer early exist.  You can just say:

if (!m_frame)
    return ScriptValue();

etc

> Source/WebCore/page/History.cpp:116
> +            ScriptValue& state = historyItem->deserializedStateObject();

This is kind of subtle given that you're assign to a non-const reference.

Maybe you should have historyItem do this work?

> Source/WebCore/page/History.cpp:123
> +#if USE(V8)
> +                state = ScriptValue(historyItem->stateObject()->deserialize());
> +#else
> +                ASSERT(exec);
> +                state = ScriptValue::deserialize(exec, historyItem->stateObject());
> +#endif

I would just add a ScriptValue::deserialize to the V8 version of SerializedScriptValue that accepts a ScriptState so you don't need this ifdef.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.