New issue
Advanced search Search tips

Issue 829534 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug
Proj-XR



Sign in to add a comment

Clicking on permissions infobar in WebXR freezes presentation and controller

Project Member Reported by dbbrooks@chromium.org, Apr 5 2018

Issue description

Chrome: 67.0.3387.0
VRCore: 1.13.185188193
Android N
Device: S8+

What steps will reproduce the problem?
(1) Go to Apps > { Chrome | Chrome Beta | Chrome Canary | Chrome Dev | Chromium } > Storage > MANAGE SPACE > CLEAR ALL DATA
(2) Apps > { Chrome | Chrome Beta | Chrome Canary | Chrome Dev | Chromium } >  Permissions and enable Microphone permission.
(3) Complete First Run Experience, then go to https://immersive-web.github.io/webxr-samples/tests/permission-request.html and click Enter VR button
(4) Put on headset and follow DON flow.
(5) Click on the Microphone icon.
(6) An infobar appears asking for permission to use your microphone. Click either button "allow" or "block".

What is the expected result? Infobar is dismissed after clicking button.

What happens instead? The controller is frozen and the button state is frozen.
 
Labels: -Restrict-View-Google
Labels: M-67 Hotlist-VRB-MVP Pri-2
Owner: vollick@chromium.org
Status: Assigned (was: Untriaged)
Owner: ymalik@chromium.org
Status: Started (was: Assigned)
Owner: klausw@chromium.org
Status: Assigned (was: Started)
Looks like this happens on Pixel as well, and is not specific to Samsung.

Seems like a regression, I did a manual bisect, has something to do with the recent changed to vr_shell_gl. Reverting this seems to make it work: https://chromium-review.googlesource.com/c/chromium/src/+/987301

Klaus, can you take a look? Here's my patch that added this: https://chromium-review.googlesource.com/c/chromium/src/+/982605

We basically have "ShouldRenderWebVR" return false while we're showing the permission prompt so we don't send vsync to the WebVR page and avoid drawing webvr frames.
Status: Started (was: Assigned)
Looking. This is likely related to the lifecycle refactor I'm working on, sorry about that.

Comment 8 by ericde@google.com, Apr 6 2018

Labels: -Pri-2 Pri-1 Type-Bug
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dbff879cf3e9f62d055433c8f664003a58c99aec

commit dbff879cf3e9f62d055433c8f664003a58c99aec
Author: Klaus Weidner <klausw@chromium.org>
Date: Fri Apr 06 21:18:52 2018

Fix frame lifecycle for UI<->WebVR handoff

Permission prompts temporarily take over the UI during WebVR presentation, but
incomplete WebVR frames need to cleanly finish their lifecycle. In previous
versions this worked by letting WebVR run parts of the drawing code while UI was
active, but this is rather hard to follow. crrev.com/c/987301 broke this due to
making incorrect assumptions about the lifecycle.

Add explicit lifecycle transitions to cleanly cancel unwanted WebVR frames at
safe times in their lifecycle. This includes sending notifications to the
Renderer to ensure its state also gets updated appropriately.

Bug:  829534 
Change-Id: I7e2fe9ebf1f9187f4dc6bc76857c41cc4aa655cd
Reviewed-on: https://chromium-review.googlesource.com/1000133
Reviewed-by: Yash Malik <ymalik@chromium.org>
Commit-Queue: Klaus Weidner <klausw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548929}
[modify] https://crrev.com/dbff879cf3e9f62d055433c8f664003a58c99aec/chrome/browser/android/vr/vr_shell_gl.cc
[modify] https://crrev.com/dbff879cf3e9f62d055433c8f664003a58c99aec/chrome/browser/android/vr/vr_shell_gl.h

Status: Fixed (was: Started)
The change from #9 fixed this in my manual testing. Please verify once it's in Canary.

FWIW, I found an additional corner case where exiting and re-entering a presentation session very rapidly could potentially still confuse state due to accidentally processing an old frame's OnWebVRFrameAvailable as part of the next session. I have a fix for that pending in https://crrev.com/c/989613 , though I think that this variation is unlikely to occur in practice.
Labels: Test-Complete
Verified on build 67.0.3396.29 beta.  Looks good.
Status: Verified (was: Fixed)
Components: Blink>WebXR

Sign in to add a comment