[webkit-reviews] review granted: [Bug 67477] WebSocket: Send ArrayBuffer as WebSocket binary message : [Attachment 106119] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has granted Yuta Kitamura
<yutak at chromium.org>'s request for review:
Bug 67477: WebSocket: Send ArrayBuffer as WebSocket binary message
https://bugs.webkit.org/show_bug.cgi?id=67477

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

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=106119&action=review


> LayoutTests/http/tests/websocket/tests/hybi/workers/resources/send-blob.js:-5

> -function startsWith(target, prefix)
> -{
> -    return target.indexOf(prefix) === 0;
> -}
> -

ChangeLog should mention the reason of this removal.

> Source/WebCore/ChangeLog:22
> +	   Rename "sent" to "sendResult" to clarify the meaning. Messages from
the script may not be sent

nit:
Hmm, the sent -> sendResult renaming looks not to improve the readability.
sendRequestResult?

> Source/WebCore/websockets/WorkerThreadableWebSocketChannel.cpp:433
> +    OwnPtr<Vector<char> > data = adoptPtr(new Vector<char>);

We had better add a comment why we need to copy the ArrayBuffer to the Vector.

> Source/WebCore/websockets/WorkerThreadableWebSocketChannel.cpp:436
> +    data->resize(binaryData.byteLength());
> +    if (binaryData.byteLength())
> +	   memcpy(data->data(), binaryData.data(), binaryData.byteLength());

resize() should be reserveCapacity().
Your code initializes the vector buffer twice.