[Webkit-unassigned] [Bug 220979] Complete XRSession::requestAnimationFrame implementation

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=220979

--- Comment #19 from Imanol Fernandez <ifernandez at igalia.com> ---
Comment on attachment 419584
  --> https://bugs.webkit.org/attachment.cgi?id=419584
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=419584&action=review

>> Source/WebCore/Modules/webxr/WebXRSession.cpp:80
>> +    m_animationFrame = WebXRFrame::create(makeRef(*this));
> 
> This creates a ref cycle. Is there a way to not create a ref cycle?
> Do we need to store the animation frame? As a ref?
> If not, how are we making sure to break it? It seems shutdown will break the ref cycle but I am not sure we will always go there.

One thing we could do is create a new XRFrame every time, that way we don't need to hold any references here. This could work well as long as GC/JavaScriptCore correctly handles short lived objects. There were some problems in the spec on some browsers due to creating many new objects in RAFs, being XRFrame one of them. Check this for more context: https://github.com/immersive-web/webxr/issues/1010. The working group decided to allow reusing XRFrame but it's not mandatory.

>> Source/WebCore/Modules/webxr/WebXRSession.cpp:536
>> +                callback->handleEvent(now, *m_animationFrame);
> 
> This is still a bit weak to have handleEvent here and m_runningCallbacks be accessed elsewhere.
> I wonder how we could make this more robust.

The problem is that XRSession::cancelAnimationFrame needs to mark a pending running callback as cancelled based on the JS code from that RAF specific callback.

I could create a separate set with the pending callbackIds to cancel instead of using m_runningCallbacks vector. I don't see a way to do it without a extra data structure that is accessed from both cancelAnimationFrame and this loop. Any more ideas?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210208/0dd067f2/attachment-0001.htm>