Bug 68345 – pop-state-event-constructor.html crashes and fails

• Component: DOM
• Status: RESOLVED FIXED

[Kentaro Hara]

I added pop-state-event-constructor.html in this bug (https://bugs.webkit.org/show_bug.cgi?id=67977), but some tests are crashing or failing. 

- The following test cases are failing, as you can see in the attached file. The IDL type of PopStateEvent.state should be 'any' (http://www.whatwg.org/specs/web-apps/current-work/#popstateevent), and the IDL spec of 'any' (http://www.w3.org/TR/WebIDL/#es-any) says that the following test cases should pass.

    FAIL new PopStateEvent('eventType', { state: '' }).state should be undefined (of type undefined). Was  (of type string).
    FAIL new PopStateEvent('eventType', { state: object1 }).state should be [object Object]. Was [object Object].
    FAIL new PopStateEvent('eventType', { state: {valueOf: function () { return object2; } } }).state should be [object Object]. Was [object Object].
    FAIL new PopStateEvent('eventType', { bubbles: true, cancelable: true, state: object3 }).state should be [object Object]. Was [object Object].

- When we pass a DOM object (i.e. an unserializable object) as follows, then DRT crashes. This crash happens only in DRT.

    shouldBe("new PopStateEvent('eventType', { state: document }).state", "document");

The reason for these failures and crash is that PopStateEvent.state is implemented just as SeriazliedScriptValue, and thus it cannot handle ScriptValue. I am preparing a patch for fixing this.

[Kentaro Hara]

Created attachment 107824 [details]
Failing test cases

[Kentaro Hara]

Created attachment 108632 [details]
Patch

[Kentaro Hara]

Created attachment 108633 [details]
Just updated ChangeLog

[Gyuyoung Kim]

Comment on attachment 108633 [details]
Just updated ChangeLog

Attachment 108633 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9843772

[Kentaro Hara]

Created attachment 108645 [details]
WIP patch to see if the patch passes efl build

[Kentaro Hara]

Created attachment 108651 [details]
patch for review

[Adam Barth]

Comment on attachment 108651 [details]
patch for review

View in context: https://bugs.webkit.org/attachment.cgi?id=108651&action=review

> Source/WebCore/ChangeLog:17
> +        popped from HistoryItem, but we cannot deserialize the SeriazliedScriptValue into

SeriazliedScriptValue => SerializedScriptValue

> Source/WebCore/ChangeLog:20
> +        PopStateEvent still needs to provide an API to construct it with SeriazliedScriptValue.

SeriazliedScriptValue

> Source/WebCore/bindings/v8/custom/V8PopStateEventCustom.cpp:46
> +    SerializedScriptValue* serializedState = event->serializedState();
> +    return serializedState ? serializedState->deserialize() : event->state().v8Value();

Will this return undefined when the JSC version returns null?

> Source/WebCore/dom/PopStateEvent.cpp:61
> +    , m_state()

We usually omit this declaration when we're just calling the default constructor.

[Kentaro Hara]

Created attachment 108763 [details]
patch for commit

[Kentaro Hara]

Created attachment 108785 [details]
patch for commit

[Kentaro Hara]

Comment on attachment 108651 [details]
patch for review

View in context: https://bugs.webkit.org/attachment.cgi?id=108651&action=review

>> Source/WebCore/ChangeLog:17
>> +        popped from HistoryItem, but we cannot deserialize the SeriazliedScriptValue into
> 
> SeriazliedScriptValue => SerializedScriptValue

Thanks! Fixed.

>> Source/WebCore/ChangeLog:20
>> +        PopStateEvent still needs to provide an API to construct it with SeriazliedScriptValue.
> 
> SeriazliedScriptValue

Fixed.

>> Source/WebCore/bindings/v8/custom/V8PopStateEventCustom.cpp:46
>> +    return serializedState ? serializedState->deserialize() : event->state().v8Value();
> 
> Will this return undefined when the JSC version returns null?

Sorry, fixed it.

>> Source/WebCore/dom/PopStateEvent.cpp:61
>> +    , m_state()
> 
> We usually omit this declaration when we're just calling the default constructor.

Omitted.

[WebKit Review Bot]

Comment on attachment 108785 [details]
patch for commit

Clearing flags on attachment: 108785

Committed r96073: <http://trac.webkit.org/changeset/96073>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.