New issue
Advanced search Search tips

Issue 735163 link

Starred by 1 user
Status: Fixed
Owner:
cjgrant@chromium.org
Closed: Jun 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug
Proj-VR
Proj-XR
Proj-XR-VR

Blocked on:
issue 736558


Sign in to add a comment

Refactor common transient visibility code out of UI elements.

Project Member Reported by cjgrant@chromium.org, Jun 20 2017 
The VR UI has several elements that exhibit transient visibility:

- Temporary WebVR warning
- Loading indicator
- WebVR URL bar
- Presentation/fullscreen toasts

Instead of each element and/or scene manager manually controlling visibility, the elements should derive from a class.  That class could offer:

- Enable() to show the element temporarily
- Disable() to immediately hide the element
- Kick() to re-show the element temporarily if enabled
- ForceVisible() to keep the element visible indefinitely if enabled.

Comment 1 by cjgrant@chromium.org, Jun 21 2017 

Also, unit tests should be amended to actually cover transient warnings disappearing.  That means baking a timebase into the message loop, and having the test advance it.

Comment 2 by ddorwin@chromium.org, Jun 21 2017

Labels: VR-TD

Comment 3 by cjgrant@chromium.org, Jun 24 2017

Blockedon: 736558

Comment 4 by cjgrant@chromium.org, Jun 24 2017

Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 26 2017 

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

commit 2f5d2f7b0721ace2fcbe408d8374f1c8277c71de
Author: cjgrant <cjgrant@chromium.org>
Date: Mon Jun 26 16:39:26 2017

VR: Add support for time-aware unit tests.

Add an initial unit test for the transient WebVR warning. This CL is a
lead-in to pulling common transient code out of scene manager and
specific elements.

Note that this development seemed to expose a bug in the mock task
runner, in conjunction with timers. This bug is now blocked on that
bug.

BUG= 735163 

Review-Url: https://codereview.chromium.org/2960533002
Cr-Commit-Position: refs/heads/master@{#482294}

[modify] https://crrev.com/2f5d2f7b0721ace2fcbe408d8374f1c8277c71de/chrome/browser/android/vr_shell/ui_scene_manager_unittest.cc

Comment 6 by cjgrant@chromium.org, Jun 26 2017 

Work has started here:

https://codereview.chromium.org/2955023002/
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 29 2017 

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

commit 27ed3b381ba509ef709ebc97860d006611bab291
Author: cjgrant <cjgrant@chromium.org>
Date: Thu Jun 29 22:24:05 2017

VR: Factor transient timing out of UiSceneManager.

- Add a class to manage transience. It can later be extended to handle
  more interesting transience, like URL bars, which appear when
  loading.
- Move presentation toast to use this new manager.

BUG= 735163 

Review-Url: https://codereview.chromium.org/2955023002
Cr-Commit-Position: refs/heads/master@{#483515}

[modify] https://crrev.com/27ed3b381ba509ef709ebc97860d006611bab291/chrome/browser/android/vr_shell/BUILD.gn
[modify] https://crrev.com/27ed3b381ba509ef709ebc97860d006611bab291/chrome/browser/android/vr_shell/ui_elements/exclusive_screen_toast.cc
[modify] https://crrev.com/27ed3b381ba509ef709ebc97860d006611bab291/chrome/browser/android/vr_shell/ui_elements/exclusive_screen_toast.h
[modify] https://crrev.com/27ed3b381ba509ef709ebc97860d006611bab291/chrome/browser/android/vr_shell/ui_elements/simple_textured_element.h
[add] https://crrev.com/27ed3b381ba509ef709ebc97860d006611bab291/chrome/browser/android/vr_shell/ui_elements/transience_manager.cc
[add] https://crrev.com/27ed3b381ba509ef709ebc97860d006611bab291/chrome/browser/android/vr_shell/ui_elements/transience_manager.h
[add] https://crrev.com/27ed3b381ba509ef709ebc97860d006611bab291/chrome/browser/android/vr_shell/ui_elements/transience_manager_unittest.cc
[modify] https://crrev.com/27ed3b381ba509ef709ebc97860d006611bab291/chrome/browser/android/vr_shell/ui_elements/transient_url_bar.cc
[modify] https://crrev.com/27ed3b381ba509ef709ebc97860d006611bab291/chrome/browser/android/vr_shell/ui_elements/transient_url_bar.h
[modify] https://crrev.com/27ed3b381ba509ef709ebc97860d006611bab291/chrome/browser/android/vr_shell/ui_scene_manager.cc
[modify] https://crrev.com/27ed3b381ba509ef709ebc97860d006611bab291/chrome/browser/android/vr_shell/ui_scene_manager.h
[modify] https://crrev.com/27ed3b381ba509ef709ebc97860d006611bab291/chrome/browser/android/vr_shell/ui_scene_manager_unittest.cc

Comment 8 by cjgrant@chromium.org, Jun 30 2017

Status: Fixed (was: Started)
►

Sign in to add a comment