New issue
Advanced search Search tips

Issue 743687 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android , Windows
Pri: 1
Type: Bug
Proj-XR
Proj-XR-VR

Blocking:
issue 773882



Sign in to add a comment

Daydream controller models are included in all VR-enabled platforms

Project Member Reported by vollick@chromium.org, Jul 15 2017

Issue description

- We will eventually want to run automated tests that check visuals. Just as we have generic scrollbars for blink tests, we should have generic assets for a VR controller.

- Move Daydream controller specific code from //chrome/browser/vr/vr_controller_model.cc into Android-specific folder. Along with this, include //chrome/browser/resources/vr_shell_resources.grd on Android only.
 

Comment 1 by tiborg@chromium.org, Jul 17 2017

Summary: VR: Refactor Android specific controller model code and resources out of common build. (was: Create generic controller models for testing)

Comment 2 by tiborg@chromium.org, Jul 17 2017

Description: Show this description

Comment 3 by tiborg@chromium.org, Jul 17 2017

Description: Show this description
Labels: -M-62 M-63
Blocking: 773882
Cc: billorr@chromium.org
Labels: -Pri-3 -M-63 M-64 VR-Desktop OS-Android OS-Windows Pri-1
Summary: Daydream controller models are included in all VR-enabled platforms (was: VR: Refactor Android specific controller model code and resources out of common build.)
These Daydream-specific resources are going to be included in non-Android platforms when we enable VR devices on Windows.

Summarizing the current state:

//src/chrome/browser/vr/vr_controller_model.cc is included by //src/chrome/browser/vr/BUILD.gn, which applies to all builds with enable_vr=true.

vr_controller_model.cc uses several IDR_VR_SHELL_DDCONTROLLER_* resources. In addition, //src/chrome/browser/resources/vr_shell_resources.grd and its build files do not have platform checks.

We may want/need to add a build flag for VR browser so we don't need this code and associated resources on platforms that don't currently support it.
On a somewhat related note, IDS_PAGE_INFO_VR_BROWSER_UNSUPPORTED_MODE is being included in all builds. It should probably only be included `<if expr="enable_vr">`. Fixing this doesn't require any refactoring.
Owner: billorr@chromium.org
Status: Started (was: Available)
We'll want this soonish to avoid install size bloat in M63.
M64?
The goal of this bug has shifted over time. I've logged crbug.com/779108 to capture the original intent. I feel that creating neutral controllers is the correct long term direction, but this is a reasonable tactical solution in the short term.
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 9 2017

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

commit fd1e89557fa78a467061f732a8473fd863a89b1a
Author: Bill Orr <billorr@chromium.org>
Date: Thu Nov 09 01:41:49 2017

Don't include Daydream controller models on all VR-enabled platforms

This change moves daydream-specific assets to a vr_daydream_resources.grd,
which is processed on all enable_vr builds, but only included in Chrome on
enable_gvr_services builds.  Test code can still depend on these assets
even if running on desktop.

BUG= 743687 

Change-Id: Ie2516ae2534858044c00f281eb2f71d0cccd8aef
Reviewed-on: https://chromium-review.googlesource.com/752035
Commit-Queue: Bill Orr <billorr@chromium.org>
Reviewed-by: Ian Vollick <vollick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515052}
[modify] https://crrev.com/fd1e89557fa78a467061f732a8473fd863a89b1a/chrome/browser/vr/BUILD.gn
[modify] https://crrev.com/fd1e89557fa78a467061f732a8473fd863a89b1a/chrome/chrome_paks.gni

Status: Fixed (was: Started)

Sign in to add a comment