Title: [274470] trunk/Source/WebCore
Revision
274470
Author
ifernan...@igalia.com
Date
2021-03-16 01:45:51 -0700 (Tue, 16 Mar 2021)

Log Message

Protect this in all PlatformOpenXR queue->dispatch() calls
https://bugs.webkit.org/show_bug.cgi?id=223211

Reviewed by Chris Dumez.

The OpenXR WorkQueue may outlive OpenXRDevice so queue->dispatch() calls without protecting this are unsafe.

* Modules/webxr/WebXRSystem.cpp:
(WebCore::WebXRSystem::ensureImmersiveXRDeviceIsSelected):
* platform/xr/PlatformXR.h:
* platform/xr/openxr/OpenXRInstance.cpp:
(PlatformXR::Instance::Impl::~Impl):
(PlatformXR::Instance::enumerateImmersiveXRDevices):
* platform/xr/openxr/PlatformXROpenXR.cpp:
(PlatformXR::OpenXRDevice::create):
(PlatformXR::OpenXRDevice::OpenXRDevice):
(PlatformXR::OpenXRDevice::initialize):
(PlatformXR::OpenXRDevice::initializeTrackingAndRendering):
(PlatformXR::OpenXRDevice::shutDownTrackingAndRendering):
(PlatformXR::OpenXRDevice::requestFrame):
(PlatformXR::OpenXRDevice::waitUntilStopping):
* platform/xr/openxr/PlatformXROpenXR.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (274469 => 274470)


--- trunk/Source/WebCore/ChangeLog	2021-03-16 08:25:52 UTC (rev 274469)
+++ trunk/Source/WebCore/ChangeLog	2021-03-16 08:45:51 UTC (rev 274470)
@@ -1,3 +1,28 @@
+2021-03-16  Imanol Fernandez  <ifernan...@igalia.com>
+
+        Protect this in all PlatformOpenXR queue->dispatch() calls
+        https://bugs.webkit.org/show_bug.cgi?id=223211
+
+        Reviewed by Chris Dumez.
+
+        The OpenXR WorkQueue may outlive OpenXRDevice so queue->dispatch() calls without protecting this are unsafe.
+
+        * Modules/webxr/WebXRSystem.cpp:
+        (WebCore::WebXRSystem::ensureImmersiveXRDeviceIsSelected):
+        * platform/xr/PlatformXR.h:
+        * platform/xr/openxr/OpenXRInstance.cpp:
+        (PlatformXR::Instance::Impl::~Impl):
+        (PlatformXR::Instance::enumerateImmersiveXRDevices):
+        * platform/xr/openxr/PlatformXROpenXR.cpp:
+        (PlatformXR::OpenXRDevice::create):
+        (PlatformXR::OpenXRDevice::OpenXRDevice):
+        (PlatformXR::OpenXRDevice::initialize):
+        (PlatformXR::OpenXRDevice::initializeTrackingAndRendering):
+        (PlatformXR::OpenXRDevice::shutDownTrackingAndRendering):
+        (PlatformXR::OpenXRDevice::requestFrame):
+        (PlatformXR::OpenXRDevice::waitUntilStopping):
+        * platform/xr/openxr/PlatformXROpenXR.h:
+
 2021-03-16  Youenn Fablet  <you...@apple.com>
 
         Add a new delegate for geolocation permission

Modified: trunk/Source/WebCore/Modules/webxr/WebXRSystem.cpp (274469 => 274470)


--- trunk/Source/WebCore/Modules/webxr/WebXRSystem.cpp	2021-03-16 08:25:52 UTC (rev 274469)
+++ trunk/Source/WebCore/Modules/webxr/WebXRSystem.cpp	2021-03-16 08:45:51 UTC (rev 274470)
@@ -100,7 +100,7 @@
             return;
         }
 
-        if (m_activeImmersiveSession && oldDevice && immersiveXRDevices.findMatching([&](auto& entry) { return &entry == oldDevice; }) != notFound)
+        if (m_activeImmersiveSession && oldDevice && immersiveXRDevices.findMatching([&](auto& entry) { return entry.ptr() == oldDevice; }) != notFound)
             ASSERT(m_activeImmersiveDevice.get() == oldDevice);
         else {
             // FIXME: implement a better UA selection mechanism if required.

Modified: trunk/Source/WebCore/platform/xr/PlatformXR.h (274469 => 274470)


--- trunk/Source/WebCore/platform/xr/PlatformXR.h	2021-03-16 08:25:52 UTC (rev 274469)
+++ trunk/Source/WebCore/platform/xr/PlatformXR.h	2021-03-16 08:45:51 UTC (rev 274470)
@@ -23,6 +23,7 @@
 #include <memory>
 #include <wtf/CompletionHandler.h>
 #include <wtf/HashMap.h>
+#include <wtf/Ref.h>
 #include <wtf/UniqueRef.h>
 #include <wtf/Variant.h>
 #include <wtf/Vector.h>
@@ -61,7 +62,7 @@
     // FIXME: handle visibility changes
 };
 
-class Device : public CanMakeWeakPtr<Device> {
+class Device : public ThreadSafeRefCounted<Device>, public CanMakeWeakPtr<Device> {
     WTF_MAKE_FAST_ALLOCATED;
     WTF_MAKE_NONCOPYABLE(Device);
 public:
@@ -159,7 +160,7 @@
 public:
     static Instance& singleton();
 
-    using DeviceList = Vector<UniqueRef<Device>>;
+    using DeviceList = Vector<Ref<Device>>;
     void enumerateImmersiveXRDevices(CompletionHandler<void(const DeviceList&)>&&);
 
 private:

Modified: trunk/Source/WebCore/platform/xr/openxr/OpenXRInstance.cpp (274469 => 274470)


--- trunk/Source/WebCore/platform/xr/openxr/OpenXRInstance.cpp	2021-03-16 08:25:52 UTC (rev 274469)
+++ trunk/Source/WebCore/platform/xr/openxr/OpenXRInstance.cpp	2021-03-16 08:45:51 UTC (rev 274470)
@@ -86,9 +86,9 @@
 
 Instance::Impl::~Impl()
 {
-    m_workQueue->dispatch([this] {
-        if (m_instance != XR_NULL_HANDLE)
-            xrDestroyInstance(m_instance);
+    m_workQueue->dispatch([instance = m_instance] {
+        if (instance != XR_NULL_HANDLE)
+            xrDestroyInstance(instance);
     });
 }
 
@@ -132,7 +132,7 @@
         callbackOnExit.release();
 
         callOnMainThread([this, callback = WTFMove(callback), systemId]() mutable {
-            m_immersiveXRDevices = DeviceList::from(makeUniqueRef<OpenXRDevice>(m_impl->xrInstance(), systemId, m_impl->queue(), *m_impl->extensions(), [this, callback = WTFMove(callback)]() mutable {
+            m_immersiveXRDevices = DeviceList::from(OpenXRDevice::create(m_impl->xrInstance(), systemId, m_impl->queue(), *m_impl->extensions(), [this, callback = WTFMove(callback)]() mutable {
                 ASSERT(isMainThread());
                 callback(m_immersiveXRDevices);
             }));

Modified: trunk/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp (274469 => 274470)


--- trunk/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp	2021-03-16 08:25:52 UTC (rev 274469)
+++ trunk/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp	2021-03-16 08:45:51 UTC (rev 274470)
@@ -42,14 +42,25 @@
     return state >= XR_SESSION_STATE_READY  && state < XR_SESSION_STATE_STOPPING;
 }
 
-OpenXRDevice::OpenXRDevice(XrInstance instance, XrSystemId system, WorkQueue& queue, const OpenXRExtensions& extensions, CompletionHandler<void()>&& callback)
+Ref<OpenXRDevice> OpenXRDevice::create(XrInstance instance, XrSystemId system, Ref<WorkQueue>&& queue, const OpenXRExtensions& extensions, CompletionHandler<void()>&& callback)
+{
+    auto device = adoptRef(*new OpenXRDevice(instance, system, WTFMove(queue), extensions));
+    device->initialize(WTFMove(callback));
+    return device;
+}
+
+OpenXRDevice::OpenXRDevice(XrInstance instance, XrSystemId system, Ref<WorkQueue>&& queue, const OpenXRExtensions& extensions)
     : m_instance(instance)
     , m_systemId(system)
     , m_queue(queue)
     , m_extensions(extensions)
 {
+}
+
+void OpenXRDevice::initialize(CompletionHandler<void()>&& callback)
+{
     ASSERT(isMainThread());
-    m_queue.dispatch([this, callback = WTFMove(callback)]() mutable {
+    m_queue.dispatch([this, protectedThis = makeRef(*this), callback = WTFMove(callback)]() mutable {
         auto systemProperties = createStructure<XrSystemProperties, XR_TYPE_SYSTEM_PROPERTIES>();
         auto result = xrGetSystemProperties(m_instance, m_systemId, &systemProperties);
         if (XR_SUCCEEDED(result))
@@ -78,7 +89,7 @@
 
 void OpenXRDevice::initializeTrackingAndRendering(SessionMode mode)
 {
-    m_queue.dispatch([this, mode]() {
+    m_queue.dispatch([this, protectedThis = makeRef(*this), mode]() {
         ASSERT(m_instance != XR_NULL_HANDLE);
         ASSERT(m_session == XR_NULL_HANDLE);
         ASSERT(m_extensions.methods().xrGetOpenGLGraphicsRequirementsKHR);
@@ -123,7 +134,7 @@
 
 void OpenXRDevice::shutDownTrackingAndRendering()
 {
-    m_queue.dispatch([this]() {
+    m_queue.dispatch([this, protectedThis = makeRef(*this)]() {
         if (m_session == XR_NULL_HANDLE)
             return;
 
@@ -149,7 +160,7 @@
 
 void OpenXRDevice::requestFrame(RequestFrameCallback&& callback)
 {
-    m_queue.dispatch([this, callback = WTFMove(callback)]() mutable {
+    m_queue.dispatch([this, protectedThis = makeRef(*this), callback = WTFMove(callback)]() mutable {
         pollEvents();
         if (!isSessionReady(m_sessionState)) {
             callOnMainThread([callback = WTFMove(callback)]() mutable {
@@ -448,7 +459,7 @@
     pollEvents();
     if (m_sessionState >= XR_SESSION_STATE_STOPPING)
         return;
-    m_queue.dispatch([this]() {
+    m_queue.dispatch([this, protectedThis = makeRef(*this)]() {
         waitUntilStopping();
     });
 }

Modified: trunk/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.h (274469 => 274470)


--- trunk/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.h	2021-03-16 08:25:52 UTC (rev 274469)
+++ trunk/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.h	2021-03-16 08:45:51 UTC (rev 274470)
@@ -46,9 +46,12 @@
 // the XRSystem is basically the entry point for the WebXR API available via the Navigator object.
 class OpenXRDevice final : public Device {
 public:
-    OpenXRDevice(XrInstance, XrSystemId, WorkQueue&, const OpenXRExtensions&, CompletionHandler<void()>&&);
+    static Ref<OpenXRDevice> create(XrInstance, XrSystemId, Ref<WorkQueue>&&, const OpenXRExtensions&, CompletionHandler<void()>&&);
 
 private:
+    OpenXRDevice(XrInstance, XrSystemId, Ref<WorkQueue>&&, const OpenXRExtensions&);
+    void initialize(CompletionHandler<void()>&& callback);
+
     // PlatformXR::Device
     WebCore::IntSize recommendedResolution(SessionMode) final;
     void initializeTrackingAndRendering(SessionMode) final;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to