Implement respond() in readable stream state
Created attachment 304655 [details] Patch
Comment on attachment 304655 [details] Patch Attachment 304655 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3339459 Number of test failures exceeded the failure limit.
Created attachment 304659 [details] Archive of layout-test-results from ews117 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 304769 [details] Patch
Comment on attachment 304769 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=304769&action=review > Source/WebCore/Modules/streams/ReadableByteStreamInternals.js:388 > + return srcBuffer.slice(srcByteOffset, srcByteOffset + srcLength); This implementation is not shielded. Also it should probably be implemented in JSC directly. There is already @structuredCloneArrayBuffer. Maybe we should group these together. > Source/WebCore/Modules/streams/ReadableByteStreamInternals.js:417 > + @readableByteStreamControllerProcessPullIntoDescriptorsUsingQueue(controller); The function names are starting to be very long. Although it is good to match spec names, this hurts readability. Maybe we could start some shortening like: readableByteStreamControllerEnqueueChunkToQueue -> readableByteStreamControllerEnqueueChunk readableByteStreamControllerCommitPullIntoDescriptor -> readableByteStreamControllerCommitDescriptor readableByteStreamControllerProcessPullIntoDescriptorsUsingQueue -> readableByteStreamControllerProcessPullDescriptors ... > Source/WebCore/Modules/streams/ReadableByteStreamInternals.js:450 > + @readableByteStreamControllerShiftPendingPullInto(controller); The naming seems not totally consistent here: readableByteStreamControllerShiftPendingPullInto readableByteStreamControllerCommitPullIntoDescriptor I would rename to something like readableByteStreamControllerShiftPendingDescriptor, readableByteStreamControllerCommitDescriptor > Source/WebCore/Modules/streams/ReadableByteStreamInternals.js:503 > +{ Group this function with the other, in a new or another file ideally.
Created attachment 305095 [details] Patch
Comment on attachment 305095 [details] Patch Clearing flags on attachment: 305095 Committed r214265: <http://trac.webkit.org/changeset/214265>
All reviewed patches have been landed. Closing bug.
(In reply to youenn fablet from comment #5) > Comment on attachment 304769 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=304769&action=review > > > Source/WebCore/Modules/streams/ReadableByteStreamInternals.js:388 > > + return srcBuffer.slice(srcByteOffset, srcByteOffset + srcLength); > > This implementation is not shielded. > Also it should probably be implemented in JSC directly. > There is already @structuredCloneArrayBuffer. Maybe we should group these > together. Ok I will see how to group them together. > > Source/WebCore/Modules/streams/ReadableByteStreamInternals.js:417 > > + @readableByteStreamControllerProcessPullIntoDescriptorsUsingQueue(controller); > > The function names are starting to be very long. > Although it is good to match spec names, this hurts readability. > Maybe we could start some shortening like: > readableByteStreamControllerEnqueueChunkToQueue -> > readableByteStreamControllerEnqueueChunk > readableByteStreamControllerCommitPullIntoDescriptor -> > readableByteStreamControllerCommitDescriptor > readableByteStreamControllerProcessPullIntoDescriptorsUsingQueue -> > readableByteStreamControllerProcessPullDescriptors > ... > > > Source/WebCore/Modules/streams/ReadableByteStreamInternals.js:450 > > + @readableByteStreamControllerShiftPendingPullInto(controller); > > The naming seems not totally consistent here: > readableByteStreamControllerShiftPendingPullInto > readableByteStreamControllerCommitPullIntoDescriptor > > I would rename to something like > readableByteStreamControllerShiftPendingDescriptor, > readableByteStreamControllerCommitDescriptor > I've updated these names as well as a few other (and I've added comments to indicate the name of corresponding operation in the spec). > > Source/WebCore/Modules/streams/ReadableByteStreamInternals.js:503 > > +{ > > Group this function with the other, in a new or another file ideally. As this function is called only once by the spec and simply copies data from one buffer to another (no need to detach/transfer/...), I've inlined it.