[webkit-reviews] review granted: [Bug 224865] DOMException should be Serializable : [Attachment 426676] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Chris Lord <clord at igalia.com>'s
request for review:
Bug 224865: DOMException should be Serializable
https://bugs.webkit.org/show_bug.cgi?id=224865

Attachment 426676: Patch

https://bugs.webkit.org/attachment.cgi?id=426676&action=review




--- Comment #5 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 426676
  --> https://bugs.webkit.org/attachment.cgi?id=426676
Patch

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

r=me with comments.

> Source/WebCore/bindings/js/SerializedScriptValue.cpp:1389
> +		   write(DOMExceptionTag);

I think it would be good to move this logic to a separate dumpDOMException()
function, to follow the pattern used by other types.

> Source/WebCore/bindings/js/SerializedScriptValue.cpp:3595
> +	       CachedStringRef name;

Ditto. It would be good to move this logic to a separate readDOMException()
function.

> LayoutTests/storage/indexeddb/resources/structured-clone.js:-631
> -    evalAndExpectException("store.put(new DOMException, 'key')",
"DOMException.DATA_CLONE_ERR");

Instead of simply drop it the check, can we switch it into a check that is
supposed to succeed? It seems this test checks checks types that succeed and
fail.