New issue
Advanced search Search tips

Issue 793430 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug
Proj-XR
Proj-XR-VR

Blocking:
issue 740984
issue 793857



Sign in to add a comment

VR: Implement fallback Android UI rendering path.

Project Member Reported by mthiesse@chromium.org, Dec 8 2017

Issue description

See Same Window Fallback UI path here: https://docs.google.com/document/d/1-e4zudGHBA7nwqaDCjteLJCAx8j6XtOZu9EgMtjgX48/edit#heading=h.t1srsjv44306

This would allow us to render any java UI that shows up within the main window over top of the content quad in VR. This serves as a fallback path for any UI we fail to disable/fix/reimplement for VR. If we don't know about it, at least it will show up through this path and work reasonably well.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 12 2017

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

commit c85edd6385c70ed55dd0ed4582e7edd4604cdc84
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Tue Dec 12 18:10:54 2017

VR: Deliver onActivityShown/Hidden to VR even when there's no active tab

Bug:  793430 
Change-Id: I09108d77fb2f264f2315bb6d069ff22fa65396e8
Reviewed-on: https://chromium-review.googlesource.com/822692
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523476}
[modify] https://crrev.com/c85edd6385c70ed55dd0ed4582e7edd4604cdc84/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 15 2017

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

commit 665f9dec892d384015b6bec39035eb8bb36c32fc
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Fri Dec 15 19:23:43 2017

Account for dip scale mismatch in VR for control height.

In VR, we change the dip scale of the WebContents, but cannot properly
change the dip scale of Java UI because doing so would require
reinflating said java UI.

This leads to problems when determining the offset applied by the
top/bottom controls. RenderWidgetHostViewAndroid previously converted
the shown ratio of the controls to pixel values using the dip scale
of the WebContents, while ChromeFullscreenManager converted the control
height to pixels using the Java UI dpr. It then treated a mismatch
between these two values as the control container being visible, even
if the shown ratio was 0.

This CL changes the FullscreenManager to always apply fully-hidden
control offsets while in VR since the controls will be hidden anyways.

Bug:  793430 
Change-Id: If18aa601ed51ea91cef6b83060972d216a8cd6a9
Reviewed-on: https://chromium-review.googlesource.com/818116
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524430}
[modify] https://crrev.com/665f9dec892d384015b6bec39035eb8bb36c32fc/chrome/android/java/src/org/chromium/chrome/browser/fullscreen/ChromeFullscreenManager.java

Blocking: 740984
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 21 2017

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

commit 5bcdce39b9f9bbc93a7cfbf5361b3675ddebfb69
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Thu Dec 21 19:50:10 2017

Disable 2D progress bar while in VR.

VR has its own progress bar, so the 2D one shouldn't be visible.

Bug:  793430 
Change-Id: Ia8c7b5e3441c851d84d9e1c715207dcb5800f2c1
Reviewed-on: https://chromium-review.googlesource.com/833991
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525776}
[modify] https://crrev.com/5bcdce39b9f9bbc93a7cfbf5361b3675ddebfb69/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
[modify] https://crrev.com/5bcdce39b9f9bbc93a7cfbf5361b3675ddebfb69/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java

Blocking: 793857
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 8 2018

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

commit e70dd08d0a7d0a62639af3e553cfa80736949105
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Mon Jan 08 19:17:46 2018

Use event source window scaling to scale input in Android EventForwarder

In VR, the Android UI and the WebContents can have different dip scales
due to being unable to change the Android UI dip scale. In order to
deal with this, VR scales the Android UI after drawing it to a texture
to match the size of the WebContents. This means that input needs to be
scaled when coming from Android UI to align with input to the
WebContents.

Bug:  793430 
Change-Id: I918a80c586c58f6b6741eccf24f5594cb0ae45b3
Reviewed-on: https://chromium-review.googlesource.com/823236
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527704}
[modify] https://crrev.com/e70dd08d0a7d0a62639af3e553cfa80736949105/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
[modify] https://crrev.com/e70dd08d0a7d0a62639af3e553cfa80736949105/ui/android/event_forwarder.cc
[modify] https://crrev.com/e70dd08d0a7d0a62639af3e553cfa80736949105/ui/android/event_forwarder.h
[modify] https://crrev.com/e70dd08d0a7d0a62639af3e553cfa80736949105/ui/android/java/src/org/chromium/ui/base/EventForwarder.java
[modify] https://crrev.com/e70dd08d0a7d0a62639af3e553cfa80736949105/ui/android/java/src/org/chromium/ui/display/DisplayAndroid.java
[modify] https://crrev.com/e70dd08d0a7d0a62639af3e553cfa80736949105/ui/android/java/src/org/chromium/ui/display/VirtualDisplayAndroid.java

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 10 2018

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

commit ec629e65bf168556afd1a6ecaa958e23a8009752
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Wed Jan 10 21:44:17 2018

Allow VR to reuse the CompositorView with its own Surface

This CL allows the CompositorSurfaceManager to be replaced while in VR,
and allows VR to provide the Surface to render into.

The WindowAndroid the CompositorImplAndroid uses also has to be swapped
out as this isn't yet covered by the Tab reparenting flow (presumably
because it doesn't expect the reparented tab to stay in the same
Activity).

This CL was split off of
https://chromium-review.googlesource.com/c/chromium/src/+/652354, where
this new path is actually used.

Bug:  793430 
Change-Id: Id210df51b6f11206d492bb21ec0a64b09e45f953
Reviewed-on: https://chromium-review.googlesource.com/841222
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Ted Choc (back but slow, ping me) <tedchoc@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528433}
[modify] https://crrev.com/ec629e65bf168556afd1a6ecaa958e23a8009752/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorSurfaceManager.java
[add] https://crrev.com/ec629e65bf168556afd1a6ecaa958e23a8009752/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorSurfaceManagerImpl.java
[modify] https://crrev.com/ec629e65bf168556afd1a6ecaa958e23a8009752/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorView.java
[modify] https://crrev.com/ec629e65bf168556afd1a6ecaa958e23a8009752/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
[add] https://crrev.com/ec629e65bf168556afd1a6ecaa958e23a8009752/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrCompositorSurfaceManager.java
[modify] https://crrev.com/ec629e65bf168556afd1a6ecaa958e23a8009752/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappSplashScreenController.java
[modify] https://crrev.com/ec629e65bf168556afd1a6ecaa958e23a8009752/chrome/android/java_sources.gni
[rename] https://crrev.com/ec629e65bf168556afd1a6ecaa958e23a8009752/chrome/android/junit/src/org/chromium/chrome/browser/compositor/CompositorSurfaceManagerImplTest.java
[modify] https://crrev.com/ec629e65bf168556afd1a6ecaa958e23a8009752/chrome/browser/android/compositor/compositor_view.cc
[modify] https://crrev.com/ec629e65bf168556afd1a6ecaa958e23a8009752/chrome/browser/android/compositor/compositor_view.h
[modify] https://crrev.com/ec629e65bf168556afd1a6ecaa958e23a8009752/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/ec629e65bf168556afd1a6ecaa958e23a8009752/content/browser/renderer_host/compositor_impl_android.h
[modify] https://crrev.com/ec629e65bf168556afd1a6ecaa958e23a8009752/content/public/browser/android/compositor.h

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 10 2018

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

commit c76ab69c428b4bebbb07864c417aa27121daa567
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Wed Jan 10 23:00:54 2018

VR: Fix Controls being hidden when exiting VR *ONCE AND FOR ALL*

This CL removes our old hackiness for trying to get the controls to show
again after exiting VR, and instead waits for the next compositor frame
to show the controls (as the stale controls values will have propagated
by then)

Bug:  793430 
Change-Id: I6ee0fd29f9dafa2ae1e056100eeba165ac324e93
Reviewed-on: https://chromium-review.googlesource.com/860266
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Yash Malik <ymalik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528459}
[modify] https://crrev.com/c76ab69c428b4bebbb07864c417aa27121daa567/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java

Project Member

Comment 9 by bugdroid1@chromium.org, Jan 11 2018

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

commit 700d164101b69cb8f3af7a7f580ad7380af21c3c
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Thu Jan 11 00:00:42 2018

VR: Fix flaky testPresentationLocksFocus

Wait for presentation to be entered before changing focus. I'm really
not sure how this passed so consistently before... but it gets really
flaky with the fallback UI patch.

Bug:  793430 
Change-Id: I60d242a399d343fe02468d11a6aae9f681b88b44
Reviewed-on: https://chromium-review.googlesource.com/861102
Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528495}
[modify] https://crrev.com/700d164101b69cb8f3af7a7f580ad7380af21c3c/chrome/test/data/vr/e2e_test_files/html/test_presentation_locks_focus.html

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 22 2018

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

commit 196af7236168febb6fe898709ab264fcb04ee6a9
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Mon Jan 22 21:48:36 2018

Don't tear down ToolbarManager when its detached from the window.

It appears that the ToolbarManager was using onViewDetachedFromWindow
in order to perform destruction/cleanup, though the code I'm moving
here dates back to the merging of clank into chromium. Going back
through the history on the internal review site prior to the merge, the
destroy() function didn't exist at the time onViewDetachedFromWindow
was first used for cleanup, though it's not clear to me why this was
the case.

VR needs to reparent the view tree, which means detaching and
re-attaching views from/to the window. ToolbarManager already has a
destroy() function that appears to serve the same purpose as
onViewDetachedFromWindow, so this CL moves the code from there to
destroy().

Bug:  793430 
Change-Id: Iff5414b6698be81ec4bc4899a5686ed905e4359d
Reviewed-on: https://chromium-review.googlesource.com/878978
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531010}
[modify] https://crrev.com/196af7236168febb6fe898709ab264fcb04ee6a9/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 23 2018

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

commit 787c991f6677df8b8816d067f21c08a7510e75dc
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Tue Jan 23 00:03:56 2018

Don't crash when taking screenshot of Compositor-less window.

Under some circumstances the window having its screenshot taken might
not have a Compositor attached (like some of the time while in VR).

Avoid crashing, and just return an empty screenshot.

Bug:  793430 

Change-Id: Idfe6ef9937bc261e4e07b3630aa34d2a98df8ee7
Reviewed-on: https://chromium-review.googlesource.com/874792
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531070}
[modify] https://crrev.com/787c991f6677df8b8816d067f21c08a7510e75dc/ui/snapshot/snapshot.cc
[modify] https://crrev.com/787c991f6677df8b8816d067f21c08a7510e75dc/ui/snapshot/snapshot_android.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Jan 23 2018

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

commit 6396120689bb229328c2f351f44a05de54c40fde
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Tue Jan 23 01:17:53 2018

2D UI fallback path for VR.

This will draw any 2D UI VR isn't aware of on top of the content quad in
VR.

Design doc: https://docs.google.com/document/d/1-e4zudGHBA7nwqaDCjteLJCAx8j6XtOZu9EgMtjgX48/edit

This CL does a number of things that don't really make sense in isolation,
and so is difficult to split up:
1. Adds a parent view above R.id.content that draws the entire view tree
(through software rendering) into a texture while in VR (VrViewContainer).
Being a fallback path, the intent is that the vast majority of the time
nothing is rendered through this path. However, in practice some native
pages like chrome://crash will render through this path, as will infobars.

2. Routes VR input through the VrViewContainer. This does two things: It
allows java UI rendered through the fallback path to receive input, and
fixes long-standing issues with OOPIFs and such because it removes our
hacky input path that injected WebInputEvents directly into the
RenderWidgetHost, and instead uses the normal Android input path.

3. Necessitated by 1. and 2., this CL deletes vr_compositor and instead
integrates into the CompositorView, replacing its surface with the VR
surface that renders into a texture rather than a SurfaceView. This also
required changes to CompositorImplAndroid to allow VR to swap out the
window the compositor uses, as tab reparenting fails to account for this
when the reparenting doesn't actually move the Tab across Activities.
This part was split out into
https://chromium-review.googlesource.com/c/chromium/src/+/841222.

4. Adds the fallback UI texture to the VR scene, drawing it as a layer on
top of the content rect, with the region normally covered by the
CompositorView's SurfaceTexture transparent.

Bug:  793430 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Idc9c52e344daad77bac92b5f3aa0ff78c3e9e00f
Reviewed-on: https://chromium-review.googlesource.com/652354
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Yash Malik <ymalik@chromium.org>
Reviewed-by: Ian Vollick <vollick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531105}
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShell.java
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
[add] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrViewContainer.java
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/android/java_sources.gni
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/android/javatests/src/org/chromium/chrome/browser/share/ShareMenuActionHandlerTest.java
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/VrShellCompositorViewHolderTest.java
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/android/vr_shell/BUILD.gn
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/android/vr_shell/android_ui_gesture_target.cc
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/android/vr_shell/gl_browser_interface.h
[delete] https://crrev.com/8dc058cb83a65482774f838e7ad872a1ae6424f0/chrome/browser/android/vr_shell/vr_compositor.cc
[delete] https://crrev.com/8dc058cb83a65482774f838e7ad872a1ae6424f0/chrome/browser/android/vr_shell/vr_compositor.h
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/android/vr_shell/vr_gl_thread.cc
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/android/vr_shell/vr_gl_thread.h
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/android/vr_shell/vr_shell.cc
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/android/vr_shell/vr_shell.h
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/android/vr_shell/vr_shell_gl.cc
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/android/vr_shell/vr_shell_gl.h
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/android/vr_shell/vr_web_contents_observer.cc
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/android/vr_shell/vr_web_contents_observer.h
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/vr/BUILD.gn
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/vr/elements/content_element.cc
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/vr/elements/content_element.h
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/vr/model/model.h
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/vr/test/ui_pixel_test.cc
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/vr/test/ui_pixel_test.h
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/vr/testapp/vr_test_context.cc
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/vr/ui.cc
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/vr/ui.h
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/chrome/browser/vr/ui_scene_creator.cc
[delete] https://crrev.com/8dc058cb83a65482774f838e7ad872a1ae6424f0/chrome/browser/vr/web_contents_event_forwarder.cc
[delete] https://crrev.com/8dc058cb83a65482774f838e7ad872a1ae6424f0/chrome/browser/vr/web_contents_event_forwarder.h
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/content/browser/web_contents/web_contents_android.cc
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/content/browser/web_contents/web_contents_android.h
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java
[modify] https://crrev.com/6396120689bb229328c2f351f44a05de54c40fde/content/public/android/java/src/org/chromium/content_public/browser/WebContents.java

Status: Fixed (was: Started)
Remaining input/popup related issues are tracked by other bugs. Considering this fixed.
Labels: Test-Complete

Sign in to add a comment