Bug 67180 - WebSocket: Receive binary message as ArrayBuffer
Summary: WebSocket: Receive binary message as ArrayBuffer
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Yuta Kitamura
URL:
Keywords:
Depends on:
Blocks: 65249
  Show dependency tree / graph
 
Reported: 2011-08-29 23:06 PDT by Yuta Kitamura
Modified: 2011-08-31 01:32 PDT (History)
4 users (show)

See Also:

Attachments
Patch (17.77 KB, patch)
2011-08-29 23:29 PDT, Yuta Kitamura 		no flags Details | Formatted Diff | Diff
Patch v2 (20.74 KB, patch)
2011-08-30 05:16 PDT, Yuta Kitamura 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.