Following on bug 150627, vended promise capabilities in the streams API should not need @resolve/@reject fields. I also suspect we could do clean-up as well in the promise implementation, as a GC-friendly step.
Created attachment 264683 [details] Patch
Comment on attachment 264683 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=264683&action=review > Source/WebCore/Modules/streams/WritableStreamInternals.js:52 > + const shouldApplyBackpressure = stream.@queue.size > stream.@strategy.highWaterMark; > + if (shouldApplyBackpressure && stream.@state === @streamWritable) { > stream.@state = @streamWaiting; > stream.@readyPromiseCapability = @newPromiseCapability(Promise); > - } else { > + } > + if (!shouldApplyBackpressure && stream.@state === @streamWaiting) { I personally dislike this option. I think it was better to use the functions to create the vended promises because we are closer to the spec and it is less error prone in case we have to change anything. Instead of this patch I would have functions to create vended promise capabilities as I did in earlier versions of patch for bug 150627.
(In reply to comment #2) > Comment on attachment 264683 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=264683&action=review > > > Source/WebCore/Modules/streams/WritableStreamInternals.js:52 > > + const shouldApplyBackpressure = stream.@queue.size > stream.@strategy.highWaterMark; > > + if (shouldApplyBackpressure && stream.@state === @streamWritable) { > > stream.@state = @streamWaiting; > > stream.@readyPromiseCapability = @newPromiseCapability(Promise); > > - } else { > > + } > > + if (!shouldApplyBackpressure && stream.@state === @streamWaiting) { > > I personally dislike this option. I think it was better to use the functions > to create the vended promises because we are closer to the spec and it is > less error prone in case we have to change anything. Instead of this patch I > would have functions to create vended promise capabilities as I did in > earlier versions of patch for bug 150627. These specific changes make the implementation closer to the spec and removes the fact that we were resolving an already resolved promise. These changes should be done no matter whether we remove @resolve/@reject for vended promises.
(In reply to comment #3) > These specific changes make the implementation closer to the spec and > removes the fact that we were resolving an already resolved promise. > > These changes should be done no matter whether we remove @resolve/@reject > for vended promises. You're right. I missed that from the spec that it was causing that the promises were vended more than once.
Comment on attachment 264683 [details] Patch Clearing flags on attachment: 264683 Committed r191956: <http://trac.webkit.org/changeset/191956>
All reviewed patches have been landed. Closing bug.