Bug 90877

Summary: WebSocket.send() should accept ArrayBufferView
Product: WebKit Reporter: Yuta Kitamura <yutak>
Component: WebCore Misc.Assignee: Yuta Kitamura <yutak>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, haraken, japhet, jochen, ojan, tkent, webkit.review.bot
Priority: P2 Keywords: WebExposed
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 85961    
Attachments:
Description Flags
Patch
none
Patch v2
none
Patch v3 none

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.