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

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> 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 401135: Patch

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




--- Comment #14 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 401135
  --> https://bugs.webkit.org/attachment.cgi?id=401135
Patch

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

> Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp:95
> +    }();

Why do we need these lambdas? It seems its purpose is to not do an if/else.
Can we remove them, maybe by using functions with a name that will tell what it
does?

> Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp:157
> +    XrResult result = xrEnumerateViewConfigurations(m_instance,
device.xrSystemId(), 0, &viewConfigurationCount, nullptr);

auto

> Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp:167
> +	   WTFLogAlways("xrEnumerateViewConfigurations(): error %s\n",
resultToString(result, m_instance).utf8().data());

No need for these two WTFLogAlways.
Maybe we should use RELEASE_LOG(XR, ...) as well instead.

> Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp:182
> +	       supportedModes.append(SessionMode::ImmersiveVr);

Should we use a switch here?