New issue
Advanced search Search tips

Issue 752193 link

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



Sign in to add a comment

VR: Move device/vr implementation out of device::GvrDelegateProvider

Project Member Reported by mthiesse@chromium.org, Aug 3 2017 
The gvrDelegateProvider interface is implements parts of device/vr/android/gvr in the browser layer. Much of this isn't necessary and we should split vr_shell_delegate.cc into a separate browser layer and device layer class. We should handle focus tracking in device/vr instead of chrome/browser/android as well so that it can be shared across platforms.

device::GvrDelegateProvider should only have to handle presentRequests and listeningForActivate changing.

Comment 1 by mthiesse@chromium.org, Aug 3 2017

Summary: VR: Move device/vr implementation out of device::GvrDelegateProvider (was: VR: Remove the need for a device::GvrDelegateProvider)

Comment 2 by mthiesse@chromium.org, Aug 3 2017

Description: Show this description
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 16 2017 

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

commit 5def8d45e821877c0bf84fd7dbef9a154f96f72c
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Wed Aug 16 21:10:24 2017

device/vr refactor: Remove GvrDelegate interface, clean up GvrDeviceProvider and VrShellDelegate.

This CL removes the inheritance of GvrDelegate from vr_shell.cc, and
refactors VrShellDelegate to get the Device directly from
VrDeviceProvider and simplify lifecycles.

Bug:  752193 
Change-Id: If662537bb5e5fc15236db772a94db41803e32255
Reviewed-on: https://chromium-review.googlesource.com/617765
Reviewed-by: Brandon Jones <bajones@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494950}
[modify] https://crrev.com/5def8d45e821877c0bf84fd7dbef9a154f96f72c/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
[modify] https://crrev.com/5def8d45e821877c0bf84fd7dbef9a154f96f72c/chrome/browser/android/vr_shell/vr_shell.cc
[modify] https://crrev.com/5def8d45e821877c0bf84fd7dbef9a154f96f72c/chrome/browser/android/vr_shell/vr_shell.h
[modify] https://crrev.com/5def8d45e821877c0bf84fd7dbef9a154f96f72c/chrome/browser/android/vr_shell/vr_shell_delegate.cc
[modify] https://crrev.com/5def8d45e821877c0bf84fd7dbef9a154f96f72c/chrome/browser/android/vr_shell/vr_shell_delegate.h
[modify] https://crrev.com/5def8d45e821877c0bf84fd7dbef9a154f96f72c/device/vr/android/gvr/gvr_delegate.cc
[modify] https://crrev.com/5def8d45e821877c0bf84fd7dbef9a154f96f72c/device/vr/android/gvr/gvr_delegate.h
[modify] https://crrev.com/5def8d45e821877c0bf84fd7dbef9a154f96f72c/device/vr/android/gvr/gvr_delegate_provider.h
[modify] https://crrev.com/5def8d45e821877c0bf84fd7dbef9a154f96f72c/device/vr/android/gvr/gvr_device.cc
[modify] https://crrev.com/5def8d45e821877c0bf84fd7dbef9a154f96f72c/device/vr/android/gvr/gvr_device.h
[modify] https://crrev.com/5def8d45e821877c0bf84fd7dbef9a154f96f72c/device/vr/android/gvr/gvr_device_provider.cc
[modify] https://crrev.com/5def8d45e821877c0bf84fd7dbef9a154f96f72c/device/vr/android/gvr/gvr_device_provider.h
[modify] https://crrev.com/5def8d45e821877c0bf84fd7dbef9a154f96f72c/device/vr/vr_device_manager.cc
[modify] https://crrev.com/5def8d45e821877c0bf84fd7dbef9a154f96f72c/device/vr/vr_device_manager.h
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 17 2017 

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

commit bc15910d7bca4939f15660af1e9384859728d83b
Author: Eric Stevenson <estevenson@chromium.org>
Date: Thu Aug 17 04:19:29 2017

Revert "device/vr refactor: Remove GvrDelegate interface, clean up GvrDeviceProvider and VrShellDelegate."

This reverts commit 5def8d45e821877c0bf84fd7dbef9a154f96f72c.

Reason for revert: Adds static initializers on Android.

Original change's description:
> device/vr refactor: Remove GvrDelegate interface, clean up GvrDeviceProvider and VrShellDelegate.
> 
> This CL removes the inheritance of GvrDelegate from vr_shell.cc, and
> refactors VrShellDelegate to get the Device directly from
> VrDeviceProvider and simplify lifecycles.
> 
> Bug:  752193 
> Change-Id: If662537bb5e5fc15236db772a94db41803e32255
> Reviewed-on: https://chromium-review.googlesource.com/617765
> Reviewed-by: Brandon Jones <bajones@chromium.org>
> Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#494950}

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

Change-Id: If94172a4673b668a20eaef398391fa60bd047d6d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  752193 ,  756302 
Reviewed-on: https://chromium-review.googlesource.com/618514
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495068}
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/chrome/browser/android/vr_shell/vr_shell.cc
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/chrome/browser/android/vr_shell/vr_shell.h
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/chrome/browser/android/vr_shell/vr_shell_delegate.cc
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/chrome/browser/android/vr_shell/vr_shell_delegate.h
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/android/gvr/gvr_delegate.cc
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/android/gvr/gvr_delegate.h
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/android/gvr/gvr_delegate_provider.h
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/android/gvr/gvr_device.cc
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/android/gvr/gvr_device.h
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/android/gvr/gvr_device_provider.cc
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/android/gvr/gvr_device_provider.h
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/vr_device_manager.cc
[modify] https://crrev.com/bc15910d7bca4939f15660af1e9384859728d83b/device/vr/vr_device_manager.h
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 17 2017 

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

commit a36196109a35194d212e9c6b089ff23d343499d5
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Thu Aug 17 15:18:46 2017

Reland "device/vr refactor: Remove GvrDelegate interface, clean up GvrDeviceProvider and VrShellDelegate."

This is a reland of 5def8d45e821877c0bf84fd7dbef9a154f96f72c
Original change's description:
> device/vr refactor: Remove GvrDelegate interface, clean up GvrDeviceProvider and VrShellDelegate.
> 
> This CL removes the inheritance of GvrDelegate from vr_shell.cc, and
> refactors VrShellDelegate to get the Device directly from
> VrDeviceProvider and simplify lifecycles.
> 
> Bug:  752193 
> Change-Id: If662537bb5e5fc15236db772a94db41803e32255
> Reviewed-on: https://chromium-review.googlesource.com/617765
> Reviewed-by: Brandon Jones <bajones@chromium.org>
> Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#494950}

TBR=bajones@chromium.org

Bug:  752193 
Change-Id: I61c4516546119f39df153f98de9285e2a360e828
Reviewed-on: https://chromium-review.googlesource.com/619166
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495177}
[modify] https://crrev.com/a36196109a35194d212e9c6b089ff23d343499d5/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
[modify] https://crrev.com/a36196109a35194d212e9c6b089ff23d343499d5/chrome/browser/android/vr_shell/vr_shell.cc
[modify] https://crrev.com/a36196109a35194d212e9c6b089ff23d343499d5/chrome/browser/android/vr_shell/vr_shell.h
[modify] https://crrev.com/a36196109a35194d212e9c6b089ff23d343499d5/chrome/browser/android/vr_shell/vr_shell_delegate.cc
[modify] https://crrev.com/a36196109a35194d212e9c6b089ff23d343499d5/chrome/browser/android/vr_shell/vr_shell_delegate.h
[modify] https://crrev.com/a36196109a35194d212e9c6b089ff23d343499d5/device/vr/android/gvr/gvr_delegate.cc
[modify] https://crrev.com/a36196109a35194d212e9c6b089ff23d343499d5/device/vr/android/gvr/gvr_delegate.h
[modify] https://crrev.com/a36196109a35194d212e9c6b089ff23d343499d5/device/vr/android/gvr/gvr_delegate_provider.h
[modify] https://crrev.com/a36196109a35194d212e9c6b089ff23d343499d5/device/vr/android/gvr/gvr_device.cc
[modify] https://crrev.com/a36196109a35194d212e9c6b089ff23d343499d5/device/vr/android/gvr/gvr_device.h
[modify] https://crrev.com/a36196109a35194d212e9c6b089ff23d343499d5/device/vr/android/gvr/gvr_device_provider.cc
[modify] https://crrev.com/a36196109a35194d212e9c6b089ff23d343499d5/device/vr/android/gvr/gvr_device_provider.h
[modify] https://crrev.com/a36196109a35194d212e9c6b089ff23d343499d5/device/vr/vr_device_manager.cc
[modify] https://crrev.com/a36196109a35194d212e9c6b089ff23d343499d5/device/vr/vr_device_manager.h
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 5 2017 

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

commit 126077576f4e2bcdb65ca6f7d1e9ffda66aff153
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Tue Sep 05 19:12:07 2017

VR: Move non-presenting gvr context creation/ownership into device/vr/

No functional changes, just cleanup and preparation for moving the
focus related code down into device/vr/ as well, dramatically
simplifying VrShellDelegate.

Bug:  752193 
Change-Id: Id4820613f44b74da7b696921ff4af56bc1f10861
Reviewed-on: https://chromium-review.googlesource.com/621486
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Biao She <bshe@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499697}
[delete] https://crrev.com/3220623dee36d1b76776808e1db42159310b2080/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/NonPresentingGvrContext.java
[delete] https://crrev.com/3220623dee36d1b76776808e1db42159310b2080/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/NonPresentingGvrContextImpl.java
[modify] https://crrev.com/126077576f4e2bcdb65ca6f7d1e9ffda66aff153/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrClassesWrapper.java
[modify] https://crrev.com/126077576f4e2bcdb65ca6f7d1e9ffda66aff153/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrClassesWrapperImpl.java
[modify] https://crrev.com/126077576f4e2bcdb65ca6f7d1e9ffda66aff153/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
[modify] https://crrev.com/126077576f4e2bcdb65ca6f7d1e9ffda66aff153/chrome/android/java_sources.gni
[modify] https://crrev.com/126077576f4e2bcdb65ca6f7d1e9ffda66aff153/chrome/browser/android/vr_shell/vr_shell_delegate.cc
[modify] https://crrev.com/126077576f4e2bcdb65ca6f7d1e9ffda66aff153/chrome/browser/android/vr_shell/vr_shell_delegate.h
[modify] https://crrev.com/126077576f4e2bcdb65ca6f7d1e9ffda66aff153/content/public/android/BUILD.gn
[modify] https://crrev.com/126077576f4e2bcdb65ca6f7d1e9ffda66aff153/device/BUILD.gn
[modify] https://crrev.com/126077576f4e2bcdb65ca6f7d1e9ffda66aff153/device/vr/BUILD.gn
[modify] https://crrev.com/126077576f4e2bcdb65ca6f7d1e9ffda66aff153/device/vr/android/gvr/gvr_delegate_provider.h
[modify] https://crrev.com/126077576f4e2bcdb65ca6f7d1e9ffda66aff153/device/vr/android/gvr/gvr_device.cc
[modify] https://crrev.com/126077576f4e2bcdb65ca6f7d1e9ffda66aff153/device/vr/android/gvr/gvr_device.h
[modify] https://crrev.com/126077576f4e2bcdb65ca6f7d1e9ffda66aff153/device/vr/android/gvr/gvr_device_provider.cc
[add] https://crrev.com/126077576f4e2bcdb65ca6f7d1e9ffda66aff153/device/vr/android/java/src/org/chromium/device/vr/NonPresentingGvrContext.java
[modify] https://crrev.com/126077576f4e2bcdb65ca6f7d1e9ffda66aff153/device/vr/vr_device.h
[modify] https://crrev.com/126077576f4e2bcdb65ca6f7d1e9ffda66aff153/device/vr/vr_service_impl.cc
[modify] https://crrev.com/126077576f4e2bcdb65ca6f7d1e9ffda66aff153/device/vr/vr_service_impl.h
Project Member

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

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

commit 345f52dc2dabbf88f614b5971676debf203599b0
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Mon Sep 18 17:56:44 2017

VR: Make GetVrDisplayInfo synchronous, remove count-based synchronization.

This is just a cleanup refactor. The only user-visible change is a fix
to the WebVR surface possibly having the wrong default size when viewing
multiple webVR experiences while browsing in VR.

Calling OnChanged and getting the VrDisplayInfo from the GL thread on
Android was just a very complex no-op, so I removed it. On Android the
VrDisplayInfo doesn't ever actually change.

Bug:  752193 
Change-Id: I85d0d668be798ebbe27007bd5d115cba5341d2b6
Reviewed-on: https://chromium-review.googlesource.com/655122
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502608}
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/chrome/browser/android/vr_shell/gl_browser_interface.h
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/chrome/browser/android/vr_shell/vr_gl_thread.cc
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/chrome/browser/android/vr_shell/vr_gl_thread.h
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/chrome/browser/android/vr_shell/vr_shell.cc
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/chrome/browser/android/vr_shell/vr_shell.h
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/chrome/browser/android/vr_shell/vr_shell_delegate.cc
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/chrome/browser/android/vr_shell/vr_shell_delegate.h
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/chrome/browser/android/vr_shell/vr_shell_gl.cc
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/chrome/browser/android/vr_shell/vr_shell_gl.h
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/device/vr/android/gvr/gvr_delegate.cc
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/device/vr/android/gvr/gvr_delegate.h
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/device/vr/android/gvr/gvr_delegate_provider.h
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/device/vr/android/gvr/gvr_device.cc
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/device/vr/android/gvr/gvr_device.h
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/device/vr/openvr/openvr_device.cc
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/device/vr/openvr/openvr_device.h
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/device/vr/test/fake_vr_device.cc
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/device/vr/test/fake_vr_device.h
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/device/vr/vr_device.cc
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/device/vr/vr_device.h
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/device/vr/vr_device_manager_unittest.cc
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/device/vr/vr_display_impl_unittest.cc
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/device/vr/vr_service.mojom
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/device/vr/vr_service_impl.cc
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/device/vr/vr_service_impl.h
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/device/vr/vr_service_impl_unittest.cc
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/third_party/WebKit/Source/modules/vr/VRController.cpp
[modify] https://crrev.com/345f52dc2dabbf88f614b5971676debf203599b0/third_party/WebKit/Source/modules/vr/VRController.h

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

Status: Fixed (was: Started)
Going to close this bug, this has been super-ceded by  issue 768923  and issue 769373

Comment 9 by btebbs@chromium.org, Jul 4

Components: Internals>XR
►

Sign in to add a comment