Bug 90877 - WebSocket.send() should accept ArrayBufferView
Summary: WebSocket.send() should accept ArrayBufferView
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: WebExposed
Depends on:
Blocks: 85961
  Show dependency treegraph
 
Reported: 2012-07-10 06:22 PDT by Yuta Kitamura
Modified: 2012-08-06 21:06 PDT (History)
8 users (show)

See Also:


Attachments
Patch (24.37 KB, patch)
2012-07-11 01:06 PDT, Yuta Kitamura
no flags Details | Formatted Diff | Diff
Patch v2 (36.02 KB, patch)
2012-07-11 22:39 PDT, Yuta Kitamura
no flags Details | Formatted Diff | Diff
Patch v3 (24.22 KB, patch)
2012-08-06 19:16 PDT, Yuta Kitamura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yuta Kitamura 2012-07-10 06:22:15 PDT
This is a child bug of bug 85961.
Comment 1 Yuta Kitamura 2012-07-11 01:06:07 PDT
Created attachment 151637 [details]
Patch
Comment 2 Kinuko Yasuda 2012-07-11 07:58:48 PDT
Comment on attachment 151637 [details]
Patch

(Drive-by comment)

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

> Source/WebCore/Modules/websockets/WebSocket.cpp:294
> +    return m_channel->send(*binaryData, 0, binaryData->byteLength()) == ThreadableWebSocketChannel::SendSuccess;

Might be nicer to show deprecation warning message (and optionally collect metrics) if ArrayBuffer is given as well as we do in Blob constructor and XHR.send?
Comment 3 Yuta Kitamura 2012-07-11 22:39:05 PDT
Created attachment 151861 [details]
Patch v2
Comment 4 Yuta Kitamura 2012-07-11 22:41:06 PDT
Comment on attachment 151637 [details]
Patch

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

>> Source/WebCore/Modules/websockets/WebSocket.cpp:294
>> +    return m_channel->send(*binaryData, 0, binaryData->byteLength()) == ThreadableWebSocketChannel::SendSuccess;
> 
> Might be nicer to show deprecation warning message (and optionally collect metrics) if ArrayBuffer is given as well as we do in Blob constructor and XHR.send?

Thanks, I added these in patch v2.
Comment 5 Kent Tamura 2012-08-02 21:27:31 PDT
Comment on attachment 151861 [details]
Patch v2

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

> Source/WebCore/ChangeLog:16
> +        Also, histogram metrics collection is added in WebSocket.send() to keep track of
> +        the usage of WebSocket.send() family.

This can be a separated patch.
Comment 6 Yuta Kitamura 2012-08-06 18:17:42 PDT
Comment on attachment 151861 [details]
Patch v2

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

http://dev.w3.org/html5/websockets/

WebSocket API draft was updated and now it includes *both* send(ArrayBuffer) and
send(ArrayBufferView). So, deprecation warning for send(ArrayBuffer) does not
make sense, thus I'm going to update the patch to remove it.

>> Source/WebCore/ChangeLog:16
>> +        the usage of WebSocket.send() family.
> 
> This can be a separated patch.

Will be removed.
Comment 7 Yuta Kitamura 2012-08-06 19:16:40 PDT
Created attachment 156830 [details]
Patch v3
Comment 8 Kent Tamura 2012-08-06 20:12:58 PDT
Comment on attachment 156830 [details]
Patch v3

Looks good.
Comment 9 WebKit Review Bot 2012-08-06 21:06:33 PDT
Comment on attachment 156830 [details]
Patch v3

Clearing flags on attachment: 156830

Committed r124846: <http://trac.webkit.org/changeset/124846>
Comment 10 WebKit Review Bot 2012-08-06 21:06:38 PDT
All reviewed patches have been landed.  Closing bug.