[webkit-reviews] review denied: [Bug 66127] Construct SerializedScriptValue from Blob : [Attachment 103747] Patch

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Yuta Kitamura
<yutak at chromium.org>'s request for review:
Bug 66127: Construct SerializedScriptValue from Blob
https://bugs.webkit.org/show_bug.cgi?id=66127

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

------- Additional Comments from David Levin <levin at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=103747&action=review


SerializedScriptValue is used to support structured clones, so this change
brings up a lot of questions at a high level:
1. Why is this change being done without adding blob to the general algorithm
for structured clones?
2. Has the change to structured clones been talked about with the appropriate
mailing lists (and what was the response)?
3. How does this fit with serializing to database and then pulling it back out
in a later session?  (For cross process, I get that the url should be global to
the browser.)

It would be useful to know if this has gone through a some public mailing list
discussion.

Thanks!

> Source/WebCore/bindings/js/SerializedScriptValue.cpp:278
> +    static bool serialize(PassRefPtr<Blob> blob, Vector<uint8_t>& out)

Why does this take a PassRefPtr when it doesn't take ownership? (It doesn't put
the passed in ref counted object into something like a RefPtr.)

It looks like it should take a Blob*.

> Source/WebCore/bindings/js/SerializedScriptValue.cpp:1387
> +PassRefPtr<SerializedScriptValue>
SerializedScriptValue::create(PassRefPtr<Blob> blob)

Ditto re PassRefPtr.

> Source/WebCore/bindings/v8/SerializedScriptValue.cpp:1883
> +PassRefPtr<SerializedScriptValue>
SerializedScriptValue::create(PassRefPtr<Blob> data)

Ditto re PassRefPtr.