150835 2015-11-03 03:14:03 -0800 [Streams API] Vended promise capabilities should not need @resolve/@reject fields 2015-11-03 10:30:56 -0800 1 1 1 Unclassified WebKit WebCore Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 150627 1 youennf youennf benjamin calvaris commit-queue darin oldest_to_newest 1138897 0 youennf 2015-11-03 03:14:03 -0800 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. 1138899 1 264683 youennf 2015-11-03 03:21:27 -0800 Created attachment 264683 Patch 1138920 2 264683 calvaris 2015-11-03 06:35:16 -0800 Comment on attachment 264683 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. 1138924 3 youennf 2015-11-03 06:57:48 -0800 (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. 1138936 4 calvaris 2015-11-03 08:45:08 -0800 (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. 1138963 5 264683 commit-queue 2015-11-03 10:30:52 -0800 Comment on attachment 264683 Patch Clearing flags on attachment: 264683 Committed r191956: <http://trac.webkit.org/changeset/191956> 1138964 6 commit-queue 2015-11-03 10:30:56 -0800 All reviewed patches have been landed. Closing bug. 264683 2015-11-03 03:21:27 -0800 2015-11-03 10:30:52 -0800 Patch bug-150835-20151103082029.patch text/plain 4511 youennf U3VidmVyc2lvbiBSZXZpc2lvbjogMTkxOTQ0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYjJkZWY5OGJhODZkY2E5 ZTZiODQwZWM2NmRhZTkxNTE5ZTFkMjVlYS4uNWIxMTcxODQ5ZjYxOTM5Zjc1ZmIwNThmN2FjNTE0 MjlmOWU2NWNmNyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDE1LTExLTAzICBZb3Vl bm4gRmFibGV0ICA8eW91ZW5uLmZhYmxldEBjcmYuY2Fub24uZnI+CisKKyAgICAgICAgW1N0cmVh bXMgQVBJXSBWZW5kZWQgcHJvbWlzZSBjYXBhYmlsaXRpZXMgc2hvdWxkIG5vdCBuZWVkIEByZXNv bHZlL0ByZWplY3QgZmllbGRzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD0xNTA4MzUKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBObyBjaGFuZ2UgaW4gYmVoYXZpb3IsIGNvdmVyZWQgYnkgZXhpc3RpbmcgdGVz dHMuCisKKyAgICAgICAgKiBNb2R1bGVzL3N0cmVhbXMvUmVhZGFibGVTdHJlYW1JbnRlcm5hbHMu anM6CisgICAgICAgIChwcml2YXRlSW5pdGlhbGl6ZVJlYWRhYmxlU3RyZWFtUmVhZGVyKTogUmVt b3ZlZCBAcmVzb2x2ZS9AcmVqZWN0IGZpZWxkcyBmcm9tIHJlc29sdmVkL3JlamVjdGVkIEBjbG9z ZWRQcm9taXNlQ2FwYWJpbGl0eS4KKyAgICAgICAgKiBNb2R1bGVzL3N0cmVhbXMvV3JpdGFibGVT dHJlYW0uanM6CisgICAgICAgIChpbml0aWFsaXplV3JpdGFibGVTdHJlYW0pOiBSZW1vdmVkIEBy ZXNvbHZlL0ByZWplY3QgZmllbGRzIGZyb20gcmVzb2x2ZWQgcmVhZHlQcm9taXNlQ2FwYWJpbGl0 eS4KKyAgICAgICAgKiBNb2R1bGVzL3N0cmVhbXMvV3JpdGFibGVTdHJlYW1JbnRlcm5hbHMuanM6 CisgICAgICAgIChzeW5jV3JpdGFibGVTdHJlYW1TdGF0ZVdpdGhRdWV1ZSk6IFVwZGF0ZWQgY29k ZSB0byBiZSBjbG9zZXIgdG8gc3BlYyBhbmQgcmVtb3ZpbmcgdGhlIG5lZWQgdG8gcmVzb2x2ZSBh biBhbHJlYWR5IHJlc29sdmVkIHByb21pc2UuCisKIDIwMTUtMTEtMDMgIFhhYmllciBSb2RyaWd1 ZXogQ2FsdmFyICA8Y2FsdmFyaXNAaWdhbGlhLmNvbT4KIAogICAgICAgICBbU3RyZWFtcyBBUEld IFJld29yayBwcm9taXNlcyB0byB1c2UgQG5ld1Byb21pc2VDYXBhYmlsaXR5CmRpZmYgLS1naXQg YS9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL3N0cmVhbXMvUmVhZGFibGVTdHJlYW1JbnRlcm5hbHMu anMgYi9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL3N0cmVhbXMvUmVhZGFibGVTdHJlYW1JbnRlcm5h bHMuanMKaW5kZXggYzc5ZDZhOWI3MGM5NjAyYzQ0NjkzYWRhNGQ1NjcwZWY0MjBlNTQxOS4uYzEz MDlhOWUxYTRmZWI0MDVkNzkzZGI2NzM5Y2YxZTQwZDQzZjE3MiAxMDA2NDQKLS0tIGEvU291cmNl L1dlYkNvcmUvTW9kdWxlcy9zdHJlYW1zL1JlYWRhYmxlU3RyZWFtSW50ZXJuYWxzLmpzCisrKyBi L1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvc3RyZWFtcy9SZWFkYWJsZVN0cmVhbUludGVybmFscy5q cwpAQCAtNDgsMTMgKzQ4LDEzIEBAIGZ1bmN0aW9uIHByaXZhdGVJbml0aWFsaXplUmVhZGFibGVT dHJlYW1SZWFkZXIoc3RyZWFtKQogICAgIGlmIChzdHJlYW0uQHN0YXRlID09PSBAc3RyZWFtQ2xv c2VkKSB7CiAgICAgICAgIHRoaXMuQG93bmVyUmVhZGFibGVTdHJlYW0gPSBudWxsOwogICAgICAg ICB0aGlzLkBzdG9yZWRFcnJvciA9IHVuZGVmaW5lZDsKLSAgICAgICAgdGhpcy5AY2xvc2VkUHJv bWlzZUNhcGFiaWxpdHkgPSB7IEBwcm9taXNlOiBQcm9taXNlLnJlc29sdmUodW5kZWZpbmVkKSwg QHJlc29sdmU6IGZ1bmN0aW9uKCkge30sIEByZWplY3Q6IGZ1bmN0aW9uKCkge30gfTsKKyAgICAg ICAgdGhpcy5AY2xvc2VkUHJvbWlzZUNhcGFiaWxpdHkgPSB7IEBwcm9taXNlOiBQcm9taXNlLnJl c29sdmUodW5kZWZpbmVkKSB9OwogICAgICAgICByZXR1cm4gdGhpczsKICAgICB9CiAgICAgLy8g RklYTUU6IEFTU0VSVChzdHJlYW0uQHN0YXRlID09PSBAc3RyZWFtRXJyb3JlZCk7CiAgICAgdGhp cy5Ab3duZXJSZWFkYWJsZVN0cmVhbSA9IG51bGw7CiAgICAgdGhpcy5Ac3RvcmVkRXJyb3IgPSBz dHJlYW0uQHN0b3JlZEVycm9yOwotICAgIHRoaXMuQGNsb3NlZFByb21pc2VDYXBhYmlsaXR5ID0g eyBAcHJvbWlzZTogUHJvbWlzZS5yZWplY3Qoc3RyZWFtLkBzdG9yZWRFcnJvciksIEByZXNvbHZl OiBmdW5jdGlvbigpIHt9LCBAcmVqZWN0OiBmdW5jdGlvbigpIHt9IH07CisgICAgdGhpcy5AY2xv c2VkUHJvbWlzZUNhcGFiaWxpdHkgPSB7IEBwcm9taXNlOiBQcm9taXNlLnJlamVjdChzdHJlYW0u QHN0b3JlZEVycm9yKSB9OwogCiAgICAgcmV0dXJuIHRoaXM7CiB9CmRpZmYgLS1naXQgYS9Tb3Vy Y2UvV2ViQ29yZS9Nb2R1bGVzL3N0cmVhbXMvV3JpdGFibGVTdHJlYW0uanMgYi9Tb3VyY2UvV2Vi Q29yZS9Nb2R1bGVzL3N0cmVhbXMvV3JpdGFibGVTdHJlYW0uanMKaW5kZXggNWQ1ZDRhODVlNWVk NTVjNjhiYzg0MDUxMTVkNzkwYzhmOThiNDgwZC4uOWFlNThkN2NhMmMxZmI5NzFkMzg5OWViMWE4 MjBkMTE0NTBlOGRmMCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvTW9kdWxlcy9zdHJlYW1z L1dyaXRhYmxlU3RyZWFtLmpzCisrKyBiL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvc3RyZWFtcy9X cml0YWJsZVN0cmVhbS5qcwpAQCAtNDMsNyArNDMsNyBAQCBmdW5jdGlvbiBpbml0aWFsaXplV3Jp dGFibGVTdHJlYW0odW5kZXJseWluZ1NpbmssIHN0cmF0ZWd5KQogCiAgICAgdGhpcy5AdW5kZXJs eWluZ1NpbmsgPSB1bmRlcmx5aW5nU2luazsKICAgICB0aGlzLkBjbG9zZWRQcm9taXNlQ2FwYWJp bGl0eSA9IEBuZXdQcm9taXNlQ2FwYWJpbGl0eShQcm9taXNlKTsKLSAgICB0aGlzLkByZWFkeVBy b21pc2VDYXBhYmlsaXR5ID0geyBAcHJvbWlzZTogUHJvbWlzZS5yZXNvbHZlKHVuZGVmaW5lZCks IEByZXNvbHZlOiBmdW5jdGlvbigpIHt9LCBAcmVqZWN0OiBmdW5jdGlvbigpIHt9IH07CisgICAg dGhpcy5AcmVhZHlQcm9taXNlQ2FwYWJpbGl0eSA9IHsgQHByb21pc2U6IFByb21pc2UucmVzb2x2 ZSh1bmRlZmluZWQpIH07CiAgICAgdGhpcy5AcXVldWUgPSBAbmV3UXVldWUoKTsKICAgICB0aGlz LkBzdGF0ZSA9IEBzdHJlYW1Xcml0YWJsZTsKICAgICB0aGlzLkBzdGFydGVkID0gZmFsc2U7CmRp ZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL3N0cmVhbXMvV3JpdGFibGVTdHJlYW1J bnRlcm5hbHMuanMgYi9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL3N0cmVhbXMvV3JpdGFibGVTdHJl YW1JbnRlcm5hbHMuanMKaW5kZXggMTdkZjA5ZTIyN2QyYmJiMzk3YTk2ODZhN2ZhY2U1NmNmNTc3 MjAyMy4uNWI4OGQ3NTU3MmFjZTNhN2UwMmY5N2ZhNDcxNWQ5NGNmNTU0YWY0NiAxMDA2NDQKLS0t IGEvU291cmNlL1dlYkNvcmUvTW9kdWxlcy9zdHJlYW1zL1dyaXRhYmxlU3RyZWFtSW50ZXJuYWxz LmpzCisrKyBiL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvc3RyZWFtcy9Xcml0YWJsZVN0cmVhbUlu dGVybmFscy5qcwpAQCAtMzksMjAgKzM5LDIwIEBAIGZ1bmN0aW9uIHN5bmNXcml0YWJsZVN0cmVh bVN0YXRlV2l0aFF1ZXVlKHN0cmVhbSkKICAgICAidXNlIHN0cmljdCI7CiAKICAgICBpZiAoc3Ry ZWFtLkBzdGF0ZSA9PT0gQHN0cmVhbUNsb3NpbmcpCi0gICAgICAgIHJldHVybiB1bmRlZmluZWQ7 CisgICAgICAgIHJldHVybjsKIAogICAgIC8vIEZJWE1FCiAgICAgLy8gYXNzZXJ0KHN0cmVhbS5A c3RhdGUgPT09IEBzdHJlYW1Xcml0YWJsZSB8fCBzdHJlYW0uQHN0YXRlID09PSBAc3RyZWFtV2Fp dGluZyk7CiAKLSAgICBpZiAoc3RyZWFtLkBxdWV1ZS5zaXplID4gc3RyZWFtLkBzdHJhdGVneS5o aWdoV2F0ZXJNYXJrKSB7CisgICAgY29uc3Qgc2hvdWxkQXBwbHlCYWNrcHJlc3N1cmUgPSBzdHJl YW0uQHF1ZXVlLnNpemUgPiBzdHJlYW0uQHN0cmF0ZWd5LmhpZ2hXYXRlck1hcms7CisgICAgaWYg KHNob3VsZEFwcGx5QmFja3ByZXNzdXJlICYmIHN0cmVhbS5Ac3RhdGUgPT09IEBzdHJlYW1Xcml0 YWJsZSkgewogICAgICAgICBzdHJlYW0uQHN0YXRlID0gQHN0cmVhbVdhaXRpbmc7CiAgICAgICAg IHN0cmVhbS5AcmVhZHlQcm9taXNlQ2FwYWJpbGl0eSA9IEBuZXdQcm9taXNlQ2FwYWJpbGl0eShQ cm9taXNlKTsKLSAgICB9IGVsc2UgeworICAgIH0KKyAgICBpZiAoIXNob3VsZEFwcGx5QmFja3By ZXNzdXJlICYmIHN0cmVhbS5Ac3RhdGUgPT09IEBzdHJlYW1XYWl0aW5nKSB7CiAgICAgICAgIHN0 cmVhbS5Ac3RhdGUgPSBAc3RyZWFtV3JpdGFibGU7CiAgICAgICAgIHN0cmVhbS5AcmVhZHlQcm9t aXNlQ2FwYWJpbGl0eS5AcmVzb2x2ZS5AY2FsbCh1bmRlZmluZWQsIHVuZGVmaW5lZCk7CiAgICAg fQotCi0gICAgcmV0dXJuIHVuZGVmaW5lZDsKIH0KIAogZnVuY3Rpb24gZXJyb3JXcml0YWJsZVN0 cmVhbShlKQo=