147487 2015-07-31 05:16:53 -0700 [Streams API] ReadableStreamReader closed promise should use CachedAttribute 2015-08-10 03:01:53 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 youennf youennf commit-queue darin ggaren oldest_to_newest 1113704 0 youennf 2015-07-31 05:16:53 -0700 This would allow removing the current specific promise attribute binding. This would allow using a JSC::WriteBarrier in lieu of JSC::Strong as well. 1113705 1 257918 youennf 2015-07-31 05:20:18 -0700 Created attachment 257918 Patch 1113706 2 youennf 2015-07-31 05:21:15 -0700 (In reply to comment #1) > Created attachment 257918 [details] > Patch This patch does not remove the specific binding generator code related to promise attribute. This would be done as a follow-up patch. 1116065 3 257918 darin 2015-08-08 17:57:40 -0700 Comment on attachment 257918 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257918&action=review Looks OK. I don’t really know what CachedAttribute does but this looks sensible. > Source/WebCore/bindings/js/JSReadableStreamReaderCustom.cpp:51 > + const_cast<JSReadableStreamReader*>(this)->m_closed.set(exec->vm(), this, closedPromise->promise()); Is this preferred over marking m_closed mutable? 1116190 4 youennf 2015-08-10 02:13:36 -0700 (In reply to comment #3) > Comment on attachment 257918 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=257918&action=review > > Looks OK. I don’t really know what CachedAttribute does but this looks > sensible. CachedAttribute generates some code like: - a class member field in the JS wrapper class (as would be done in JSC) - marking the member field when the JS wrapper object is being visited. In our case, this removes one JSC::Strong. This does not solve entirely the issue since the promise is strongified when being wrapped as a DeferredWrapper. > > Source/WebCore/bindings/js/JSReadableStreamReaderCustom.cpp:51 > > + const_cast<JSReadableStreamReader*>(this)->m_closed.set(exec->vm(), this, closedPromise->promise()); > > Is this preferred over marking m_closed mutable? Right, marking cached attributes as mutable would be an improvement. 1116195 5 257918 commit-queue 2015-08-10 03:01:49 -0700 Comment on attachment 257918 Patch Clearing flags on attachment: 257918 Committed r188209: <http://trac.webkit.org/changeset/188209> 1116196 6 commit-queue 2015-08-10 03:01:53 -0700 All reviewed patches have been landed. Closing bug. 257918 2015-07-31 05:20:18 -0700 2015-08-10 03:01:49 -0700 Patch bug-147487-20150731142015.patch text/plain 2817 youennf U3VidmVyc2lvbiBSZXZpc2lvbjogMTg3NDM0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMWNlMDcwOTBkYzFhMzg0 NGFhYzk4MmQyZmVkYTMxODVjMjFkYTI2OS4uYzZjMDNmNmIyZDE5MzhkMGUwOWI3NjcwNjQyMzM5 NGU5NjNhZDcwZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDE1LTA3LTMxICBZb3Vl bm4gRmFibGV0ICA8eW91ZW5uLmZhYmxldEBjcmYuY2Fub24uZnI+CisKKyAgICAgICAgW1N0cmVh bXMgQVBJXSBSZWFkYWJsZVN0cmVhbVJlYWRlciBjbG9zZWQgcHJvbWlzZSBzaG91bGQgdXNlIENh Y2hlZEF0dHJpYnV0ZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MTQ3NDg3CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAg ICAgICAgQ292ZXJlZCBieSBleGlzdGluZyB0ZXN0cy4KKworICAgICAgICAqIE1vZHVsZXMvc3Ry ZWFtcy9SZWFkYWJsZVN0cmVhbVJlYWRlci5pZGw6IE1hZGUgY2xvc2VkIGEgQ2FjaGVkQXR0cmli dXRlLgorICAgICAgICAqIGJpbmRpbmdzL2pzL0pTUmVhZGFibGVTdHJlYW1SZWFkZXJDdXN0b20u Y3BwOgorICAgICAgICAoV2ViQ29yZTo6SlNSZWFkYWJsZVN0cmVhbVJlYWRlcjo6Y2xvc2VkKTog VXBkYXRlZCBhY2NvcmRpbmcgQ2FjaGVkQXR0cmlidXRlIHNwZWNpZmljIGZpZWxkLgorCiAyMDE1 LTA3LTMwICBZb3Vlbm4gRmFibGV0ICA8eW91ZW5uLmZhYmxldEBjcmYuY2Fub24uZnI+CiAKICAg ICAgICAgW0VGTF0gaHR0cC90ZXN0cy9tZWRpYS9tZWRpYS1zdHJlYW0vZGlzY29ubmVjdGVkLWZy YW1lLWFscmVhZHkuaHRtbCBpcyBjcmFzaGluZyBhZnRlciByMTg1OTAzCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViQ29yZS9Nb2R1bGVzL3N0cmVhbXMvUmVhZGFibGVTdHJlYW1SZWFkZXIuaWRsIGIv U291cmNlL1dlYkNvcmUvTW9kdWxlcy9zdHJlYW1zL1JlYWRhYmxlU3RyZWFtUmVhZGVyLmlkbApp bmRleCBkYTljZTkzMTIxNGE4Mzg3N2VlZmYxZDI5ODM2NDY0YTRkMTU0MWZjLi40MWFmZTI1ZGI1 OWZkYmFkNzE5YWM5NDIwY2VkOGE0ZjhmYmNhYTI3IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9Nb2R1bGVzL3N0cmVhbXMvUmVhZGFibGVTdHJlYW1SZWFkZXIuaWRsCisrKyBiL1NvdXJjZS9X ZWJDb3JlL01vZHVsZXMvc3RyZWFtcy9SZWFkYWJsZVN0cmVhbVJlYWRlci5pZGwKQEAgLTM3LDUg KzM3LDUgQEAKICAgICBQcm9taXNlIGNhbmNlbChbRGVmYXVsdD1VbmRlZmluZWRdIG9wdGlvbmFs IGFueSByZWFzb24pOwogICAgIFtSYWlzZXNFeGNlcHRpb25dIHZvaWQgcmVsZWFzZUxvY2soKTsK IAotICAgIFtDdXN0b21HZXR0ZXJdIHJlYWRvbmx5IGF0dHJpYnV0ZSBQcm9taXNlIGNsb3NlZDsK KyAgICBbQ3VzdG9tR2V0dGVyLCBDYWNoZWRBdHRyaWJ1dGVdIHJlYWRvbmx5IGF0dHJpYnV0ZSBQ cm9taXNlIGNsb3NlZDsKIH07CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy9q cy9KU1JlYWRhYmxlU3RyZWFtUmVhZGVyQ3VzdG9tLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2JpbmRp bmdzL2pzL0pTUmVhZGFibGVTdHJlYW1SZWFkZXJDdXN0b20uY3BwCmluZGV4IDEzZTJlMWNiMjQ0 NTAyMzE4MmQ3ZjEzZjkwZWViNzQyODRiYWU4NzguLjA2M2FjY2IwNzZjODY2ZDdhOGZkZjZlMDRk NDBmZWQxZTQ0MWQwMTkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL2pzL0pT UmVhZGFibGVTdHJlYW1SZWFkZXJDdXN0b20uY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2JpbmRp bmdzL2pzL0pTUmVhZGFibGVTdHJlYW1SZWFkZXJDdXN0b20uY3BwCkBAIC00NiwxMyArNDYsMTIg QEAgbmFtZXNwYWNlIFdlYkNvcmUgewogCiBKU1ZhbHVlIEpTUmVhZGFibGVTdHJlYW1SZWFkZXI6 OmNsb3NlZChFeGVjU3RhdGUqIGV4ZWMpIGNvbnN0CiB7Ci0gICAgaWYgKG1fY2xvc2VkUHJvbWlz ZURlZmVycmVkKQotICAgICAgICByZXR1cm4gbV9jbG9zZWRQcm9taXNlRGVmZXJyZWQtPnByb21p c2UoKTsKLQotICAgIGNvbnN0X2Nhc3Q8SlNSZWFkYWJsZVN0cmVhbVJlYWRlcio+KHRoaXMpLT5t X2Nsb3NlZFByb21pc2VEZWZlcnJlZC5zZXQoZXhlYy0+dm0oKSwgSlNQcm9taXNlRGVmZXJyZWQ6 OmNyZWF0ZShleGVjLCBnbG9iYWxPYmplY3QoKSkpOwotICAgIGltcGwoKS5jbG9zZWQoRGVmZXJy ZWRXcmFwcGVyKGV4ZWMsIGdsb2JhbE9iamVjdCgpLCBtX2Nsb3NlZFByb21pc2VEZWZlcnJlZC5n ZXQoKSkpOwotCi0gICAgcmV0dXJuIG1fY2xvc2VkUHJvbWlzZURlZmVycmVkLT5wcm9taXNlKCk7 CisgICAgaWYgKCFtX2Nsb3NlZCkgeworICAgICAgICBKU1Byb21pc2VEZWZlcnJlZCogY2xvc2Vk UHJvbWlzZSA9IEpTUHJvbWlzZURlZmVycmVkOjpjcmVhdGUoZXhlYywgZ2xvYmFsT2JqZWN0KCkp OworICAgICAgICBjb25zdF9jYXN0PEpTUmVhZGFibGVTdHJlYW1SZWFkZXIqPih0aGlzKS0+bV9j bG9zZWQuc2V0KGV4ZWMtPnZtKCksIHRoaXMsIGNsb3NlZFByb21pc2UtPnByb21pc2UoKSk7Cisg ICAgICAgIGltcGwoKS5jbG9zZWQoRGVmZXJyZWRXcmFwcGVyKGV4ZWMsIGdsb2JhbE9iamVjdCgp LCBjbG9zZWRQcm9taXNlKSk7CisgICAgfQorICAgIHJldHVybiBtX2Nsb3NlZC5nZXQoKTsKIH0K IAogRW5jb2RlZEpTVmFsdWUgSlNDX0hPU1RfQ0FMTCBjb25zdHJ1Y3RKU1JlYWRhYmxlU3RyZWFt UmVhZGVyKEV4ZWNTdGF0ZSogZXhlYykK