The last part of bug 65249, sibling of bug 67465.
Created attachment 106119 [details] Patch
Comment on attachment 106119 [details] Patch 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.
Created attachment 106248 [details] Patch for landing
Comment on attachment 106119 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=106119&action=review Thank you for your comments! >> LayoutTests/http/tests/websocket/tests/hybi/workers/resources/send-blob.js:-5 >> - > > ChangeLog should mention the reason of this removal. Added to ChangeLog. >> 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? Renamed. >> 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. Added a comment. >> Source/WebCore/websockets/WorkerThreadableWebSocketChannel.cpp:436 >> + memcpy(data->data(), binaryData.data(), binaryData.byteLength()); > > resize() should be reserveCapacity(). > Your code initializes the vector buffer twice. I don't think using reserveCapaity() is right, for two reasons: - Initializing the vector elements will be turned into no-op, if the element type is primitive one. (VectorInitializer is doing some template magic) - reserveCapacity() does not change the size of the vector, so we must add elements anyway. Appending each element should not be better than memcpy(). However, I realized resize() is not necessary becase we can use Vector constructor instead. I update the patch to do this.
Comment on attachment 106248 [details] Patch for landing Clearing flags on attachment: 106248 Committed r94482: <http://trac.webkit.org/changeset/94482>
All reviewed patches have been landed. Closing bug.