Bug 67180

Summary: WebSocket: Receive binary message as ArrayBuffer
Product: WebKit Reporter: Yuta Kitamura <yutak>
Component: WebCore Misc.Assignee: Yuta Kitamura <yutak>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 65249    
Attachments:
Description Flags
Patch
none
Patch v2 none

Description Yuta Kitamura 2011-08-29 23:06:25 PDT 
Part of bug 65249. See also: bug 67115 (Receive binary message as Blob)
Comment 1 Yuta Kitamura 2011-08-29 23:29:11 PDT 
Created attachment 105585 [details]
Patch
Comment 2 Kent Tamura 2011-08-30 01:37:19 PDT 
Comment on attachment 105585 [details]
Patch

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

> LayoutTests/http/tests/websocket/tests/hybi/receive-arraybuffer.html:32
> +// Create an ArrayBuffer containing all distinct bytes ("\x00" to "\xFF").
> +function createAllBytesValue()

nit: The comment is not so helpful because the function code is simple, and we can rename it to "createArrayBufferContainingAllDistinctBytes"

> LayoutTests/http/tests/websocket/tests/hybi/workers/resources/receive-arraybuffer.js:16
> +// Create an ArrayBuffer containing all distinct bytes ("\x00" to "\xFF").
> +function createAllBytesValue()

ditto.

> Source/WebCore/websockets/WebSocket.cpp:419
> +        RefPtr<ArrayBuffer> arrayBuffer = ArrayBuffer::create(binaryData->data(), binaryData->size());
> +        dispatchEvent(MessageEvent::create(arrayBuffer.release()));

The variable 'arrayBuffer' is not needed.
Comment 3 Yuta Kitamura 2011-08-30 05:16:32 PDT 
Created attachment 105618 [details]
Patch v2
Comment 4 Yuta Kitamura 2011-08-30 05:18:12 PDT 
Comment on attachment 105585 [details]
Patch

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

>> LayoutTests/http/tests/websocket/tests/hybi/receive-arraybuffer.html:32
>> +function createAllBytesValue()
> 
> nit: The comment is not so helpful because the function code is simple, and we can rename it to "createArrayBufferContainingAllDistinctBytes"

Fixed. Updated function names in receive-blob.html too.

>> LayoutTests/http/tests/websocket/tests/hybi/workers/resources/receive-arraybuffer.js:16
>> +function createAllBytesValue()
> 
> ditto.

Fixed.

>> Source/WebCore/websockets/WebSocket.cpp:419
>> +        dispatchEvent(MessageEvent::create(arrayBuffer.release()));
> 
> The variable 'arrayBuffer' is not needed.

Fixed.
Comment 5 Kent Tamura 2011-08-31 00:22:11 PDT 
Comment on attachment 105618 [details]
Patch v2

Looks ok.
Comment 6 WebKit Review Bot 2011-08-31 01:32:45 PDT 
Comment on attachment 105618 [details]
Patch v2

Clearing flags on attachment: 105618

Committed r94161: <http://trac.webkit.org/changeset/94161>
Comment 7 WebKit Review Bot 2011-08-31 01:32:50 PDT 
All reviewed patches have been landed.  Closing bug.