Re: [whatwg/streams] Add ReadableStreamBYOBReader.readFully() (#1145)

@MattiasBuelens commented on this pull request.



>  
   let totalBytesToCopyRemaining = maxBytesToCopy;
   let ready = false;
-  if (maxAlignedBytes > currentAlignedBytes) {
-    totalBytesToCopyRemaining = maxAlignedBytes - pullIntoDescriptor.bytesFilled;
-    ready = true;
+  if (pullIntoDescriptor.readFully) {
+    // A descriptor for a readFully() request that is not yet completely filled will stay at the head of the queue,
+    // so the underlying source can keep filling it.
+    if (maxBytesFilled >= pullIntoDescriptor.byteLength) {
+      totalBytesToCopyRemaining = pullIntoDescriptor.byteLength - pullIntoDescriptor.bytesFilled;
+      ready = true;
+    }
+  } else {
+    const maxAlignedBytes = maxBytesFilled - maxBytesFilled % elementSize;
+    if (maxAlignedBytes > currentAlignedBytes) {

Can't this be simplified to `maxAlignedBytes > 0`? 🤔 If the pull-into descriptor was still in the queue, then it must have `bytesFilled < elementSize`. So `currentAlignedBytes = bytesFilled - bytesFilled % elementSize` must also be `0`. (I feel like I never understood why `currentAlignedBytes` was necessary.)

All tests pass fine when I add an `assert(currentAlignedBytes === 0)` here, so I think this is valid? Unless I'm missing an edge case (that isn't covered by any WPT test)?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/streams/pull/1145#pullrequestreview-713309897