New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 847271 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2018
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 830121



Sign in to add a comment

VR: Separate the gesture recognition code from the VrController

Project Member Reported by acondor@chromium.org, May 28 2018

Issue description

The dependency of VrController on GvrApi makes it very difficult to unit test. Gesture recognition is an important logic that should be tested and doesn't need to depend on the GvrApi directly.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 30 2018

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

commit a45859e2bafed9e482e61aceb2ab0cb7efee4ab5
Author: Aldo Culquicondor <acondor@chromium.org>
Date: Wed May 30 21:42:30 2018

VR: Separate gesture recognition from VrController

Bug:  847271 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
Change-Id: I56edc54dcb24fd387944dc1dd937a66348ade484
Reviewed-on: https://chromium-review.googlesource.com/1077053
Reviewed-by: Amirhossein Simjour <asimjour@chromium.org>
Commit-Queue: Aldo Culquicondor <acondor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563009}
[modify] https://crrev.com/a45859e2bafed9e482e61aceb2ab0cb7efee4ab5/chrome/browser/android/vr/vr_controller.cc
[modify] https://crrev.com/a45859e2bafed9e482e61aceb2ab0cb7efee4ab5/chrome/browser/android/vr/vr_controller.h
[modify] https://crrev.com/a45859e2bafed9e482e61aceb2ab0cb7efee4ab5/chrome/browser/vr/BUILD.gn
[add] https://crrev.com/a45859e2bafed9e482e61aceb2ab0cb7efee4ab5/chrome/browser/vr/gesture_detector.cc
[add] https://crrev.com/a45859e2bafed9e482e61aceb2ab0cb7efee4ab5/chrome/browser/vr/gesture_detector.h
[add] https://crrev.com/a45859e2bafed9e482e61aceb2ab0cb7efee4ab5/chrome/browser/vr/gesture_detector_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, May 30 2018

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

commit 02d1ac1c43e96831149f3b92473e8b58c0af6d29
Author: Thomas Anderson <thomasanderson@chromium.org>
Date: Wed May 30 22:11:25 2018

Revert "VR: Separate gesture recognition from VrController"

This reverts commit a45859e2bafed9e482e61aceb2ab0cb7efee4ab5.

Reason for revert: Causing build failure on Linux Builder (dbg):
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Builder%20%28dbg%29/125172

Original change's description:
> VR: Separate gesture recognition from VrController
> 
> Bug:  847271 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
> Change-Id: I56edc54dcb24fd387944dc1dd937a66348ade484
> Reviewed-on: https://chromium-review.googlesource.com/1077053
> Reviewed-by: Amirhossein Simjour <asimjour@chromium.org>
> Commit-Queue: Aldo Culquicondor <acondor@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#563009}

TBR=asimjour@chromium.org,acondor@chromium.org

Change-Id: I1e22c9a964352ce63895fcaad902ed8413e03609
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  847271 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
Reviewed-on: https://chromium-review.googlesource.com/1079988
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563012}
[modify] https://crrev.com/02d1ac1c43e96831149f3b92473e8b58c0af6d29/chrome/browser/android/vr/vr_controller.cc
[modify] https://crrev.com/02d1ac1c43e96831149f3b92473e8b58c0af6d29/chrome/browser/android/vr/vr_controller.h
[modify] https://crrev.com/02d1ac1c43e96831149f3b92473e8b58c0af6d29/chrome/browser/vr/BUILD.gn
[delete] https://crrev.com/892c291ac3526d45fd01d37655eac336c36836ce/chrome/browser/vr/gesture_detector.cc
[delete] https://crrev.com/892c291ac3526d45fd01d37655eac336c36836ce/chrome/browser/vr/gesture_detector.h
[delete] https://crrev.com/892c291ac3526d45fd01d37655eac336c36836ce/chrome/browser/vr/gesture_detector_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, May 31 2018

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

commit 891014ef9cf494f09ca0a77e2a94f93a6658f3f4
Author: Aldo Culquicondor <acondor@chromium.org>
Date: Thu May 31 16:16:17 2018

Reland "VR: Separate gesture recognition from VrController"

This is a reland of a45859e2bafed9e482e61aceb2ab0cb7efee4ab5

Original change's description:
> VR: Separate gesture recognition from VrController
> 
> Bug:  847271 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
> Change-Id: I56edc54dcb24fd387944dc1dd937a66348ade484
> Reviewed-on: https://chromium-review.googlesource.com/1077053
> Reviewed-by: Amirhossein Simjour <asimjour@chromium.org>
> Commit-Queue: Aldo Culquicondor <acondor@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#563009}

Bug:  847271 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:ios-simulator-full-configs;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ie3835bec81a24026a49fc5663db82615a7a7d44d
Reviewed-on: https://chromium-review.googlesource.com/1080373
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Commit-Queue: Aldo Culquicondor <acondor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563264}
[modify] https://crrev.com/891014ef9cf494f09ca0a77e2a94f93a6658f3f4/chrome/browser/android/vr/vr_controller.cc
[modify] https://crrev.com/891014ef9cf494f09ca0a77e2a94f93a6658f3f4/chrome/browser/android/vr/vr_controller.h
[modify] https://crrev.com/891014ef9cf494f09ca0a77e2a94f93a6658f3f4/chrome/browser/vr/BUILD.gn
[add] https://crrev.com/891014ef9cf494f09ca0a77e2a94f93a6658f3f4/chrome/browser/vr/gesture_detector.cc
[add] https://crrev.com/891014ef9cf494f09ca0a77e2a94f93a6658f3f4/chrome/browser/vr/gesture_detector.h
[add] https://crrev.com/891014ef9cf494f09ca0a77e2a94f93a6658f3f4/chrome/browser/vr/gesture_detector_unittest.cc

Status: Fixed (was: Untriaged)
Owner: acondor@chromium.org

Sign in to add a comment