[WebXR] Add state checking to WebXR IPC calls #23536
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
EWS run on previous version of this PR (hash 2ff5f4f) |
webkit-ews-buildbot
added
the
merging-blocked
achan00
removed
the
merging-blocked
achan00
force-pushed
the
eng/WebXR-Add-state-checking-to-WebXR-IPC-calls
branch
from
2ff5f4f
to
1e5485d
Compare
|
EWS run on previous version of this PR (hash 1e5485d)
|
achan00
force-pushed
the
eng/WebXR-Add-state-checking-to-WebXR-IPC-calls
branch
from
1e5485d
to
c61ea0a
Compare
|
EWS run on current version of this PR (hash c61ea0a) |
🛠 mac
|
EWS run on previous version of this PR (hash c61ea0a)
|
djg
approved these changes
Jan 31, 2024
There was a problem hiding this comment.
I'm happy to accept this now, but in a future patch, I'd like to refactor the state management to use std::variant for each state to eliminate the need to check for valid data keep in std::optional.
This could be done in an effort to unify the WebXR ARKit system with Compositor.
| @@ -146,6 +211,22 @@ void PlatformXRSystem::sessionDidUpdateVisibilityState(XRDeviceIdentifier device | |||
| }); | |||
| } | |||
|
|
|||
| void PlatformXRSystem::setImmersiveSessionState(ImmersiveSessionState state) | |||
| { | |||
| RELEASE_LOG_DEBUG(XR, "PlatformXRSystem: Immersive state update: Old=%d, New=%d", (int)m_immersiveSessionState, (int)state); | |||
There was a problem hiding this comment.
Do we want to ship with this logging? (Dean always asked me this question)
achan00
force-pushed
the
eng/WebXR-Add-state-checking-to-WebXR-IPC-calls
branch
from
c61ea0a
to
f43336e
Compare
|
EWS run on current version of this PR (hash f43336e)
|
https://bugs.webkit.org/show_bug.cgi?id=268404 rdar://121553978 Reviewed by Dan Glastonbury. Track internal state in PlatformXRSystem and check that we are in the expected state when the IPC call is made. Remove unnecessary parameters in PlatformXRSystem::initializeTrackingAndRendering() as that information can be cached in this class and there's no need to pass that info over again. * Source/WebCore/Modules/webxr/WebXRSystem.cpp: (WebCore::WebXRSystem::resolveFeaturePermissions const): We should not call into platform code for requesting permissions for simulated test devices. * Source/WebKit/Shared/XR/XRDeviceProxy.cpp: (WebKit::XRDeviceProxy::initializeTrackingAndRendering): * Source/WebKit/UIProcess/XR/PlatformXRSystem.cpp: (WebKit::PlatformXRSystem::invalidate): (WebKit::PlatformXRSystem::ensureImmersiveSessionActivity): (WebKit::checkFeaturesConsent): Helper function to check whether all the features in the first argument are included in the second list of granted features. (WebKit::PlatformXRSystem::requestPermissionOnSessionFeatures): For immersive modes, check that we are in the idle state before requesting permissions via the PlatformXRCoordinator. When we get the granted permissions back, check whether all the required features have been granted permission. Update the immersive session state accordingly based on whether the session can start. (WebKit::PlatformXRSystem::initializeTrackingAndRendering): Remove the parameters and use the cached immersive session mode and granted features instead. (WebKit::PlatformXRSystem::shutDownTrackingAndRendering): (WebKit::PlatformXRSystem::requestFrame): (WebKit::PlatformXRSystem::submitFrame): (WebKit::PlatformXRSystem::sessionDidEnd): (WebKit::PlatformXRSystem::setImmersiveSessionState): (WebKit::PlatformXRSystem::invalidateImmersiveSessionState): * Source/WebKit/UIProcess/XR/PlatformXRSystem.h: * Source/WebKit/UIProcess/XR/PlatformXRSystem.messages.in: * Source/WebKit/WebProcess/XR/PlatformXRSystemProxy.cpp: (WebKit::PlatformXRSystemProxy::initializeTrackingAndRendering): * Source/WebKit/WebProcess/XR/PlatformXRSystemProxy.h: Canonical link: https://commits.webkit.org/273875@main
webkit-commit-queue
force-pushed
the
eng/WebXR-Add-state-checking-to-WebXR-IPC-calls
branch
from
f43336e
to
bd55010
Compare
|
Committed 273875@main (bd55010): https://commits.webkit.org/273875@main Reviewed commits have been landed. Closing PR #23536 and removing active labels. |
webkit-commit-queue
removed
the
merge-queue
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
bd55010
f43336e
🛠 wincairo🧪 wpe-wk2🧪 ios-wk2🧪 api-mac🧪 api-wpe🧪 ios-wk2-wpt🧪 mac-wk1🧪 mac-wk2🧪 gtk-wk2🧪 mac-AS-debug-wk2🧪 api-gtk🛠 tv-sim🛠 watch🛠 watch-sim