203963 2019-11-07 09:33:45 -0800 Port ServiceWorkerContainer to the HTML5 event loop 2019-11-07 14:37:19 -0800 1 1 1 Unclassified WebKit Service Workers WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 202843 1 cdumez cdumez commit-queue ggaren rniwa webkit-bug-importer oldest_to_newest 1588256 0 cdumez 2019-11-07 09:33:45 -0800 Port ServiceWorkerContainer to the HTML5 event loop. 1588257 1 383058 cdumez 2019-11-07 09:34:28 -0800 Created attachment 383058 Patch 1588380 2 383058 rniwa 2019-11-07 13:38:42 -0800 Comment on attachment 383058 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383058&action=review > Source/WebCore/workers/service/ServiceWorkerContainer.h:145 > + bool m_shouldDeferMessageEvents { false }; > + Vector<Ref<Event>> m_deferredMessageEvents; Hm... how does GC keep wrappers for objects held by this event alive? 1588387 3 cdumez 2019-11-07 13:52:40 -0800 (In reply to Ryosuke Niwa from comment #2) > Comment on attachment 383058 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=383058&action=review > > > Source/WebCore/workers/service/ServiceWorkerContainer.h:145 > > + bool m_shouldDeferMessageEvents { false }; > > + Vector<Ref<Event>> m_deferredMessageEvents; > > Hm... how does GC keep wrappers for objects held by this event alive? I don't think we have any logic to keep those wrappers alive currently. This is not a regression in this patch though. 1588388 4 rniwa 2019-11-07 13:54:35 -0800 (In reply to Chris Dumez from comment #3) > (In reply to Ryosuke Niwa from comment #2) > > Comment on attachment 383058 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=383058&action=review > > > > > Source/WebCore/workers/service/ServiceWorkerContainer.h:145 > > > + bool m_shouldDeferMessageEvents { false }; > > > + Vector<Ref<Event>> m_deferredMessageEvents; > > > > Hm... how does GC keep wrappers for objects held by this event alive? > > I don't think we have any logic to keep those wrappers alive currently. This > is not a regression in this patch though. Okay, sounds like a bug we should track somewhere. How do we track these bugs for service workers? 1588389 5 cdumez 2019-11-07 13:55:53 -0800 (In reply to Ryosuke Niwa from comment #4) > (In reply to Chris Dumez from comment #3) > > (In reply to Ryosuke Niwa from comment #2) > > > Comment on attachment 383058 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=383058&action=review > > > > > > > Source/WebCore/workers/service/ServiceWorkerContainer.h:145 > > > > + bool m_shouldDeferMessageEvents { false }; > > > > + Vector<Ref<Event>> m_deferredMessageEvents; > > > > > > Hm... how does GC keep wrappers for objects held by this event alive? > > > > I don't think we have any logic to keep those wrappers alive currently. This > > is not a regression in this patch though. > > Okay, sounds like a bug we should track somewhere. How do we track these > bugs for service workers? It is unclear to me at this point that we want to keep those wrappers alive though. You're talking about wrappers to MessagePorts I assume? 1588398 6 rniwa 2019-11-07 14:10:02 -0800 (In reply to Chris Dumez from comment #5) > (In reply to Ryosuke Niwa from comment #4) > > (In reply to Chris Dumez from comment #3) > > > (In reply to Ryosuke Niwa from comment #2) > > > > Comment on attachment 383058 [details] > > > > Patch > > > > > > > > View in context: > > > > https://bugs.webkit.org/attachment.cgi?id=383058&action=review > > > > > > > > > Source/WebCore/workers/service/ServiceWorkerContainer.h:145 > > > > > + bool m_shouldDeferMessageEvents { false }; > > > > > + Vector<Ref<Event>> m_deferredMessageEvents; > > > > > > > > Hm... how does GC keep wrappers for objects held by this event alive? > > > > > > I don't think we have any logic to keep those wrappers alive currently. This > > > is not a regression in this patch though. > > > > Okay, sounds like a bug we should track somewhere. How do we track these > > bugs for service workers? > > It is unclear to me at this point that we want to keep those wrappers alive > though. You're talking about wrappers to MessagePorts I assume? Yeah, MessagePorts and other transferred objects. Because you can postMessage to yourself, or some other window, right? In those cases, it's totally observable as to whether the wrapper has changed or not. 1588415 7 383058 commit-queue 2019-11-07 14:36:52 -0800 Comment on attachment 383058 Patch Clearing flags on attachment: 383058 Committed r252206: <https://trac.webkit.org/changeset/252206> 1588416 8 commit-queue 2019-11-07 14:36:53 -0800 All reviewed patches have been landed. Closing bug. 1588417 9 webkit-bug-importer 2019-11-07 14:37:19 -0800 <rdar://problem/56998897> 383058 2019-11-07 09:34:28 -0800 2019-11-07 14:36:52 -0800 Patch bug-203963-20191107093427.patch text/plain 4197 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMjUyMTg1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZDYzYWVhM2I4MTZkNTMw NWRjMDcwZDEzZThlZWVmNGVmMWJiOGFjNS4uNzc1NGU1NjkwZTFhMThhZjFiZDJkNmM3Yzg2ZGI1 N2I1NGQ1ZTk1MSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDE5LTExLTA3ICBDaHJp cyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CisKKyAgICAgICAgUG9ydCBTZXJ2aWNlV29ya2Vy Q29udGFpbmVyIHRvIHRoZSBIVE1MNSBldmVudCBsb29wCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMDM5NjMKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIHdvcmtlcnMvc2VydmljZS9TZXJ2aWNlV29ya2Vy Q29udGFpbmVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlNlcnZpY2VXb3JrZXJDb250YWluZXI6 OlNlcnZpY2VXb3JrZXJDb250YWluZXIpOgorICAgICAgICAoV2ViQ29yZTo6U2VydmljZVdvcmtl ckNvbnRhaW5lcjo6c3RhcnRNZXNzYWdlcyk6CisgICAgICAgIChXZWJDb3JlOjpTZXJ2aWNlV29y a2VyQ29udGFpbmVyOjpwb3N0TWVzc2FnZSk6CisgICAgICAgICogd29ya2Vycy9zZXJ2aWNlL1Nl cnZpY2VXb3JrZXJDb250YWluZXIuaDoKKwogMjAxOS0xMS0wNyAgQWxleCBDaHJpc3RlbnNlbiAg PGFjaHJpc3RlbnNlbkB3ZWJraXQub3JnPgogCiAgICAgICAgIFJlLWVuYWJsZSBOU1VSTFNlc3Np b24gaXNvbGF0aW9uIGFmdGVyIHIyNTIxMTYKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3dv cmtlcnMvc2VydmljZS9TZXJ2aWNlV29ya2VyQ29udGFpbmVyLmNwcCBiL1NvdXJjZS9XZWJDb3Jl L3dvcmtlcnMvc2VydmljZS9TZXJ2aWNlV29ya2VyQ29udGFpbmVyLmNwcAppbmRleCBjZjNhMzdi MzZiNjJmOGI1ZDc4ZjEwZjZiNjdlNThjMTJmZjYxMGVlLi4zOWE3MWQ4NTVmZWFiMTg3NDk2OWQz MDU4ZWE0MGM5MzAxYTVjNDU4IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS93b3JrZXJzL3Nl cnZpY2UvU2VydmljZVdvcmtlckNvbnRhaW5lci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvd29y a2Vycy9zZXJ2aWNlL1NlcnZpY2VXb3JrZXJDb250YWluZXIuY3BwCkBAIC03MywxMyArNzMsMTIg QEAgV1RGX01BS0VfSVNPX0FMTE9DQVRFRF9JTVBMKFNlcnZpY2VXb3JrZXJDb250YWluZXIpOwog U2VydmljZVdvcmtlckNvbnRhaW5lcjo6U2VydmljZVdvcmtlckNvbnRhaW5lcihTY3JpcHRFeGVj dXRpb25Db250ZXh0KiBjb250ZXh0LCBOYXZpZ2F0b3JCYXNlJiBuYXZpZ2F0b3IpCiAgICAgOiBB Y3RpdmVET01PYmplY3QoY29udGV4dCkKICAgICAsIG1fbmF2aWdhdG9yKG5hdmlnYXRvcikKLSAg ICAsIG1fbWVzc2FnZVF1ZXVlKEdlbmVyaWNFdmVudFF1ZXVlOjpjcmVhdGUoKnRoaXMpKQogewog ICAgIHN1c3BlbmRJZk5lZWRlZCgpOwogICAgIAogICAgIC8vIFdlIHNob3VsZCBxdWV1ZSBtZXNz YWdlcyB1bnRpbCB0aGUgRE9NQ29udGVudExvYWRlZCBldmVudCBoYXMgZmlyZWQgb3Igc3RhcnRN ZXNzYWdlcygpIGhhcyBiZWVuIGNhbGxlZC4KICAgICBpZiAoaXM8RG9jdW1lbnQ+KGNvbnRleHQp ICYmIGRvd25jYXN0PERvY3VtZW50PigqY29udGV4dCkucGFyc2luZygpKQotICAgICAgICBtX21l c3NhZ2VRdWV1ZS0+c2V0UGF1c2VkKHRydWUpOworICAgICAgICBtX3Nob3VsZERlZmVyTWVzc2Fn ZUV2ZW50cyA9IHRydWU7CiB9CiAKIFNlcnZpY2VXb3JrZXJDb250YWluZXI6On5TZXJ2aWNlV29y a2VyQ29udGFpbmVyKCkKQEAgLTMxNCw3ICszMTMsMTAgQEAgdm9pZCBTZXJ2aWNlV29ya2VyQ29u dGFpbmVyOjpnZXRSZWdpc3RyYXRpb25zKFJlZjxEZWZlcnJlZFByb21pc2U+JiYgcHJvbWlzZSkK IAogdm9pZCBTZXJ2aWNlV29ya2VyQ29udGFpbmVyOjpzdGFydE1lc3NhZ2VzKCkKIHsKLSAgICBt X21lc3NhZ2VRdWV1ZS0+c2V0UGF1c2VkKGZhbHNlKTsKKyAgICBtX3Nob3VsZERlZmVyTWVzc2Fn ZUV2ZW50cyA9IGZhbHNlOworICAgIGF1dG8gZGVmZXJyZWRNZXNzYWdlRXZlbnRzID0gV1RGTW92 ZShtX2RlZmVycmVkTWVzc2FnZUV2ZW50cyk7CisgICAgZm9yIChhdXRvJiBtZXNzYWdlRXZlbnQg OiBkZWZlcnJlZE1lc3NhZ2VFdmVudHMpCisgICAgICAgIHF1ZXVlVGFza1RvRGlzcGF0Y2hFdmVu dCgqdGhpcywgVGFza1NvdXJjZTo6RE9NTWFuaXB1bGF0aW9uLCBXVEZNb3ZlKG1lc3NhZ2VFdmVu dCkpOwogfQogCiB2b2lkIFNlcnZpY2VXb3JrZXJDb250YWluZXI6OmpvYkZhaWxlZFdpdGhFeGNl cHRpb24oU2VydmljZVdvcmtlckpvYiYgam9iLCBjb25zdCBFeGNlcHRpb24mIGV4Y2VwdGlvbikK QEAgLTQwNCw4ICs0MDYsMTIgQEAgdm9pZCBTZXJ2aWNlV29ya2VyQ29udGFpbmVyOjpwb3N0TWVz c2FnZShNZXNzYWdlV2l0aE1lc3NhZ2VQb3J0cyYmIG1lc3NhZ2UsIFNlcnYKICAgICBNZXNzYWdl RXZlbnRTb3VyY2Ugc291cmNlID0gUmVmUHRyPFNlcnZpY2VXb3JrZXI+IHsgU2VydmljZVdvcmtl cjo6Z2V0T3JDcmVhdGUoY29udGV4dCwgV1RGTW92ZShzb3VyY2VEYXRhKSkgfTsKIAogICAgIGF1 dG8gbWVzc2FnZUV2ZW50ID0gTWVzc2FnZUV2ZW50OjpjcmVhdGUoTWVzc2FnZVBvcnQ6OmVudGFu Z2xlUG9ydHMoY29udGV4dCwgV1RGTW92ZShtZXNzYWdlLnRyYW5zZmVycmVkUG9ydHMpKSwgbWVz c2FnZS5tZXNzYWdlLnJlbGVhc2VOb25OdWxsKCksIHNvdXJjZU9yaWdpbiwgeyB9LCBXVEZNb3Zl KHNvdXJjZSkpOwotICAgIAotICAgIG1fbWVzc2FnZVF1ZXVlLT5lbnF1ZXVlRXZlbnQoV1RGTW92 ZShtZXNzYWdlRXZlbnQpKTsKKyAgICBpZiAobV9zaG91bGREZWZlck1lc3NhZ2VFdmVudHMpCisg ICAgICAgIG1fZGVmZXJyZWRNZXNzYWdlRXZlbnRzLmFwcGVuZChXVEZNb3ZlKG1lc3NhZ2VFdmVu dCkpOworICAgIGVsc2UgeworICAgICAgICBBU1NFUlQobV9kZWZlcnJlZE1lc3NhZ2VFdmVudHMu aXNFbXB0eSgpKTsKKyAgICAgICAgcXVldWVUYXNrVG9EaXNwYXRjaEV2ZW50KCp0aGlzLCBUYXNr U291cmNlOjpET01NYW5pcHVsYXRpb24sIFdURk1vdmUobWVzc2FnZUV2ZW50KSk7CisgICAgfQog fQogCiB2b2lkIFNlcnZpY2VXb3JrZXJDb250YWluZXI6Om5vdGlmeVJlZ2lzdHJhdGlvbklzU2V0 dGxlZChjb25zdCBTZXJ2aWNlV29ya2VyUmVnaXN0cmF0aW9uS2V5JiByZWdpc3RyYXRpb25LZXkp CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS93b3JrZXJzL3NlcnZpY2UvU2VydmljZVdvcmtl ckNvbnRhaW5lci5oIGIvU291cmNlL1dlYkNvcmUvd29ya2Vycy9zZXJ2aWNlL1NlcnZpY2VXb3Jr ZXJDb250YWluZXIuaAppbmRleCBkM2E2OTczNGM1Nzg1MzU0NmYzZWJlMmIwNWQzY2QyZDdmNWU3 YTFkLi44YzQxOWJkZGM5OTRjMzhmZjc0MWQ1YTc4OTdkY2ExOTI3OGU0ZGQ0IDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViQ29yZS93b3JrZXJzL3NlcnZpY2UvU2VydmljZVdvcmtlckNvbnRhaW5lci5o CisrKyBiL1NvdXJjZS9XZWJDb3JlL3dvcmtlcnMvc2VydmljZS9TZXJ2aWNlV29ya2VyQ29udGFp bmVyLmgKQEAgLTI5LDcgKzI5LDYgQEAKIAogI2luY2x1ZGUgIkFjdGl2ZURPTU9iamVjdC5oIgog I2luY2x1ZGUgIkV2ZW50VGFyZ2V0LmgiCi0jaW5jbHVkZSAiR2VuZXJpY0V2ZW50UXVldWUuaCIK ICNpbmNsdWRlICJJRExUeXBlcy5oIgogI2luY2x1ZGUgIlNXQ2xpZW50Q29ubmVjdGlvbi5oIgog I2luY2x1ZGUgIlNXU2VydmVyLmgiCkBAIC0xNDIsNyArMTQxLDggQEAgcHJpdmF0ZToKIAogICAg IHVpbnQ2NF90IG1fbGFzdE9uZ29pbmdTZXR0bGVkUmVnaXN0cmF0aW9uSWRlbnRpZmllciB7IDAg fTsKICAgICBIYXNoTWFwPHVpbnQ2NF90LCBTZXJ2aWNlV29ya2VyUmVnaXN0cmF0aW9uS2V5PiBt X29uZ29pbmdTZXR0bGVkUmVnaXN0cmF0aW9uczsKLSAgICBVbmlxdWVSZWY8R2VuZXJpY0V2ZW50 UXVldWU+IG1fbWVzc2FnZVF1ZXVlOworICAgIGJvb2wgbV9zaG91bGREZWZlck1lc3NhZ2VFdmVu dHMgeyBmYWxzZSB9OworICAgIFZlY3RvcjxSZWY8RXZlbnQ+PiBtX2RlZmVycmVkTWVzc2FnZUV2 ZW50czsKIH07CiAKIH0gLy8gbmFtZXNwYWNlIFdlYkNvcmUK