New issue
Advanced search Search tips

Issue 862829 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 16
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Proj-VR
Proj-XR



Sign in to add a comment

VR black overlay stays permanently if VR entered/exited quickly

Project Member Reported by bsheedy@chromium.org, Jul 11

Issue description

If you enter then exit VR really quickly, the black overlay that shows during the transition can get stuck permanently on top of 2D Chrome. I reproed this by adding a forceExitVr call at the beginning of a VrShellNavigationTest test (which automatically enters VR before the test starts), so no idea if this is possible outside of tests.

I tried adding a check at the end of shutdownVr to remove the overlay if it's still visible, but that only partially works. This briefly removes the overlay before it becomes visible again.

This is caused by us launching an intent in VR from here https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java?q=launchinvr&sq=package:chromium&dr=C&l=310, receiving it, and re-applying the overlay here https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java?q=launchinvr&sq=package:chromium&dr=C&l=700
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 16

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

commit 0ecd23528b4e23754f917744f9ea2d2b6460b06e
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Mon Jul 16 18:02:20 2018

VR: Clean up VR intent handling.

We were duplicating code to handle VR entry failing, so I de-duplicated
and refactored some code here.

Might fix  issue 862829 .

TBR=yfriedman@chromium.org

Bug:  862829 
Change-Id: I357d40112f4d9a67e0b5206ff2dc2788e1d5d205
Reviewed-on: https://chromium-review.googlesource.com/1138400
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575342}
[modify] https://crrev.com/0ecd23528b4e23754f917744f9ea2d2b6460b06e/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[modify] https://crrev.com/0ecd23528b4e23754f917744f9ea2d2b6460b06e/chrome/android/java/src/org/chromium/chrome/browser/vr/VrShellDelegate.java

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 16

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

commit 9f8158c12189567f8b9f4bcb44dd3cfc68ecd470
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Mon Jul 16 19:51:00 2018

VR: Ensure we remove black overlay if VR is exited while entering.

This fixes a failure mode where we shutdownVR after handling the VR NFC
broadcast, and fail to properly cancel VR entry.

Also does some other random cleanup of comments/visibility.

Bug:  862829 
Change-Id: Ic382692d7255aa5893391a602a6c949b4b4d498b
Reviewed-on: https://chromium-review.googlesource.com/1138726
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575388}
[modify] https://crrev.com/9f8158c12189567f8b9f4bcb44dd3cfc68ecd470/chrome/android/java/src/org/chromium/chrome/browser/vr/VrAlertDialog.java
[modify] https://crrev.com/9f8158c12189567f8b9f4bcb44dd3cfc68ecd470/chrome/android/java/src/org/chromium/chrome/browser/vr/VrFirstRunActivity.java
[modify] https://crrev.com/9f8158c12189567f8b9f4bcb44dd3cfc68ecd470/chrome/android/java/src/org/chromium/chrome/browser/vr/VrShellDelegate.java
[modify] https://crrev.com/9f8158c12189567f8b9f4bcb44dd3cfc68ecd470/chrome/android/java/src/org/chromium/chrome/browser/vr/VrToast.java
[modify] https://crrev.com/9f8158c12189567f8b9f4bcb44dd3cfc68ecd470/chrome/android/java/src/org/chromium/chrome/browser/vr/VrUiWidgetFactory.java
[modify] https://crrev.com/9f8158c12189567f8b9f4bcb44dd3cfc68ecd470/chrome/android/javatests/src/org/chromium/chrome/browser/vr/TestVrShellDelegate.java
[modify] https://crrev.com/9f8158c12189567f8b9f4bcb44dd3cfc68ecd470/chrome/android/javatests/src/org/chromium/chrome/browser/vr/util/TransitionUtils.java

Status: Fixed (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 17

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

commit 0062abf0e2d46669d756ad16493f684ecc38a832
Author: Brian Sheedy <bsheedy@chromium.org>
Date: Tue Jul 17 18:26:27 2018

Revert "VR: Ensure we remove black overlay if VR is exited while entering."

This reverts commit 9f8158c12189567f8b9f4bcb44dd3cfc68ecd470.

Reason for revert: Culprit of failed VR tests https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Nougat%20Phone%20Tester/6884

Original change's description:
> VR: Ensure we remove black overlay if VR is exited while entering.
> 
> This fixes a failure mode where we shutdownVR after handling the VR NFC
> broadcast, and fail to properly cancel VR entry.
> 
> Also does some other random cleanup of comments/visibility.
> 
> Bug:  862829 
> Change-Id: Ic382692d7255aa5893391a602a6c949b4b4d498b
> Reviewed-on: https://chromium-review.googlesource.com/1138726
> Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
> Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575388}

TBR=mthiesse@chromium.org,tiborg@chromium.org

Change-Id: Ibe67ee8c09dc44ac34d40c3eaf890884b400fc24
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  862829 
Reviewed-on: https://chromium-review.googlesource.com/1140813
Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575725}
[modify] https://crrev.com/0062abf0e2d46669d756ad16493f684ecc38a832/chrome/android/java/src/org/chromium/chrome/browser/vr/VrAlertDialog.java
[modify] https://crrev.com/0062abf0e2d46669d756ad16493f684ecc38a832/chrome/android/java/src/org/chromium/chrome/browser/vr/VrFirstRunActivity.java
[modify] https://crrev.com/0062abf0e2d46669d756ad16493f684ecc38a832/chrome/android/java/src/org/chromium/chrome/browser/vr/VrShellDelegate.java
[modify] https://crrev.com/0062abf0e2d46669d756ad16493f684ecc38a832/chrome/android/java/src/org/chromium/chrome/browser/vr/VrToast.java
[modify] https://crrev.com/0062abf0e2d46669d756ad16493f684ecc38a832/chrome/android/java/src/org/chromium/chrome/browser/vr/VrUiWidgetFactory.java
[modify] https://crrev.com/0062abf0e2d46669d756ad16493f684ecc38a832/chrome/android/javatests/src/org/chromium/chrome/browser/vr/TestVrShellDelegate.java
[modify] https://crrev.com/0062abf0e2d46669d756ad16493f684ecc38a832/chrome/android/javatests/src/org/chromium/chrome/browser/vr/util/TransitionUtils.java

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 18

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

commit 0fe8dc47e49278af69d09c988351fd6307524dd2
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Wed Jul 18 19:52:17 2018

Reland "VR: Ensure we remove black overlay if VR is exited while entering."

This reverts commit 0062abf0e2d46669d756ad16493f684ecc38a832.

Reason for revert: Fixing...

Original change's description:
> Revert "VR: Ensure we remove black overlay if VR is exited while entering."
>
> This reverts commit 9f8158c12189567f8b9f4bcb44dd3cfc68ecd470.
>
> Reason for revert: Culprit of failed VR tests https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Nougat%20Phone%20Tester/6884
>
> Original change's description:
> > VR: Ensure we remove black overlay if VR is exited while entering.
> >
> > This fixes a failure mode where we shutdownVR after handling the VR NFC
> > broadcast, and fail to properly cancel VR entry.
> >
> > Also does some other random cleanup of comments/visibility.
> >
> > Bug:  862829 
> > Change-Id: Ic382692d7255aa5893391a602a6c949b4b4d498b
> > Reviewed-on: https://chromium-review.googlesource.com/1138726
> > Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
> > Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#575388}
>
> TBR=mthiesse@chromium.org,tiborg@chromium.org
>
> Change-Id: Ibe67ee8c09dc44ac34d40c3eaf890884b400fc24
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  862829 
> Reviewed-on: https://chromium-review.googlesource.com/1140813
> Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
> Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575725}

Change-Id: I71738aafff0b703083f465085fb71f54d047729b
Bug:  862829 
Reviewed-on: https://chromium-review.googlesource.com/1142167
Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576173}
[modify] https://crrev.com/0fe8dc47e49278af69d09c988351fd6307524dd2/chrome/android/java/src/org/chromium/chrome/browser/vr/VrAlertDialog.java
[modify] https://crrev.com/0fe8dc47e49278af69d09c988351fd6307524dd2/chrome/android/java/src/org/chromium/chrome/browser/vr/VrFirstRunActivity.java
[modify] https://crrev.com/0fe8dc47e49278af69d09c988351fd6307524dd2/chrome/android/java/src/org/chromium/chrome/browser/vr/VrShellDelegate.java
[modify] https://crrev.com/0fe8dc47e49278af69d09c988351fd6307524dd2/chrome/android/java/src/org/chromium/chrome/browser/vr/VrToast.java
[modify] https://crrev.com/0fe8dc47e49278af69d09c988351fd6307524dd2/chrome/android/java/src/org/chromium/chrome/browser/vr/VrUiWidgetFactory.java
[modify] https://crrev.com/0fe8dc47e49278af69d09c988351fd6307524dd2/chrome/android/javatests/src/org/chromium/chrome/browser/vr/TestVrShellDelegate.java

Labels: -VR-Caught-By-Test XR-Caught-By-Test

Sign in to add a comment