[SameObject] is not currently implemented in WebKit. We need to implement custom getters.
Created attachment 424019 [details] Patch
Comment on attachment 424019 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424019&action=review > Source/WebCore/Modules/webxr/WebXRRigidTransform.idl:38 > + [CustomGetter] readonly attribute Float32Array matrix; Same issue as below. > Source/WebCore/Modules/webxr/WebXRViewerPose.idl:35 > + [CustomGetter, SameObject] readonly attribute FrozenArray<WebXRView> views; You also need [JSCustomMarkFunction] on the interface and custom bindings code for visitAdditionalChildren() which visits cachedViews() on the impl object.
Comment on attachment 424019 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424019&action=review > Source/WebCore/Modules/webxr/WebXRRigidTransform.idl:36 > [SameObject] readonly attribute DOMPointReadOnly position; Note that SameObject does nothing besides documenting. You don't have to do it in this change since it does not seem needed for WPT tests. That said, I suggest you add a new test that does: transfer.position.foo = 1; gc(); setTimeout(() => { gc(); shouldBe("transfer.position.foo", "1"); }, 0); I suspect this will fail because we don't guarantee we are returning the same object after GC. You can see how JSDOMQuad::visitAdditionalChildren() visits its internal DOMPoint attributes to keep them alive. You'll likely need to do the exact same thing as JSDOMQuad::visitAdditionalChildren() for position & orientation attributes here. The inverse attribute will likely be an issue too.
Created attachment 424025 [details] Patch Add JSCustomMarkFunction
(In reply to Chris Dumez from comment #3) > Comment on attachment 424019 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=424019&action=review > > > Source/WebCore/Modules/webxr/WebXRRigidTransform.idl:36 > > [SameObject] readonly attribute DOMPointReadOnly position; > > Note that SameObject does nothing besides documenting. You don't have to do > it in this change since it does not seem needed for WPT tests. That said, I > suggest you add a new test that does: > transfer.position.foo = 1; > gc(); > setTimeout(() => { > gc(); > shouldBe("transfer.position.foo", "1"); > }, 0); > > I suspect this will fail because we don't guarantee we are returning the > same object after GC. You can see how JSDOMQuad::visitAdditionalChildren() > visits its internal DOMPoint attributes to keep them alive. You'll likely > need to do the exact same thing as JSDOMQuad::visitAdditionalChildren() for > position & orientation attributes here. The inverse attribute will likely be > an issue too. Thanks for the info. I'll test that and add the additional visitAdditionalChildren and test if required
(In reply to Chris Dumez from comment #2) > Comment on attachment 424019 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=424019&action=review > > > Source/WebCore/Modules/webxr/WebXRRigidTransform.idl:38 > > + [CustomGetter] readonly attribute Float32Array matrix; > > Same issue as below. > > > Source/WebCore/Modules/webxr/WebXRViewerPose.idl:35 > > + [CustomGetter, SameObject] readonly attribute FrozenArray<WebXRView> views; > > You also need [JSCustomMarkFunction] on the interface and custom bindings > code for visitAdditionalChildren() which visits cachedViews() on the impl > object. Could something like: [CachedAttribute, SameObject] readonly attribute FrozenArray<WebXRView> views; be used instead for a case like this (it's been a while since I looked into this, so I can never quire remember).
Comment on attachment 424025 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424025&action=review > Source/WebCore/Modules/webxr/WebXRViewerPose.idl:36 > + [CustomGetter, SameObject] readonly attribute FrozenArray<WebXRView> views; I suggest you try [CachedAttribute, SameObject] for this FrozenArray, as Sam suggested. If it works, then you won't need custom bindings for this one, which would be nice.
Comment on attachment 424025 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424025&action=review >> Source/WebCore/Modules/webxr/WebXRViewerPose.idl:36 >> + [CustomGetter, SameObject] readonly attribute FrozenArray<WebXRView> views; > > I suggest you try [CachedAttribute, SameObject] for this FrozenArray, as Sam suggested. If it works, then you won't need custom bindings for this one, which would be nice. It works, thanks for the suggestion
Created attachment 424363 [details] Patch for landing
Committed r275102: <https://commits.webkit.org/r275102> All reviewed patches have been landed. Closing bug and clearing flags on attachment 424363 [details].
<rdar://problem/75889305>