⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 283880 in webkit


Ignore:
Timestamp:
Oct 10, 2021, 4:05:00 PM (5 years ago)
Author:
dino@apple.com
Message:

[WebXR] Replace the session reference in WebXRSpace subclasses with weak pointers
​https://bugs.webkit.org/show_bug.cgi?id=231482

Reviewed by Sam Weinig.

WebXRSpace had a pure virtual session() accessor that returned a
reference to a WebXRSession. This made subclasses hold strong
references to the WebXRSession, and is problematic for WebXRSpace
subclasses that are (indirectly) owned by the WebXRSession.

Change the accessor to return a pointer, and all the subclasses
to use weak pointers instead.

  • Modules/webxr/WebXRBoundedReferenceSpace.h: Use a WeakPtr and change

the constructor parameter to take a reference instead of a Ref<>.

  • Modules/webxr/WebXRBoundedReferenceSpace.cpp:

(WebCore::WebXRBoundedReferenceSpace::create):
(WebCore::WebXRBoundedReferenceSpace::WebXRBoundedReferenceSpace):
(WebCore::WebXRBoundedReferenceSpace::getOffsetReferenceSpace):
(WebCore::WebXRBoundedReferenceSpace::updateIfNeeded): Add early return.

  • Modules/webxr/WebXRFrame.cpp:

(WebCore::WebXRFrame::populatePose): Compare pointers directly.

  • Modules/webxr/WebXRInputSpace.h: Use WeakPtr.
  • Modules/webxr/WebXRInputSpace.cpp:

(WebCore::WebXRInputSpace::WebXRInputSpace):
(WebCore::WebXRInputSpace::nativeOrigin const): Change this to return an
optional so we can detect errors and raise Exceptions.

  • Modules/webxr/WebXRReferenceSpace.h: Use a WeakPtr and change

the constructor parameter to take a reference instead of a Ref<>.

  • Modules/webxr/WebXRReferenceSpace.cpp:

(WebCore::WebXRReferenceSpace::create):
(WebCore::WebXRReferenceSpace::WebXRReferenceSpace):
(WebCore::WebXRReferenceSpace::nativeOrigin const): Return an optional.
(WebCore::WebXRReferenceSpace::getOffsetReferenceSpace):
(WebCore::WebXRReferenceSpace::floorOriginTransform const): Return an optional.

  • Modules/webxr/WebXRSpace.cpp: Return a * instead of a &.

(WebCore::WebXRSpace::isPositionEmulated const):
(WebCore::WebXRViewerSpace::WebXRViewerSpace):
(WebCore::WebXRViewerSpace::nativeOrigin const):

  • Modules/webxr/WebXRSpace.h:
Location:
trunk/Source/WebCore
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r283879 r283880  
     12021-10-09  Dean Jackson  <dino@apple.com>
     2
     3        [WebXR] Replace the session reference in WebXRSpace subclasses with weak pointers
     4        https://bugs.webkit.org/show_bug.cgi?id=231482
     5
     6        Reviewed by Sam Weinig.
     7
     8        WebXRSpace had a pure virtual session() accessor that returned a
     9        reference to a WebXRSession. This made subclasses hold strong
     10        references to the WebXRSession, and is problematic for WebXRSpace
     11        subclasses that are (indirectly) owned by the WebXRSession.
     12
     13        Change the accessor to return a pointer, and all the subclasses
     14        to use weak pointers instead.
     15
     16        * Modules/webxr/WebXRBoundedReferenceSpace.h: Use a WeakPtr and change
     17        the constructor parameter to take a reference instead of a Ref<>.
     18        * Modules/webxr/WebXRBoundedReferenceSpace.cpp:
     19        (WebCore::WebXRBoundedReferenceSpace::create):
     20        (WebCore::WebXRBoundedReferenceSpace::WebXRBoundedReferenceSpace):
     21        (WebCore::WebXRBoundedReferenceSpace::getOffsetReferenceSpace):
     22        (WebCore::WebXRBoundedReferenceSpace::updateIfNeeded): Add early return.
     23
     24        * Modules/webxr/WebXRFrame.cpp:
     25        (WebCore::WebXRFrame::populatePose): Compare pointers directly.
     26
     27        * Modules/webxr/WebXRInputSpace.h: Use WeakPtr.
     28        * Modules/webxr/WebXRInputSpace.cpp:
     29        (WebCore::WebXRInputSpace::WebXRInputSpace):
     30        (WebCore::WebXRInputSpace::nativeOrigin const): Change this to return an
     31        optional so we can detect errors and raise Exceptions.
     32
     33        * Modules/webxr/WebXRReferenceSpace.h: Use a WeakPtr and change
     34        the constructor parameter to take a reference instead of a Ref<>.
     35        * Modules/webxr/WebXRReferenceSpace.cpp:
     36        (WebCore::WebXRReferenceSpace::create):
     37        (WebCore::WebXRReferenceSpace::WebXRReferenceSpace):
     38        (WebCore::WebXRReferenceSpace::nativeOrigin const): Return an optional.
     39        (WebCore::WebXRReferenceSpace::getOffsetReferenceSpace):
     40        (WebCore::WebXRReferenceSpace::floorOriginTransform const): Return an optional.
     41
     42        * Modules/webxr/WebXRSpace.cpp: Return a * instead of a &.
     43        (WebCore::WebXRSpace::isPositionEmulated const):
     44        (WebCore::WebXRViewerSpace::WebXRViewerSpace):
     45        (WebCore::WebXRViewerSpace::nativeOrigin const):
     46        * Modules/webxr/WebXRSpace.h:
     47
    1482021-10-10  Alan Bujtas  <zalan@apple.com>
    249

  • trunk/Source/WebCore/Modules/webxr/WebXRBoundedReferenceSpace.cpp

    r278562 r283880  
    4545WTF_MAKE_ISO_ALLOCATED_IMPL(WebXRBoundedReferenceSpace);
    4646
    47 Ref<WebXRBoundedReferenceSpace> WebXRBoundedReferenceSpace::create(Document& document, Ref<WebXRSession>&& session, XRReferenceSpaceType type)
     47Ref<WebXRBoundedReferenceSpace> WebXRBoundedReferenceSpace::create(Document& document, WebXRSession& session, XRReferenceSpaceType type)
    4848{
    49     return adoptRef(*new WebXRBoundedReferenceSpace(document, WTFMove(session), WebXRRigidTransform::create(), type));
     49    return adoptRef(*new WebXRBoundedReferenceSpace(document, session, WebXRRigidTransform::create(), type));
    5050}
    5151
    52 Ref<WebXRBoundedReferenceSpace> WebXRBoundedReferenceSpace::create(Document& document, Ref<WebXRSession>&& session, Ref<WebXRRigidTransform>&& offset, XRReferenceSpaceType type)
     52Ref<WebXRBoundedReferenceSpace> WebXRBoundedReferenceSpace::create(Document& document, WebXRSession& session, Ref<WebXRRigidTransform>&& offset, XRReferenceSpaceType type)
    5353{
    54     return adoptRef(*new WebXRBoundedReferenceSpace(document, WTFMove(session), WTFMove(offset), type));
     54    return adoptRef(*new WebXRBoundedReferenceSpace(document, session, WTFMove(offset), type));
    5555}
    5656
    57 WebXRBoundedReferenceSpace::WebXRBoundedReferenceSpace(Document& document, Ref<WebXRSession>&& session, Ref<WebXRRigidTransform>&& offset, XRReferenceSpaceType type)
    58     : WebXRReferenceSpace(document, WTFMove(session), WTFMove(offset), type)
     57WebXRBoundedReferenceSpace::WebXRBoundedReferenceSpace(Document& document, WebXRSession& session, Ref<WebXRRigidTransform>&& offset, XRReferenceSpaceType type)
     58    : WebXRReferenceSpace(document, session, WTFMove(offset), type)
    5959{
    6060}
    … …  
    6262WebXRBoundedReferenceSpace::~WebXRBoundedReferenceSpace() = default;
    6363
    64 TransformationMatrix WebXRBoundedReferenceSpace::nativeOrigin() const
     64std::optional<TransformationMatrix> WebXRBoundedReferenceSpace::nativeOrigin() const
    6565{
    6666    // https://immersive-web.github.io/webxr/#dom-xrreferencespacetype-bounded-floor.
    … …  
    7777ExceptionOr<Ref<WebXRReferenceSpace>> WebXRBoundedReferenceSpace::getOffsetReferenceSpace(const WebXRRigidTransform& offsetTransform)
    7878{
     79    if (!m_session)
     80        return Exception { InvalidStateError };
     81
    7982    auto* document = downcast<Document>(scriptExecutionContext());
    8083    if (!document)
    … …  
    8588    auto offset = WebXRRigidTransform::create(originOffset().rawTransform() * offsetTransform.rawTransform());
    8689
    87     return { create(*document, m_session.copyRef(), WTFMove(offset), m_type) };
     90    return { create(*document, *m_session.get(), WTFMove(offset), m_type) };
    8891}
    8992
    … …  
    9194void WebXRBoundedReferenceSpace::updateIfNeeded()
    9295{
     96    if (!m_session)
     97        return;
     98
    9399    auto& frameData = m_session->frameData();
    94100    if (frameData.stageParameters.id == m_lastUpdateId)

  • trunk/Source/WebCore/Modules/webxr/WebXRBoundedReferenceSpace.h

    r277251 r283880  
    4040    WTF_MAKE_ISO_ALLOCATED(WebXRBoundedReferenceSpace);
    4141public:
    42     static Ref<WebXRBoundedReferenceSpace> create(Document&, Ref<WebXRSession>&&, XRReferenceSpaceType);
    43     static Ref<WebXRBoundedReferenceSpace> create(Document&, Ref<WebXRSession>&&, Ref<WebXRRigidTransform>&&, XRReferenceSpaceType);
     42    static Ref<WebXRBoundedReferenceSpace> create(Document&, WebXRSession&, XRReferenceSpaceType);
     43    static Ref<WebXRBoundedReferenceSpace> create(Document&, WebXRSession&, Ref<WebXRRigidTransform>&&, XRReferenceSpaceType);
    4444
    4545    virtual ~WebXRBoundedReferenceSpace();
    4646
    47     TransformationMatrix nativeOrigin() const final;
     47    std::optional<TransformationMatrix> nativeOrigin() const final;
    4848    const Vector<Ref<DOMPointReadOnly>>& boundsGeometry();
    4949    ExceptionOr<Ref<WebXRReferenceSpace>> getOffsetReferenceSpace(const WebXRRigidTransform&) final;
    5050
    5151private:
    52     WebXRBoundedReferenceSpace(Document&, Ref<WebXRSession>&&, Ref<WebXRRigidTransform>&&, XRReferenceSpaceType);
     52    WebXRBoundedReferenceSpace(Document&, WebXRSession&, Ref<WebXRRigidTransform>&&, XRReferenceSpaceType);
    5353
    5454    bool isBoundedReferenceSpace() const final { return true; }

  • trunk/Source/WebCore/Modules/webxr/WebXRFrame.cpp

    r282755 r283880  
    105105    // 2. Let session be frame’s session object.
    106106    // 3. If space’s session does not equal session, throw an InvalidStateError and abort these steps.
    107     if (&space.session() != m_session.ptr())
     107    if (space.session() != m_session.ptr())
    108108        return Exception { InvalidStateError };
    109109
    110110    // 4. If baseSpace’s session does not equal session, throw an InvalidStateError and abort these steps.
    111     if (&baseSpace.session() != m_session.ptr())
     111    if (baseSpace.session() != m_session.ptr())
    112112        return Exception { InvalidStateError };
    113113
    … …  
    128128
    129129    auto baseTransform = baseSpace.effectiveOrigin();
    130     if (!baseTransform.isInvertible())
     130    if (!baseTransform)
     131        return Exception { InvalidStateError };
     132
     133    if (!baseTransform.value().isInvertible())
    131134        return { std::nullopt };
    132135
    133     auto transform =  *baseTransform.inverse() * space.effectiveOrigin();
    134     bool emulatedPosition = space.isPositionEmulated() || baseSpace.isPositionEmulated();
     136    auto effectiveOrigin = space.effectiveOrigin();
     137    if (!effectiveOrigin)
     138        return Exception { InvalidStateError };
     139
     140    auto transform =  *baseTransform.value().inverse() * effectiveOrigin.value();
     141
     142    auto isPositionEmulated = space.isPositionEmulated();
     143    if (!isPositionEmulated)
     144        return Exception { InvalidStateError };
     145
     146    auto baseSpaceIsPositionEmulated = baseSpace.isPositionEmulated();
     147    if (!baseSpaceIsPositionEmulated)
     148        return Exception { InvalidStateError };
     149
     150    bool emulatedPosition = isPositionEmulated.value() || baseSpaceIsPositionEmulated.value();
    135151
    136152    bool limit = mustPosesBeLimited(space, baseSpace);

  • trunk/Source/WebCore/Modules/webxr/WebXRInputSpace.cpp

    r277251 r283880  
    4747WebXRInputSpace::WebXRInputSpace(Document& document, WebXRSession& session, const PlatformXR::Device::FrameData::InputSourcePose& pose)
    4848    : WebXRSpace(document, WebXRRigidTransform::create())
    49     , m_session(session)
     49    , m_session(makeWeakPtr(&session))
    5050    , m_pose(pose)
    5151{
    … …  
    5454WebXRInputSpace::~WebXRInputSpace() = default;
    5555
    56 TransformationMatrix WebXRInputSpace::nativeOrigin() const
     56std::optional<TransformationMatrix> WebXRInputSpace::nativeOrigin() const
    5757{
    5858    return WebXRFrame::matrixFromPose(m_pose.pose);

  • trunk/Source/WebCore/Modules/webxr/WebXRInputSpace.h

    r277251 r283880  
    4545    using RefCounted<WebXRInputSpace>::deref;
    4646
    47     bool isPositionEmulated() const final { return m_pose.isPositionEmulated; }
     47    std::optional<bool> isPositionEmulated() const final { return m_pose.isPositionEmulated; }
    4848    void setPose(const PlatformXR::Device::FrameData::InputSourcePose& pose) { m_pose = pose; }
    4949
    5050private:
    5151    WebXRInputSpace(Document&, WebXRSession&, const PlatformXR::Device::FrameData::InputSourcePose&);
    52     WebXRSession& session() const final { return m_session.get(); }
    53     TransformationMatrix nativeOrigin() const final;
     52    WebXRSession* session() const final { return m_session.get(); }
     53    std::optional<TransformationMatrix> nativeOrigin() const final;
    5454
    5555    void refEventTarget() final { ref(); }
    5656    void derefEventTarget() final { deref(); }
    5757
    58     Ref<WebXRSession> m_session;
     58    WeakPtr<WebXRSession> m_session;
    5959    PlatformXR::Device::FrameData::InputSourcePose m_pose;
    6060};

  • trunk/Source/WebCore/Modules/webxr/WebXRReferenceSpace.cpp

    r273957 r283880  
    4141WTF_MAKE_ISO_ALLOCATED_IMPL(WebXRReferenceSpace);
    4242
    43 Ref<WebXRReferenceSpace> WebXRReferenceSpace::create(Document& document, Ref<WebXRSession>&& session, XRReferenceSpaceType type)
     43Ref<WebXRReferenceSpace> WebXRReferenceSpace::create(Document& document, WebXRSession& session, XRReferenceSpaceType type)
    4444{
    4545    // https://immersive-web.github.io/webxr/#xrspace-native-origin
    … …  
    4747    // defined by an origin offset, which is an XRRigidTransform initially set
    4848    // to an identity transform.
    49     return adoptRef(*new WebXRReferenceSpace(document, WTFMove(session), WebXRRigidTransform::create(), type));
     49    return adoptRef(*new WebXRReferenceSpace(document, session, WebXRRigidTransform::create(), type));
    5050}
    5151
    52 Ref<WebXRReferenceSpace> WebXRReferenceSpace::create(Document& document, Ref<WebXRSession>&& session, Ref<WebXRRigidTransform>&& offset, XRReferenceSpaceType type)
     52Ref<WebXRReferenceSpace> WebXRReferenceSpace::create(Document& document, WebXRSession& session, Ref<WebXRRigidTransform>&& offset, XRReferenceSpaceType type)
    5353{
    54     return adoptRef(*new WebXRReferenceSpace(document, WTFMove(session), WTFMove(offset), type));
     54    return adoptRef(*new WebXRReferenceSpace(document, session, WTFMove(offset), type));
    5555}
    5656
    57 WebXRReferenceSpace::WebXRReferenceSpace(Document& document, Ref<WebXRSession>&& session, Ref<WebXRRigidTransform>&& offset, XRReferenceSpaceType type)
     57WebXRReferenceSpace::WebXRReferenceSpace(Document& document, WebXRSession& session, Ref<WebXRRigidTransform>&& offset, XRReferenceSpaceType type)
    5858    : WebXRSpace(document, WTFMove(offset))
    59     , m_session(WTFMove(session))
     59    , m_session(makeWeakPtr(session))
    6060    , m_type(type)
    6161{
    … …  
    6565
    6666
    67 TransformationMatrix WebXRReferenceSpace::nativeOrigin() const
     67std::optional<TransformationMatrix> WebXRReferenceSpace::nativeOrigin() const
    6868{
     69    if (!m_session)
     70        return std::nullopt;
     71
    6972    TransformationMatrix identity;
    7073
    … …  
    9194        // BoundedFloor is handled by WebXRBoundedReferenceSpace subclass
    9295        RELEASE_ASSERT_NOT_REACHED();
     96        return std::nullopt;
    9397    }
    94 
    95     return identity;
    9698}
    9799
    98100ExceptionOr<Ref<WebXRReferenceSpace>> WebXRReferenceSpace::getOffsetReferenceSpace(const WebXRRigidTransform& offsetTransform)
    99101{
     102    if (!m_session)
     103        return Exception { InvalidStateError };
     104
    100105    auto* document = downcast<Document>(scriptExecutionContext());
    101106    if (!document)
    … …  
    106111    auto offset = WebXRRigidTransform::create(originOffset().rawTransform() * offsetTransform.rawTransform());
    107112
    108     return create(*document, m_session.copyRef(), WTFMove(offset), m_type);
     113    return create(*document, *m_session.get(), WTFMove(offset), m_type);
    109114}
    110115
    111 TransformationMatrix WebXRReferenceSpace::floorOriginTransform() const
     116std::optional<TransformationMatrix> WebXRReferenceSpace::floorOriginTransform() const
    112117{
     118    if (!m_session)
     119        return std::nullopt;
     120
    113121    auto& data = m_session->frameData();
    114122    if (!data.floorTransform) {

  • trunk/Source/WebCore/Modules/webxr/WebXRReferenceSpace.h

    r273957 r283880  
    3232#include <wtf/IsoMalloc.h>
    3333#include <wtf/Ref.h>
     34#include <wtf/WeakPtr.h>
    3435
    3536namespace WebCore {
    … …  
    4142    WTF_MAKE_ISO_ALLOCATED(WebXRReferenceSpace);
    4243public:
    43     static Ref<WebXRReferenceSpace> create(Document&, Ref<WebXRSession>&&, XRReferenceSpaceType);
    44     static Ref<WebXRReferenceSpace> create(Document&, Ref<WebXRSession>&&, Ref<WebXRRigidTransform>&&, XRReferenceSpaceType);
     44    static Ref<WebXRReferenceSpace> create(Document&, WebXRSession&, XRReferenceSpaceType);
     45    static Ref<WebXRReferenceSpace> create(Document&, WebXRSession&, Ref<WebXRRigidTransform>&&, XRReferenceSpaceType);
    4546
    4647    virtual ~WebXRReferenceSpace();
    … …  
    4950    using RefCounted<WebXRReferenceSpace>::deref;
    5051
    51     WebXRSession& session() const final { return m_session.get(); }
    52     TransformationMatrix nativeOrigin() const override;
     52    WebXRSession* session() const final { return m_session.get(); }
     53    std::optional<TransformationMatrix> nativeOrigin() const override;
    5354    virtual ExceptionOr<Ref<WebXRReferenceSpace>> getOffsetReferenceSpace(const WebXRRigidTransform&);
    5455    XRReferenceSpaceType type() const { return m_type; }
    5556
    5657protected:
    57     WebXRReferenceSpace(Document&, Ref<WebXRSession>&&, Ref<WebXRRigidTransform>&&, XRReferenceSpaceType);
     58    WebXRReferenceSpace(Document&, WebXRSession&, Ref<WebXRRigidTransform>&&, XRReferenceSpaceType);
    5859
    5960    bool isReferenceSpace() const final { return true; }
    6061
    61     TransformationMatrix floorOriginTransform() const;
     62    std::optional<TransformationMatrix> floorOriginTransform() const;
    6263
    63     Ref<WebXRSession> m_session;
     64    WeakPtr<WebXRSession> m_session;
    6465    XRReferenceSpaceType m_type;
    6566

  • trunk/Source/WebCore/Modules/webxr/WebXRSpace.cpp

    r278563 r283880  
    4747WebXRSpace::~WebXRSpace() = default;
    4848
    49 TransformationMatrix WebXRSpace::effectiveOrigin() const
     49std::optional<TransformationMatrix> WebXRSpace::effectiveOrigin() const
    5050{
    5151    // https://immersive-web.github.io/webxr/#xrspace-effective-origin
    5252    // The effective origin can be obtained by multiplying origin offset and the native origin.
    53     return nativeOrigin() * m_originOffset->rawTransform();
     53    auto origin = nativeOrigin();
     54    if (!origin)
     55        return std::nullopt;
     56    return origin.value() * m_originOffset->rawTransform();
    5457}
    5558
    5659
    57 bool WebXRSpace::isPositionEmulated() const
     60std::optional<bool> WebXRSpace::isPositionEmulated() const
    5861{
    59     return session().isPositionEmulated();
     62    WebXRSession* xrSession = session();
     63    if (!xrSession)
     64        return std::nullopt;
     65
     66    return xrSession->isPositionEmulated();
    6067}
    6168
    … …  
    6471WebXRViewerSpace::WebXRViewerSpace(Document& document, WebXRSession& session)
    6572    : WebXRSpace(document, WebXRRigidTransform::create())
    66     , m_session(session)
     73    , m_session(makeWeakPtr(&session))
    6774{
    6875}
    … …  
    7077WebXRViewerSpace::~WebXRViewerSpace() = default;
    7178
    72 TransformationMatrix WebXRViewerSpace::nativeOrigin() const
     79std::optional<TransformationMatrix> WebXRViewerSpace::nativeOrigin() const
    7380{
    74     return WebXRFrame::matrixFromPose(m_session.frameData().origin);
     81    if (!m_session)
     82        return std::nullopt;
     83    return WebXRFrame::matrixFromPose(m_session->frameData().origin);
    7584}
    7685

  • trunk/Source/WebCore/Modules/webxr/WebXRSpace.h

    r276433 r283880  
    3131#include "EventTarget.h"
    3232#include "TransformationMatrix.h"
     33#include "WebXRSession.h"
    3334#include <wtf/RefCounted.h>
    3435
    … …  
    3839class ScriptExecutionContext;
    3940class WebXRRigidTransform;
    40 class WebXRSession;
    4141
    4242class WebXRSpace : public EventTargetWithInlineData, public ContextDestructionObserver {
    … …  
    4545    virtual ~WebXRSpace();
    4646
    47     virtual WebXRSession& session() const = 0;
    48     virtual TransformationMatrix nativeOrigin() const = 0;
    49     TransformationMatrix effectiveOrigin() const;
    50     virtual bool isPositionEmulated() const;
     47    virtual WebXRSession* session() const = 0;
     48    virtual std::optional<TransformationMatrix> nativeOrigin() const = 0;
     49    std::optional<TransformationMatrix> effectiveOrigin() const;
     50    virtual std::optional<bool> isPositionEmulated() const;
    5151
    5252    virtual bool isReferenceSpace() const { return false; }
    … …  
    7878
    7979private:
    80     WebXRSession& session() const final { return m_session; }
    81     TransformationMatrix nativeOrigin() const final;
     80    WebXRSession* session() const final { return m_session.get(); }
     81    std::optional<TransformationMatrix> nativeOrigin() const final;
    8282
    8383    void refEventTarget() final { RELEASE_ASSERT_NOT_REACHED(); }
    8484    void derefEventTarget() final { RELEASE_ASSERT_NOT_REACHED(); }
    8585
    86     WebXRSession& m_session;
     86    WeakPtr<WebXRSession> m_session;
    8787};
    8888
Note: See TracChangeset for help on using the changeset viewer.