Attachment 423233 Details for Bug 223211 – Patch

[patch] Patch

bug-223211-20210315215459.patch (text/plain), 9.04 KB, created by Imanol Fernandez on 2021-03-15 13:55:00 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Imanol Fernandez

Created: 2021-03-15 13:55:00 PDT

Size: 9.04 KB

patch

obsolete

>Subversion Revision: 274416
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index abf4789f22a6907364e425e1fcaa2d92704654ea..0da80a6881a914756f5f1e88b0a32d1c8f470592 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,28 @@
>+2021-03-15  Imanol Fernandez  <ifernandez@igalia.com>
>+
>+        Protect this in all PlatformOpenXR queue->dispatch() calls
>+        https://bugs.webkit.org/show_bug.cgi?id=223211
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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-15  Imanol Fernandez  <ifernandez@igalia.com>
> 
>         Split WebXR features into enabledFeatures, supportedFeatures and requestedFeatures
>diff --git a/Source/WebCore/Modules/webxr/WebXRSystem.cpp b/Source/WebCore/Modules/webxr/WebXRSystem.cpp
>index 67fb663c76114438eeada3f9b8147329788a53b9..09bc8be562e6827345e341c5f11939b959025b17 100644
>--- a/Source/WebCore/Modules/webxr/WebXRSystem.cpp
>+++ b/Source/WebCore/Modules/webxr/WebXRSystem.cpp
>@@ -100,7 +100,7 @@ void WebXRSystem::ensureImmersiveXRDeviceIsSelected(CompletionHandler<void()>&&
>             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.
>diff --git a/Source/WebCore/platform/xr/PlatformXR.h b/Source/WebCore/platform/xr/PlatformXR.h
>index 7246e328805a17dbdd61aa0d3551ce002abd5519..56d8c6c8bda2d1b8683331d1d12e46aa13ec1f16 100644
>--- a/Source/WebCore/platform/xr/PlatformXR.h
>+++ b/Source/WebCore/platform/xr/PlatformXR.h
>@@ -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 @@ public:
>     // 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 @@ class Instance {
> public:
>     static Instance& singleton();
> 
>-    using DeviceList = Vector<UniqueRef<Device>>;
>+    using DeviceList = Vector<Ref<Device>>;
>     void enumerateImmersiveXRDevices(CompletionHandler<void(const DeviceList&)>&&);
> 
> private:
>diff --git a/Source/WebCore/platform/xr/openxr/OpenXRInstance.cpp b/Source/WebCore/platform/xr/openxr/OpenXRInstance.cpp
>index 8cac7a71fc6fca1c0cf597e2532f8f0467169231..d5ca70875f1db274be8a69ec5cb22af2050ba5b4 100644
>--- a/Source/WebCore/platform/xr/openxr/OpenXRInstance.cpp
>+++ b/Source/WebCore/platform/xr/openxr/OpenXRInstance.cpp
>@@ -86,9 +86,9 @@ Instance::Impl::Impl()
> 
> 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 @@ void Instance::enumerateImmersiveXRDevices(CompletionHandler<void(const DeviceLi
>         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);
>             }));
>diff --git a/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp b/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp
>index a2032ca076ce549f2b654339ed1b9d083e2b2e83..a5ec9f18d90d9984bcee59db6239461543712823 100644
>--- a/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp
>+++ b/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp
>@@ -42,14 +42,25 @@ static bool isSessionReady(XrSessionState state)
>     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 @@ WebCore::IntSize OpenXRDevice::recommendedResolution(SessionMode mode)
> 
> 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::initializeTrackingAndRendering(SessionMode mode)
> 
> 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::initializeReferenceSpace(PlatformXR::ReferenceSpaceType space
> 
> 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 @@ void OpenXRDevice::waitUntilStopping()
>     pollEvents();
>     if (m_sessionState >= XR_SESSION_STATE_STOPPING)
>         return;
>-    m_queue.dispatch([this]() {
>+    m_queue.dispatch([this, protectedThis = makeRef(*this)]() {
>         waitUntilStopping();
>     });
> }
>diff --git a/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.h b/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.h
>index 687f3bb0278144c1e1141c3e43c8a24ea2a5a451..d4e26fce855a561e2dd33f0234a215c4befc4200 100644
>--- a/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.h
>+++ b/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.h
>@@ -46,9 +46,12 @@ class OpenXRExtensions;
> // 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;

Actions: View | Formatted Diff | Diff

Attachments on bug 223211: 423231 | 423233