Skip to content

GitLab

T
tangled
Commit f2ccbfd1 authored by Michael Thiessen's avatar Michael Thiessen Committed by Commit Bot
Browse files
Options

VR: Clean up WebVR headset insertion. 

This CL makes it explicitly clear when we care about whether the page
was listening for activate before pause, and when we should autopresent
on entering VR (for headset insertion).

This also removes the workaround in vr_display_impl that allowed pages
without focus to handle displayActivate, favoring handling this case
more correctly in VrShellDelegate.

Bug: 829513
Change-Id: Ie19c55305d0c59ec3f576010ded0f61ae906a084
Reviewed-on: https://chromium-review.googlesource.com/1004927Reviewed-by: default avatarYash Malik <ymalik@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549736}
parent 9f2ecbcc
Hide whitespace changes
Inline Side-by-side
Showing with 30 additions and 62 deletions
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
View file @ f2ccbfd1
......@@ -196,7 +196,8 @@ public class VrShellDelegate
private boolean mRequestedWebVr;
private boolean mRequestedWebVrBeforePause;
private boolean mListeningForWebVrActivate;
private boolean mListeningForWebVrActivateBeforePause;
private boolean mMaybeActivateAfterHeadsetInsertion;
private boolean mActivateFromHeadsetInsertion;
// Whether or not we should autopresent WebVr. If this is set, it means that a first
// party app has asked us to autopresent WebVr content and we're waiting for the WebVr
// content to call requestPresent.
......@@ -298,7 +299,8 @@ public class VrShellDelegate
if (sInstance.mPaused) {
if (sInstance.mInVrAtChromeLaunch == null) sInstance.mInVrAtChromeLaunch = false;
sInstance.mActivateFromHeadsetInsertion =
sInstance.mMaybeActivateAfterHeadsetInsertion;
if (activity instanceof ChromeTabbedActivity) {
// We can special case singleInstance activities like CTA to avoid having to use
// moveTaskToFront. Using moveTaskToFront prevents us from disabling window
......@@ -1091,7 +1093,6 @@ public class VrShellDelegate
if (mActivity == activity) return;
if (mInVr) shutdownVr(disableVrMode, false /* stayingInChrome */);
mActivity = activity;
mListeningForWebVrActivateBeforePause = false;
}
private void maybeUpdateVrSupportLevel() {
......@@ -1181,30 +1182,18 @@ public class VrShellDelegate
*/
private boolean enterVrAfterDon() {
if (mNativeVrShellDelegate == 0) return false;
if (!canEnterVr(true)) return false;
// If the page is listening for vrdisplayactivate we assume it wants to request
// presentation. Go into WebVR mode tentatively. If the page doesn't request presentation
// in the vrdisplayactivate handler we will exit presentation later.
// Note that we don't want to dispatch vrdisplayactivate for auto-present and vr intents.
boolean tentativeWebVrMode =
mListeningForWebVrActivateBeforePause && !mRequestedWebVr && !mEnterVrOnStartup;
if (tentativeWebVrMode && !mAutopresentWebVr) {
// Before we fire DisplayActivate, we need focus to propagate to the WebContents we're
// about to send DisplayActivate to. Focus propagates during onResume, which is when
// this function is called, so if we post DisplayActivate to fire after onResume, focus
// will have propagated.
assert !mPaused;
new Handler().post(new Runnable() {
@Override
public void run() {
if (mNativeVrShellDelegate == 0) return;
nativeDisplayActivate(mNativeVrShellDelegate);
}
});
}
enterVr(tentativeWebVrMode);
if (!canEnterVr()) return false;
// If headset insertion was performed while a page was listening for vrdisplayactivate,
// we assume it wants to request presentation. Go into WebVR mode tentatively. If the page
// doesn't request presentation in the vrdisplayactivate handler we will exit presentation
// later.
if (mActivateFromHeadsetInsertion) {
assert !mRequestedWebVr;
assert !mEnterVrOnStartup;
assert !mAutopresentWebVr;
}
enterVr(mActivateFromHeadsetInsertion);
mEnterVrOnStartup = false;
// The user has successfully completed a DON flow.
......@@ -1493,14 +1482,14 @@ public class VrShellDelegate
}
}
/* package */ boolean canEnterVr(boolean justCompletedDon) {
/* package */ boolean canEnterVr() {
if (!LibraryLoader.isInitialized()) return false;
if (mVrSupportLevel == VrSupportLevel.VR_NOT_AVAILABLE || mNativeVrShellDelegate == 0)
return false;
// If vr shell is not enabled and this is not a web vr request, then return false.
boolean presenting = mRequestedWebVr || mListeningForWebVrActivate
|| (justCompletedDon && mListeningForWebVrActivateBeforePause) || mAutopresentWebVr;
|| mActivateFromHeadsetInsertion || mAutopresentWebVr;
if (!isVrBrowsingEnabled() && !presenting) return false;
return true;
}
......@@ -1538,7 +1527,7 @@ public class VrShellDelegate
// Update VR support level as it can change at runtime
maybeUpdateVrSupportLevel();
if (mVrSupportLevel == VrSupportLevel.VR_NOT_AVAILABLE) return ENTER_VR_CANCELLED;
if (!canEnterVr(false)) return ENTER_VR_CANCELLED;
if (!canEnterVr()) return ENTER_VR_CANCELLED;
if (mVrSupportLevel == VrSupportLevel.VR_DAYDREAM && isDaydreamCurrentViewerInternal()) {
// TODO(mthiesse): This is a workaround for b/66486878 (see also crbug.com/767594).
// We have to trigger the DON flow before setting VR mode enabled to prevent the DON
......@@ -1574,7 +1563,6 @@ public class VrShellDelegate
exitWebVRPresent();
mAutopresentWebVr = false;
mRequestedWebVr = false;
mListeningForWebVrActivateBeforePause = false;
}
@CalledByNative
......@@ -1628,6 +1616,7 @@ public class VrShellDelegate
protected void onResume() {
if (DEBUG_LOGS) Log.i(TAG, "onResume");
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR1) return;
mMaybeActivateAfterHeadsetInsertion = false;
if (maybeCloseVrCct()) return;
if (mNeedsAnimationCancel) {
// At least on some devices, like the Samsung S8+, a Window animation is run after our
......@@ -1723,6 +1712,7 @@ public class VrShellDelegate
if (mInVr) {
maybeSetPresentResult(true, mDonSucceeded);
mDonSucceeded = false;
assert !mActivateFromHeadsetInsertion;
return;
}
if (maybeExitVrToUpdateVrServices()) return;
......@@ -1767,7 +1757,8 @@ public class VrShellDelegate
// page becomes visible again after DON finished. So here we remember the value of
// mListeningForWebVrActivity before pause and use this value to decide if
// vrdisplayactivate event should be dispatched in enterVRFromIntent.
mListeningForWebVrActivateBeforePause = mListeningForWebVrActivate;
mMaybeActivateAfterHeadsetInsertion =
mListeningForWebVrActivate && !mInVr && !mRequestedWebVr;
if (mInVr) mVrShell.pause();
if (mNativeVrShellDelegate != 0) nativeOnPause(mNativeVrShellDelegate);
......@@ -1777,6 +1768,7 @@ public class VrShellDelegate
private void onStart() {
if (maybeCloseVrCct()) return;
mMaybeActivateAfterHeadsetInsertion = false;
mStopped = false;
if (mDonSucceeded) setWindowModeForVr();
if (mInVr) {
......@@ -1888,26 +1880,24 @@ public class VrShellDelegate
// mListeningForWebVrActivate for them.
if (mVrSupportLevel != VrSupportLevel.VR_DAYDREAM) return;
mListeningForWebVrActivate = listening;
if (mPaused) return;
if (listening) {
registerDaydreamIntent(mActivity);
if (mAutopresentWebVr) {
if (mAutopresentWebVr || mActivateFromHeadsetInsertion) {
// Dispatch vrdisplayactivate so that the WebVr page can call requestPresent
// to start presentation.
// TODO(ymalik): There will be a delay between when we're asked to autopresent and
// when the WebVr site calls requestPresent. In this time, the user sees 2D Chrome
// UI which is suboptimal.
nativeDisplayActivate(mNativeVrShellDelegate);
}
} else if (!canEnterVr(false)) {
} else if (!canEnterVr()) {
unregisterDaydreamIntent();
}
mActivateFromHeadsetInsertion = false;
}
private void cancelPendingVrEntry() {
if (DEBUG_LOGS) Log.i(TAG, "cancelPendingVrEntry");
removeBlackOverlayView(mActivity);
mDonSucceeded = false;
mActivateFromHeadsetInsertion = false;
maybeSetPresentResult(false, false);
if (!mShowingDaydreamDoff) {
setVrModeEnabled(mActivity, false);
......
device/vr/vr_device_base.cc
View file @ f2ccbfd1
......@@ -94,8 +94,6 @@ void VRDeviceBase::RemoveDisplay(VRDisplayImpl* display) {
listening_for_activate_diplay_ = nullptr;
OnListeningForActivate(false);
}
if (last_listening_for_activate_diplay_ == display)
last_listening_for_activate_diplay_ = nullptr;
}
bool VRDeviceBase::IsAccessAllowed(VRDisplayImpl* display) {
......@@ -135,10 +133,6 @@ void VRDeviceBase::OnActivate(mojom::VRDisplayEventReason reason,
base::Callback<void(bool)> on_handled) {
if (listening_for_activate_diplay_) {
listening_for_activate_diplay_->OnActivate(reason, std::move(on_handled));
} else if (last_listening_for_activate_diplay_ &&
last_listening_for_activate_diplay_->InFocusedFrame()) {
last_listening_for_activate_diplay_->OnActivate(reason,
std::move(on_handled));
} else {
std::move(on_handled).Run(true /* will_not_present */);
}
......@@ -158,7 +152,6 @@ void VRDeviceBase::UpdateListeningForActivate(VRDisplayImpl* display) {
if (!was_listening)
OnListeningForActivate(true);
} else if (listening_for_activate_diplay_ == display) {
last_listening_for_activate_diplay_ = listening_for_activate_diplay_;
listening_for_activate_diplay_ = nullptr;
OnListeningForActivate(false);
}
......
device/vr/vr_device_base.h
View file @ f2ccbfd1
......@@ -70,19 +70,6 @@ class DEVICE_VR_EXPORT VRDeviceBase : public VRDevice {
VRDisplayImpl* presenting_display_ = nullptr;
VRDisplayImpl* listening_for_activate_diplay_ = nullptr;
// On Android display activate is triggered after the Device ON flow that
// pauses Chrome, which unfocuses the webvr page, which lets us know that that
// page is no longer listening to displayActivate. We then have a race between
// blink-side getting focus back and letting us know the page is listening for
// displayactivate, and the browser sending displayactivate.
// We resolve this by remembering which display was last listening for
// displayactivate most recently, and sending the activation there so long as
// the WebContents it belongs to is focused and nothing has more recently
// started listening for displayactivate.
// This is safe because if the page is /actually/ not listening for activate
// anymore, the displayactivate signal will just be ignored.
VRDisplayImpl* last_listening_for_activate_diplay_ = nullptr;
mojom::VRDisplayInfoPtr display_info_;
unsigned int id_;
......
device/vr/vr_device_base_unittest.cc
View file @ f2ccbfd1
......@@ -126,7 +126,7 @@ TEST_F(VRDeviceTest, DisplayActivateRegsitered) {
device->OnListeningForActivateChanged(display2.get());
EXPECT_TRUE(device->ListeningForActivate());
EXPECT_CALL(*display2, OnActivate(mounted, testing::_)).Times(3);
EXPECT_CALL(*display2, OnActivate(mounted, testing::_)).Times(2);
device->FireDisplayActivate();
EXPECT_CALL(*display1, ListeningForActivate())
......@@ -141,15 +141,13 @@ TEST_F(VRDeviceTest, DisplayActivateRegsitered) {
device->OnListeningForActivateChanged(display2.get());
EXPECT_FALSE(device->ListeningForActivate());
// Even though the device says it's not listening for activate, we still send
// it the activation to handle raciness on Android.
// Make sure we don't send the DisplayActivate event.
device->FireDisplayActivate();
EXPECT_CALL(*display2, InFocusedFrame())
.WillRepeatedly(testing::Return(false));
device->OnFrameFocusChanged(display2.get());
// Now we no longer fire the activation.
device->FireDisplayActivate();
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment