[WebXR] Make enumerateImmersiveXRDevices() asynchronnous
Created attachment 410216 [details] Patch
Gentle ping for reviewers.
Comment on attachment 410216 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=410216&action=review > Source/WebCore/Modules/webxr/WebXRSystem.cpp:88 > + platformXR.enumerateImmersiveXRDevices([this, isFirstXRDevicesEnumeration, callback = WTFMove(callback)](const Vector<std::unique_ptr<PlatformXR::Device>>& immersiveXRDevices) mutable { There is no guarantee this is valid. We might want to protect it. s/const Vector<std::unique_ptr<PlatformXR::Device>>/auto&/ > Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:3934 > + xrSystem.ensureImmersiveXRDeviceIsSelected([this, promise = WTFMove(promise), &xrSystem]() mutable { There is no guarantee that 'this' stay alive in the callback. For xrSystem, it is guaranteed by ensureImmersiveXRDeviceIsSelected internals, so it might be best to not catch xrSystem by reference and either recompute it inside the callback, or ref it. > Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp:237 > + callback(m_immersiveXRDevices); It does not seem callback will be called in case of RETURN_IF_FAILED(result, "xrGetSystem", m_impl->xrInstance());. Also it does not seem callback is called asynchronously here so it does not seem necessary. I would guess you might want to run that task in a WorkQueue if this is blocking main thread?
Created attachment 410641 [details] Patch
(In reply to youenn fablet from comment #3) > Also it does not seem callback is called asynchronously here so it does not > seem necessary. > I would guess you might want to run that task in a WorkQueue if this is > blocking main thread? Right, this is why I mention in the ChangeLog that I'll post a follow up patch which will move all the OpenXR calls out of the main thread because they're blocking.
Comment on attachment 410641 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=410641&action=review > Source/WebCore/Modules/webxr/WebXRSystem.cpp:378 > + obtainCurrentDevice(mode, init.requiredFeatures, init.optionalFeatures, [this, protectedDocument = makeRef(document), immersive, init, mode, promise = WTFMove(promise)](PlatformXR::Device* device) mutable { COulbe auto* > Source/WebCore/Modules/webxr/WebXRSystem.cpp:380 > + promise.reject(Exception { NotSupportedError }); Might be fine for now. But NotSupportedError might not be good in all early return cases. > Source/WebCore/Modules/webxr/WebXRSystem.cpp:421 > + promise.resolve(session); Could WTFMove session
<rdar://problem/70087114>
Committed r268255: <https://trac.webkit.org/changeset/268255>
Note there's now a "WebXR" component as of Bug 217195. This bug's been recategorized into it; please feel free to do the same for other WebXR bugs.