Attachment #423233 for bug #223211

View Details Raw Unified Return to Bug 223211

| Differences between



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

Lines 100-106 void WebXRSystem::ensureImmersiveXRDeviceIsSelected(CompletionHandler<void()>&& a/Source/WebCore/Modules/webxr/WebXRSystem.cpp_sec1
- a/Source/WebCore/Modules/webxr/WebXRSystem.cpp -1 / +1 lines
100	return;	100	return;
101	}	101	}
102		102
103	if (m_activeImmersiveSession && oldDevice && immersiveXRDevices.findMatching([&](auto& entry) { return &entry == oldDevice; }) != notFound)	103	if (m_activeImmersiveSession && oldDevice && immersiveXRDevices.findMatching([&](auto& entry) { return entry.ptr() == oldDevice; }) != notFound)
104	ASSERT(m_activeImmersiveDevice.get() == oldDevice);	104	ASSERT(m_activeImmersiveDevice.get() == oldDevice);
105	else {	105	else {
106	// FIXME: implement a better UA selection mechanism if required.	106	// FIXME: implement a better UA selection mechanism if required.



#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>

    // FIXME: handle visibility changes
};

class Device : public ThreadSafeRefCounted<Device>, public CanMakeWeakPtr<Device> {
    WTF_MAKE_FAST_ALLOCATED;
    WTF_MAKE_NONCOPYABLE(Device);
public:

public:
    static Instance& singleton();

    using DeviceList = Vector<Ref<Device>>;
    void enumerateImmersiveXRDevices(CompletionHandler<void(const DeviceList&)>&&);

private:




Instance::Impl::~Impl()
{
    m_workQueue->dispatch([instance = m_instance] {
        if (instance != XR_NULL_HANDLE)
            xrDestroyInstance(instance);
    });
}


        callbackOnExit.release();

        callOnMainThread([this, callback = WTFMove(callback), systemId]() 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);
            }));

Lines 42-55 static bool isSessionReady(XrSessionState state) a/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp_sec1
- a/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp -6 / +17 lines
42	return state >= XR_SESSION_STATE_READY && state < XR_SESSION_STATE_STOPPING;	42	return state >= XR_SESSION_STATE_READY && state < XR_SESSION_STATE_STOPPING;
43	}	43	}
44		44
45	OpenXRDevice::OpenXRDevice(XrInstance instance, XrSystemId system, WorkQueue& queue, const OpenXRExtensions& extensions, CompletionHandler<void()>&& callback)	45	Ref<OpenXRDevice> OpenXRDevice::create(XrInstance instance, XrSystemId system, Ref<WorkQueue>&& queue, const OpenXRExtensions& extensions, CompletionHandler<void()>&& callback)
		46	{
		47	auto device = adoptRef(*new OpenXRDevice(instance, system, WTFMove(queue), extensions));
		48	device->initialize(WTFMove(callback));
		49	return device;
		50	}
		51
		52	OpenXRDevice::OpenXRDevice(XrInstance instance, XrSystemId system, Ref<WorkQueue>&& queue, const OpenXRExtensions& extensions)
46	: m_instance(instance)	53	: m_instance(instance)
47	, m_systemId(system)	54	, m_systemId(system)
48	, m_queue(queue)	55	, m_queue(queue)
49	, m_extensions(extensions)	56	, m_extensions(extensions)
		57	{
		58	}
		59
		60	void OpenXRDevice::initialize(CompletionHandler<void()>&& callback)
50	{	61	{
51	ASSERT(isMainThread());	62	ASSERT(isMainThread());
52	m_queue.dispatch([this, callback = WTFMove(callback)]() mutable {	63	m_queue.dispatch([this, protectedThis = makeRef(*this), callback = WTFMove(callback)]() mutable {
53	auto systemProperties = createStructure<XrSystemProperties, XR_TYPE_SYSTEM_PROPERTIES>();	64	auto systemProperties = createStructure<XrSystemProperties, XR_TYPE_SYSTEM_PROPERTIES>();
54	auto result = xrGetSystemProperties(m_instance, m_systemId, &systemProperties);	65	auto result = xrGetSystemProperties(m_instance, m_systemId, &systemProperties);
55	if (XR_SUCCEEDED(result))	66	if (XR_SUCCEEDED(result))
Lines 78-84 WebCore::IntSize OpenXRDevice::recommendedResolution(SessionMode mode) a/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp_sec2
78		89
79	void OpenXRDevice::initializeTrackingAndRendering(SessionMode mode)	90	void OpenXRDevice::initializeTrackingAndRendering(SessionMode mode)
80	{	91	{
81	m_queue.dispatch([this, mode]() {	92	m_queue.dispatch([this, protectedThis = makeRef(*this), mode]() {
82	ASSERT(m_instance != XR_NULL_HANDLE);	93	ASSERT(m_instance != XR_NULL_HANDLE);
83	ASSERT(m_session == XR_NULL_HANDLE);	94	ASSERT(m_session == XR_NULL_HANDLE);
84	ASSERT(m_extensions.methods().xrGetOpenGLGraphicsRequirementsKHR);	95	ASSERT(m_extensions.methods().xrGetOpenGLGraphicsRequirementsKHR);
Lines 123-129 void OpenXRDevice::initializeTrackingAndRendering(SessionMode mode) a/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp_sec3
123		134
124	void OpenXRDevice::shutDownTrackingAndRendering()	135	void OpenXRDevice::shutDownTrackingAndRendering()
125	{	136	{
126	m_queue.dispatch([this]() {	137	m_queue.dispatch([this, protectedThis = makeRef(*this)]() {
127	if (m_session == XR_NULL_HANDLE)	138	if (m_session == XR_NULL_HANDLE)
128	return;	139	return;
129		140
Lines 149-155 void OpenXRDevice::initializeReferenceSpace(PlatformXR::ReferenceSpaceType space a/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp_sec4
149		160
150	void OpenXRDevice::requestFrame(RequestFrameCallback&& callback)	161	void OpenXRDevice::requestFrame(RequestFrameCallback&& callback)
151	{	162	{
152	m_queue.dispatch([this, callback = WTFMove(callback)]() mutable {	163	m_queue.dispatch([this, protectedThis = makeRef(*this), callback = WTFMove(callback)]() mutable {
153	pollEvents();	164	pollEvents();
154	if (!isSessionReady(m_sessionState)) {	165	if (!isSessionReady(m_sessionState)) {
155	callOnMainThread([callback = WTFMove(callback)]() mutable {	166	callOnMainThread([callback = WTFMove(callback)]() mutable {
Lines 448-454 void OpenXRDevice::waitUntilStopping() a/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp_sec5
448	pollEvents();	459	pollEvents();
449	if (m_sessionState >= XR_SESSION_STATE_STOPPING)	460	if (m_sessionState >= XR_SESSION_STATE_STOPPING)
450	return;	461	return;
451	m_queue.dispatch([this]() {	462	m_queue.dispatch([this, protectedThis = makeRef(*this)]() {
452	waitUntilStopping();	463	waitUntilStopping();
453	});	464	});
454	}	465	}

Lines 46-54 class OpenXRExtensions; a/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.h_sec1
- a/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.h -1 / +4 lines
46	// the XRSystem is basically the entry point for the WebXR API available via the Navigator object.	46	// the XRSystem is basically the entry point for the WebXR API available via the Navigator object.
47	class OpenXRDevice final : public Device {	47	class OpenXRDevice final : public Device {
48	public:	48	public:
49	OpenXRDevice(XrInstance, XrSystemId, WorkQueue&, const OpenXRExtensions&, CompletionHandler<void()>&&);	49	static Ref<OpenXRDevice> create(XrInstance, XrSystemId, Ref<WorkQueue>&&, const OpenXRExtensions&, CompletionHandler<void()>&&);
50		50
51	private:	51	private:
		52	OpenXRDevice(XrInstance, XrSystemId, Ref<WorkQueue>&&, const OpenXRExtensions&);
		53	void initialize(CompletionHandler<void()>&& callback);
		54
52	// PlatformXR::Device	55	// PlatformXR::Device
53	WebCore::IntSize recommendedResolution(SessionMode) final;	56	WebCore::IntSize recommendedResolution(SessionMode) final;
54	void initializeTrackingAndRendering(SessionMode) final;	57	void initializeTrackingAndRendering(SessionMode) final;

Return to Bug 223211

Lines 1-3 a/Source/WebCore/ChangeLog_sec1
- a/Source/WebCore/ChangeLog +25 lines
		1	2021-03-15 Imanol Fernandez <ifernandez@igalia.com>
		2
		3	Protect this in all PlatformOpenXR queue->dispatch() calls
		4	https://bugs.webkit.org/show_bug.cgi?id=223211
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	The OpenXR WorkQueue may outlive OpenXRDevice so queue->dispatch() calls without protecting this are unsafe.
		9
		10	* Modules/webxr/WebXRSystem.cpp:
		11	(WebCore::WebXRSystem::ensureImmersiveXRDeviceIsSelected):
		12	* platform/xr/PlatformXR.h:
		13	* platform/xr/openxr/OpenXRInstance.cpp:
		14	(PlatformXR::Instance::Impl::~Impl):
		15	(PlatformXR::Instance::enumerateImmersiveXRDevices):
		16	* platform/xr/openxr/PlatformXROpenXR.cpp:
		17	(PlatformXR::OpenXRDevice::create):
		18	(PlatformXR::OpenXRDevice::OpenXRDevice):
		19	(PlatformXR::OpenXRDevice::initialize):
		20	(PlatformXR::OpenXRDevice::initializeTrackingAndRendering):
		21	(PlatformXR::OpenXRDevice::shutDownTrackingAndRendering):
		22	(PlatformXR::OpenXRDevice::requestFrame):
		23	(PlatformXR::OpenXRDevice::waitUntilStopping):
		24	* platform/xr/openxr/PlatformXROpenXR.h:
		25
1	2021-03-15 Imanol Fernandez <ifernandez@igalia.com>	26	2021-03-15 Imanol Fernandez <ifernandez@igalia.com>
2		27
3	Split WebXR features into enabledFeatures, supportedFeatures and requestedFeatures	28	Split WebXR features into enabledFeatures, supportedFeatures and requestedFeatures

Lines 23-28 a/Source/WebCore/platform/xr/PlatformXR.h_sec1
- a/Source/WebCore/platform/xr/PlatformXR.h -2 / +3 lines
23	#include <memory>	23	#include <memory>
24	#include <wtf/CompletionHandler.h>	24	#include <wtf/CompletionHandler.h>
25	#include <wtf/HashMap.h>	25	#include <wtf/HashMap.h>
		26	#include <wtf/Ref.h>
26	#include <wtf/UniqueRef.h>	27	#include <wtf/UniqueRef.h>
27	#include <wtf/Variant.h>	28	#include <wtf/Variant.h>
28	#include <wtf/Vector.h>	29	#include <wtf/Vector.h>
Lines 61-67 public: a/Source/WebCore/platform/xr/PlatformXR.h_sec2
61	// FIXME: handle visibility changes	62	// FIXME: handle visibility changes
62	};	63	};
63		64
64	class Device : public CanMakeWeakPtr<Device> {	65	class Device : public ThreadSafeRefCounted<Device>, public CanMakeWeakPtr<Device> {
65	WTF_MAKE_FAST_ALLOCATED;	66	WTF_MAKE_FAST_ALLOCATED;
66	WTF_MAKE_NONCOPYABLE(Device);	67	WTF_MAKE_NONCOPYABLE(Device);
67	public:	68	public:
Lines 159-165 class Instance { a/Source/WebCore/platform/xr/PlatformXR.h_sec3
159	public:	160	public:
160	static Instance& singleton();	161	static Instance& singleton();
161		162
162	using DeviceList = Vector<UniqueRef<Device>>;	163	using DeviceList = Vector<Ref<Device>>;
163	void enumerateImmersiveXRDevices(CompletionHandler<void(const DeviceList&)>&&);	164	void enumerateImmersiveXRDevices(CompletionHandler<void(const DeviceList&)>&&);
164		165
165	private:	166	private:

Lines 86-94 Instance::Impl::Impl() a/Source/WebCore/platform/xr/openxr/OpenXRInstance.cpp_sec1
- a/Source/WebCore/platform/xr/openxr/OpenXRInstance.cpp -4 / +4 lines
86		86
87	Instance::Impl::~Impl()	87	Instance::Impl::~Impl()
88	{	88	{
89	m_workQueue->dispatch([this] {	89	m_workQueue->dispatch([instance = m_instance] {
90	if (m_instance != XR_NULL_HANDLE)	90	if (instance != XR_NULL_HANDLE)
91	xrDestroyInstance(m_instance);	91	xrDestroyInstance(instance);
92	});	92	});
93	}	93	}
94		94
Lines 132-138 void Instance::enumerateImmersiveXRDevices(CompletionHandler<void(const DeviceLi a/Source/WebCore/platform/xr/openxr/OpenXRInstance.cpp_sec2
132	callbackOnExit.release();	132	callbackOnExit.release();
133		133
134	callOnMainThread([this, callback = WTFMove(callback), systemId]() mutable {	134	callOnMainThread([this, callback = WTFMove(callback), systemId]() mutable {
135	m_immersiveXRDevices = DeviceList::from(makeUniqueRef<OpenXRDevice>(m_impl->xrInstance(), systemId, m_impl->queue(), *m_impl->extensions(), [this, callback = WTFMove(callback)]() mutable {	135	m_immersiveXRDevices = DeviceList::from(OpenXRDevice::create(m_impl->xrInstance(), systemId, m_impl->queue(), *m_impl->extensions(), [this, callback = WTFMove(callback)]() mutable {
136	ASSERT(isMainThread());	136	ASSERT(isMainThread());
137	callback(m_immersiveXRDevices);	137	callback(m_immersiveXRDevices);
138	}));	138	}));