New issue
Advanced search Search tips

Issue 767896 link

Starred by 2 users
Status: Fixed
Owner:
mthiesse@chromium.org
Closed: Oct 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Proj-XR
Proj-XR-VR



Sign in to add a comment

VR: Hitting app button in webVR triggers non-optional DOFF with VrShell disabled.

Project Member Reported by mthiesse@chromium.org, Sep 22 2017 
When VrShell (Browsing) is disabled, and you press the app button, you are asked to take your headset off, and if you hit back, you're asked to take it off again.

We should either disable the app button when VrShell is disabled (and exiting WebVR means removing your headset), or we should make the DOFF optional and stay in VR when you back out of the DOFF. Preferably the former.

Comment 1 by vollick@chromium.org, Sep 22 2017 

By disabled, do you mean cardboard mode.

Comment 2 by mthiesse@chromium.org, Sep 25 2017 

No, I mean the VrShell flag is disabled, or we've intentionally disabled it like on the S8 when the resolution is wrong.

Comment 3 by mthiesse@chromium.org, Oct 13 2017

Owner: mthiesse@chromium.org
Status: Started (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 16 2017 

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

commit a0cdfa5b9ead84293e06e35a6076aa6a65e77d2a
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Mon Oct 16 16:23:38 2017

VR: Disable app button when browsing mode is disabled.

Pressing the app button would try to exit WebVR, which would trigger
a non-optional DOFF, which is a really bad user experience. The button
should just be disabled when it can't actually take you back to the
VR browser.

Bug:  767896 
Change-Id: If31a7e8959293efb48618ec40a3cd9e132e92961
Reviewed-on: https://chromium-review.googlesource.com/718986
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509072}
[modify] https://crrev.com/a0cdfa5b9ead84293e06e35a6076aa6a65e77d2a/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
[modify] https://crrev.com/a0cdfa5b9ead84293e06e35a6076aa6a65e77d2a/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
[modify] https://crrev.com/a0cdfa5b9ead84293e06e35a6076aa6a65e77d2a/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/WebVrInputTest.java
[modify] https://crrev.com/a0cdfa5b9ead84293e06e35a6076aa6a65e77d2a/chrome/browser/android/vr_shell/vr_gl_thread.cc
[modify] https://crrev.com/a0cdfa5b9ead84293e06e35a6076aa6a65e77d2a/chrome/browser/android/vr_shell/vr_gl_thread.h
[modify] https://crrev.com/a0cdfa5b9ead84293e06e35a6076aa6a65e77d2a/chrome/browser/android/vr_shell/vr_shell.cc
[modify] https://crrev.com/a0cdfa5b9ead84293e06e35a6076aa6a65e77d2a/chrome/browser/android/vr_shell/vr_shell.h
[modify] https://crrev.com/a0cdfa5b9ead84293e06e35a6076aa6a65e77d2a/chrome/browser/vr/ui.h
[modify] https://crrev.com/a0cdfa5b9ead84293e06e35a6076aa6a65e77d2a/chrome/browser/vr/ui_scene_manager.cc
[modify] https://crrev.com/a0cdfa5b9ead84293e06e35a6076aa6a65e77d2a/chrome/browser/vr/ui_scene_manager.h

Comment 5 by mthiesse@chromium.org, Oct 17 2017

Labels: Merge-Request-63
Project Member

Comment 6 by sheriffbot@chromium.org, Oct 18 2017

Labels: -Merge-Request-63 Hotlist-Merge-Approved Merge-Approved-63
Your change meets the bar and is auto-approved for M63. Please go ahead and merge the CL to branch 3239 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 18 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b2c1eea023c2275d1544ee31b2476337f6bc64fe

commit b2c1eea023c2275d1544ee31b2476337f6bc64fe
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Wed Oct 18 18:26:24 2017

VR: Disable app button when browsing mode is disabled.

Pressing the app button would try to exit WebVR, which would trigger
a non-optional DOFF, which is a really bad user experience. The button
should just be disabled when it can't actually take you back to the
VR browser.

TBR=mthiesse@chromium.org

(cherry picked from commit a0cdfa5b9ead84293e06e35a6076aa6a65e77d2a)

Bug:  767896 
Change-Id: If31a7e8959293efb48618ec40a3cd9e132e92961
Reviewed-on: https://chromium-review.googlesource.com/718986
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509072}
Reviewed-on: https://chromium-review.googlesource.com/726481
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#54}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/b2c1eea023c2275d1544ee31b2476337f6bc64fe/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
[modify] https://crrev.com/b2c1eea023c2275d1544ee31b2476337f6bc64fe/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
[modify] https://crrev.com/b2c1eea023c2275d1544ee31b2476337f6bc64fe/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/WebVrInputTest.java
[modify] https://crrev.com/b2c1eea023c2275d1544ee31b2476337f6bc64fe/chrome/browser/android/vr_shell/vr_gl_thread.cc
[modify] https://crrev.com/b2c1eea023c2275d1544ee31b2476337f6bc64fe/chrome/browser/android/vr_shell/vr_gl_thread.h
[modify] https://crrev.com/b2c1eea023c2275d1544ee31b2476337f6bc64fe/chrome/browser/android/vr_shell/vr_shell.cc
[modify] https://crrev.com/b2c1eea023c2275d1544ee31b2476337f6bc64fe/chrome/browser/android/vr_shell/vr_shell.h
[modify] https://crrev.com/b2c1eea023c2275d1544ee31b2476337f6bc64fe/chrome/browser/vr/ui.h
[modify] https://crrev.com/b2c1eea023c2275d1544ee31b2476337f6bc64fe/chrome/browser/vr/ui_scene_manager.cc
[modify] https://crrev.com/b2c1eea023c2275d1544ee31b2476337f6bc64fe/chrome/browser/vr/ui_scene_manager.h

Comment 8 by mthiesse@chromium.org, Oct 18 2017

Status: Fixed (was: Started)

Comment 9 by dougman@chromium.org, Feb 7 2018

Labels: Test-Manual
►

Sign in to add a comment