New issue
Advanced search Search tips

Issue 722543 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug
Proj-VR
Proj-XR
Proj-XR-VR



Sign in to add a comment

VR Shell: Cannot interact with bottom ~1/4 of webpage.

Project Member Reported by mthiesse@chromium.org, May 15 2017

Issue description

Navigate to a wikipedia page in VR, exit VR, then re-enter VR. Try clicking/hovering links in the bottom 1/4 of the page and note that it doesn't work.
 
Actually, I'm failing to reproduce after re-launching chrome. Not sure what the steps to trigger this are.
Description: Show this description
Related to  issue 715182 ? Looks like exiting and re-entering VR triggers this.

Comment 4 Deleted

Owner: billorr@chromium.org
Status: Assigned (was: Available)
just hit a repro on canary - I'll investigate.
Status: Fixed (was: Assigned)
We appear to be going down some of the fallback paths in PaintLayer::HitTest.  I'm unable to repro in recent builds, but Canary still hits it consistently for me.  I'm unable to repro in builds that had the fix for 715182, but can easily repro when I remove that fix.  I'm going to resolve as fixed - same cause as 715218.
Status: Assigned (was: Fixed)
Reactivating - this is hitting in builds with the fix for 715218.
Status: Started (was: Assigned)
I'm also having trouble interacting with the upper left corner of the screen.

When visiting usatoday.com for the first time, it displays a bar asking you to install their Android app. While I can click the Install button the right side, I can't click the 'x' on the left side. I can click the hamburger menu just below it. After exiting VR, I can click the 'x'.
When we enter webvr, we try to set the viewport to 960x640.  However, this ends up turning into 960x477, to leave room for the address bar.  I've lately only been able to repro on Canary rather than private builds, unless I turn on the "Chrome Home Android" setting in about:flags for private builds.
In VrShellImpl, we call setContentCssSize to update a virtual display's size, which updates CVC's size, layout size, etc.

This bubbles throughout the system, eventually making it to RenderWidgetHostViewBase, which sets the viewport size (as obtained by CVCImpl::GetViewSize().  This subtracts off the size of the controls.

DoBrowserControlsShrinkBlinkSize() is unfortunately returning true while we are in VR after we set the size.

We do try to hide the controls through updateFullscreenEnabledState, which starts sending async messages which start animations to actually hide the controls, but we want to hide them synchronously to make the sizing correct.

In order to fix this, we need doBrowserControlsShrinkBlinkSize() to return false while in VR.  We can do this by:
1. making CVC aware of VR.
2. adding methods to push an override to hide controls at the CVC level (VRShell would call these methods)
3. adding plumbing to push the override through from fullscreen manager's updateFullscreenEnabledState, so we can synchronously change state.

Wait, so why is this value not being updated after the browser controls are hidden? The content area size should change when the controls are hidden...

I think trying to make this synchronous probably wouldn't be the right solution?
Cc: jinsuk...@chromium.org
Please see if you can avoid adding more methods in CVC or let content layer be aware of VR mode. Fix would have been much easier if resize events had been refactored to go through ViewAndroid tree but it's still in progress  Issue 722543 

A quick remedy I can suggest is to have RWHVA return the adjusted size using DoBrowserControlsShrinkBlinkSize().
... I meant  Issue 622847 
mthiesse, The controls are set to animate out, and the animation doesn't play because VrShell stops vsyncs.  This means that the controls are never fully hidden (problem 1).

If we enable animations, the bug would repro until the animation completes.  Maybe we could somehow force the animation to complete (problem 2).

Finally, even if we do wait for the animation to complete, it doesn't trigger resize events throughout the stack.  I haven't fully traced down whether they aren't even triggered or whether they get stopped somewhere.  The size through much of the stack doesn't think the size has changed. (problem 3).

I agree that the ideal fix would be:
1. Either directly trigger hiding or force the animation to complete to address problem 1 and 2.
2. Ensure that notifications throughout the stack handle viewport changing because controls were hidden.

However, this looks like a more complicated/risky fix, and I'd rather merge back a simpler change, especially if there is an upcoming refactor that could address some of the complexity here.
I looked at why we don't resize when the controls are hidden.  It looks like the code that should do this is UpdateICBAndResizeViewport.  Two interesting things.  First it looks like we'll only subtract off height (so the viewport and ICB are the same whether controls are shown or not).  Second, the control height is ultimately coming from the top controls height, not the bottom controls height.

I'll investigate more tomorrow to see whether this is actually a cleaner fix.
Cc: billorr@chromium.org
Owner: mdjones@chromium.org
Project Member

Comment 18 by bugdroid1@chromium.org, Jun 13 2017

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

commit a4ab2b3a0adc017cfe2d0ab19ad2092bf3beabb7
Author: Matthew Jones <mdjones@chromium.org>
Date: Tue Jun 13 00:19:11 2017

[Home] Fix vr dimensions in Chrome Home

This change modifies the fullscreen manager's concept of whether or
not the browser controls should modify the viewport to depend on
the whether or not VR is active.

BUG= 722543 ,  730772 

Change-Id: I00770dffe6d278232d9e8a0d7ee5dd9be9c01a87
Reviewed-on: https://chromium-review.googlesource.com/528521
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478837}
[modify] https://crrev.com/a4ab2b3a0adc017cfe2d0ab19ad2092bf3beabb7/chrome/android/java/src/org/chromium/chrome/browser/fullscreen/ChromeFullscreenManager.java

Status: Fixed (was: Started)
Status: Verified (was: Fixed)
Verified in Chrome Canary 61.0.3138.0

Sign in to add a comment