Fix placing phone in headset exiting VR.

Fix placing phone in headset exiting VR.

Currently, daydream DON flow can be passed without putting the phone in the headset (rotating to landscape and pairing the controller passes the DON flow). At the same time, because we don't integrate with daydream home, and we're not a separate activity, we exit VR any time we're paused.

This leads to a situation where we're in VR without having the headset on, we put the phone into the headset, the NFC trigger fires and launches the NFC handling activity (which immediately exits because we're in VR), which causes us to pause and exit VR. There's no way at the moment to detect that we're returning from the NFC activity, short of hacks around listening to the NFC trigger ourselves.

This CL adds a timeout for re-entering VR on CTA resume, after a CTA pause, that should be short enough to avoid any user interaction intending to exit VR leading them to re-enter it.

BUG=641401
Committed: https://crrev.com/dca762297c5f26cb188075775d571c00702d77a4
Cr-Commit-Position: refs/heads/master@{#432773}

[mthiesse, 2016-11-16 15:57:02]

mthiesse@chromium.org changed reviewers:
+ bajones@chromium.org, bshe@chromium.org

[mthiesse, 2016-11-16 15:57:03]

PTAL

[bshe, 2016-11-16 16:57:51]

hum. using a timer is really unfortunate. But I agree there is no better way to
handle the case you mention. We may want to reach out to GVR team about it.

https://codereview.chromium.org/2510713003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
(right):

https://codereview.chromium.org/2510713003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:250:
if (mInVr) {
If we shutdown vr on pause, is mInVr always false here?

https://codereview.chromium.org/2510713003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:261:
enterVRIfNecessary(mRequestedWebVR);
Is it possible to exit VR after a timeout? This way, we don't need to tear down
and re create VrShell in this case.

[mthiesse, 2016-11-16 17:32:29]

https://codereview.chromium.org/2510713003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
(right):

https://codereview.chromium.org/2510713003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:250:
if (mInVr) {
On 2016/11/16 16:57:51, bshe wrote:
> If we shutdown vr on pause, is mInVr always false here?

Yes, for now.

https://codereview.chromium.org/2510713003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:261:
enterVRIfNecessary(mRequestedWebVR);
On 2016/11/16 16:57:51, bshe wrote:
> Is it possible to exit VR after a timeout? This way, we don't need to tear
down
> and re create VrShell in this case.

Should be possible, but if we want to do that I'd rather do that in a separate
CL, after I refactor our teardown logic to work better for the "take your
headset off" case.

[bshe, 2016-11-16 17:57:00]

On 2016/11/16 17:32:29, mthiesse wrote:
>
https://codereview.chromium.org/2510713003/diff/20001/chrome/android/java/src...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
> (right):
> 
>
https://codereview.chromium.org/2510713003/diff/20001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:250:
> if (mInVr) {
> On 2016/11/16 16:57:51, bshe wrote:
> > If we shutdown vr on pause, is mInVr always false here?
> 
> Yes, for now.
> 
>
https://codereview.chromium.org/2510713003/diff/20001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:261:
> enterVRIfNecessary(mRequestedWebVR);
> On 2016/11/16 16:57:51, bshe wrote:
> > Is it possible to exit VR after a timeout? This way, we don't need to tear
> down
> > and re create VrShell in this case.
> 
> Should be possible, but if we want to do that I'd rather do that in a separate
> CL, after I refactor our teardown logic to work better for the "take your
> headset off" case.

lgtm you probably need owner though.

[mthiesse, 2016-11-16 18:46:34]

mthiesse@chromium.org changed reviewers:
+ tedchoc@chromium.org

[mthiesse, 2016-11-16 18:46:34]

+tedchoc@chromium.org for OWNERS review.

[Ted C, 2016-11-16 21:26:07]

lgtm

You should consider adding an OWNERS file for:
chrome/android/java/src/org/chromium/chrome/browser/vr_shell

Still happy to help with broader android/java stuff, but for many of these
changes it could be handled by your team.

https://codereview.chromium.org/2510713003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
(right):

https://codereview.chromium.org/2510713003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:78:
private long mLastVRExit = 0;
no need for = 0; that is the default (null, 0, and false)

[mthiesse, 2016-11-17 03:50:18]

https://codereview.chromium.org/2510713003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
(right):

https://codereview.chromium.org/2510713003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:78:
private long mLastVRExit = 0;
On 2016/11/16 21:26:07, Ted C wrote:
> no need for = 0; that is the default (null, 0, and false)

Done.

[mthiesse, 2016-11-17 03:50:27]

The CQ bit was checked by mthiesse@chromium.org

[mthiesse, 2016-11-17 03:50:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
tedchoc@chromium.org, bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/2510713003/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2016-11-17 03:51:21]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-17 05:49:49]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-17 05:53:11]

Description was changed from

==========
Fix placing phone in headset exiting VR.

Currently, daydream DON flow can be passed without putting the phone in the
headset (rotating to landscape and pairing the controller passes the DON flow).
At the same time, because we don't integrate with daydream home, and we're not a
separate activity, we exit VR any time we're paused.

This leads to a situation where we're in VR without having the headset on, we
put the phone into the headset, the NFC trigger fires and launches the NFC
handling activity (which immediately exits because we're in VR), which causes us
to pause and exit VR. There's no way at the moment to detect that we're
returning from the NFC activity, short of hacks around listening to the NFC
trigger ourselves.

This CL adds a timeout for re-entering VR on CTA resume, after a CTA pause, that
should be short enough to avoid any user interaction intending to exit VR
leading them to re-enter it.

BUG=641401
==========

to

==========
Fix placing phone in headset exiting VR.

Currently, daydream DON flow can be passed without putting the phone in the
headset (rotating to landscape and pairing the controller passes the DON flow).
At the same time, because we don't integrate with daydream home, and we're not a
separate activity, we exit VR any time we're paused.

This leads to a situation where we're in VR without having the headset on, we
put the phone into the headset, the NFC trigger fires and launches the NFC
handling activity (which immediately exits because we're in VR), which causes us
to pause and exit VR. There's no way at the moment to detect that we're
returning from the NFC activity, short of hacks around listening to the NFC
trigger ourselves.

This CL adds a timeout for re-entering VR on CTA resume, after a CTA pause, that
should be short enough to avoid any user interaction intending to exit VR
leading them to re-enter it.

BUG=641401

Committed: https://crrev.com/dca762297c5f26cb188075775d571c00702d77a4
Cr-Commit-Position: refs/heads/master@{#432773}
==========

[commit-bot: I haz the power, 2016-11-17 05:53:12]

Patchset 4 (id:??) landed as
https://crrev.com/dca762297c5f26cb188075775d571c00702d77a4
Cr-Commit-Position: refs/heads/master@{#432773}