[webkit-reviews] review granted: [Bug 212470] [WebXR] Refactor OpenXR platform code : [Attachment 400468] Patch

[bugzilla-daemon at webkit.org]

Zan Dobersek <zan at falconsigh.net> has granted Sergio Villar Senin
<svillar at igalia.com>'s request for review:
Bug 212470: [WebXR] Refactor OpenXR platform code
https://bugs.webkit.org/show_bug.cgi?id=212470

Attachment 400468: Patch

https://bugs.webkit.org/attachment.cgi?id=400468&action=review




--- Comment #3 from Zan Dobersek <zan at falconsigh.net> ---
Comment on attachment 400468
  --> https://bugs.webkit.org/attachment.cgi?id=400468
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=400468&action=review

> Source/WebCore/Modules/webxr/WebXRSystem.cpp:60
> +    return this == &other ? true : false;

Should be able to avoid the ternary operator.

> Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp:83
> +	   RETURN_IF_FAILED(result, "xrEnumerateApiLayerProperties()",
m_instance);

How is m_instance supposed to be used here when it's not yet initialized? It's
passed to xrResultToString(), but the docs for that entrypoint suggest it must
be a valid handle.

> Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp:149
> +Instance::Impl::~Impl()
> +{
> +}

Can be `= default;`-ed, unless it should destroy the XrInstance object.

> Source/WebCore/testing/WebFakeXRDevice.h:73
> +    bool operator==(const Device& other) const final { return this == &other
? true : false; }

As before, the ?: operator can be omitted.