Skip to content

Commit

Permalink
VR: Disable app button when browsing mode is disabled.
Browse files Browse the repository at this point in the history
Pressing the app button would try to exit WebVR, which would trigger
a non-optional DOFF, which is a really bad user experience. The button
should just be disabled when it can't actually take you back to the
VR browser.

TBR=mthiesse@chromium.org

(cherry picked from commit a0cdfa5)

Bug: 767896
Change-Id: If31a7e8959293efb48618ec40a3cd9e132e92961
Reviewed-on: https://chromium-review.googlesource.com/718986
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509072}
Reviewed-on: https://chromium-review.googlesource.com/726481
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#54}
Cr-Branched-From: adb61db-refs/heads/master@{#508578}
  • Loading branch information
Michael Thiessen committed Oct 18, 2017
1 parent 2dcf849 commit b2c1eea
Show file tree
Hide file tree
Showing 10 changed files with 65 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ private static void unregisterDaydreamIntent(VrDaydreamApi daydreamApi) {
/**
* @return Whether or not VR Shell is currently enabled.
*/
private static boolean isVrShellEnabled(int vrSupportLevel) {
/* package */ static boolean isVrShellEnabled(int vrSupportLevel) {
// Only enable ChromeVR (VrShell) on Daydream devices as it currently needs a Daydream
// controller.
if (vrSupportLevel != VR_DAYDREAM) return false;
Expand Down Expand Up @@ -709,6 +709,11 @@ private void updateVrSupportLevel(Integer vrCorePackageVersion) {
mVrSupportLevel = supportLevel;
}

@VrSupportLevel
/* package */ int getVrSupportLevel() {
return mVrSupportLevel;
}

private void onVrServicesMaybeUpdated() {
int vrCorePackageVersion = getVrCorePackageVersion();
if (mCachedVrCorePackageVersion == vrCorePackageVersion) return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,10 +328,11 @@ public void initializeNative(Tab currentTab, boolean forWebVr,
float displayHeightMeters = (dm.heightPixels / dm.ydpi) * INCHES_TO_METERS;

mContentVrWindowAndroid = new VrWindowAndroid(mActivity, mContentVirtualDisplay);
boolean browsingDisabled = !VrShellDelegate.isVrShellEnabled(mDelegate.getVrSupportLevel());
mNativeVrShell = nativeInit(mDelegate, mContentVrWindowAndroid.getNativePointer(), forWebVr,
webVrAutopresentationExpected, inCct, getGvrApi().getNativeGvrContext(),
mReprojectedRendering, displayWidthMeters, displayHeightMeters, dm.widthPixels,
dm.heightPixels);
webVrAutopresentationExpected, inCct, browsingDisabled,
getGvrApi().getNativeGvrContext(), mReprojectedRendering, displayWidthMeters,
displayHeightMeters, dm.widthPixels, dm.heightPixels);

reparentAllTabs(mContentVrWindowAndroid);
swapToTab(currentTab);
Expand Down Expand Up @@ -825,9 +826,10 @@ public View getPresentationViewForTesting() {
}

private native long nativeInit(VrShellDelegate delegate, long nativeWindowAndroid,
boolean forWebVR, boolean webVrAutopresentationExpected, boolean inCct, long gvrApi,
boolean reprojectedRendering, float displayWidthMeters, float displayHeightMeters,
int displayWidthPixels, int displayHeightPixels);
boolean forWebVR, boolean webVrAutopresentationExpected, boolean inCct,
boolean browsingDisabled, long gvrApi, boolean reprojectedRendering,
float displayWidthMeters, float displayHeightMeters, int displayWidthPixels,
int displayHeightPixels);
private native void nativeSetSurface(long nativeVrShell, Surface surface);
private native void nativeSwapContents(
long nativeVrShell, Tab tab, AndroidUiGestureTarget androidUiGestureTarget);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.chromium.base.test.util.MinAndroidSdkLevel;
import org.chromium.base.test.util.Restriction;
import org.chromium.base.test.util.RetryOnFailure;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.vr_shell.util.VrTestRuleUtils;
import org.chromium.chrome.browser.vr_shell.util.VrTransitionUtils;
Expand Down Expand Up @@ -211,6 +212,25 @@ public void testAppButtonExitsPresentation() throws InterruptedException {
POLL_TIMEOUT_SHORT_MS, mVrTestFramework.getFirstTabWebContents()));
}

/**
* Verifies that pressing the Daydream controller's 'app' button does not cause the user to exit
* WebVR presentation when VR browsing is disabled.
*/
@Test
@MediumTest
@Restriction(RESTRICTION_TYPE_VIEWER_DAYDREAM)
@CommandLineFlags.Add({"disable-features=" + ChromeFeatureList.VR_SHELL})
public void testAppButtonNoopsWhenBrowsingDisabled() throws InterruptedException {
mVrTestFramework.loadUrlAndAwaitInitialization(
VrTestFramework.getHtmlTestFile("generic_webvr_page"), PAGE_LOAD_TIMEOUT_S);
VrTransitionUtils.enterPresentationOrFail(mVrTestFramework.getFirstTabCvc());
EmulatedVrController controller = new EmulatedVrController(mVrTestRule.getActivity());
controller.pressReleaseAppButton();
Assert.assertFalse("App button exited WebVR presentation",
VrTestFramework.pollJavaScriptBoolean("!vrDisplay.isPresenting",
POLL_TIMEOUT_SHORT_MS, mVrTestFramework.getFirstTabWebContents()));
}

/**
* Tests that focus loss updates synchronously.
*/
Expand Down
17 changes: 3 additions & 14 deletions chrome/browser/android/vr_shell/vr_gl_thread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "chrome/browser/android/vr_shell/vr_shell_gl.h"
#include "chrome/browser/vr/browser_ui_interface.h"
#include "chrome/browser/vr/toolbar_state.h"
#include "chrome/browser/vr/ui.h"
#include "third_party/skia/include/core/SkBitmap.h"

namespace vr_shell {
Expand All @@ -20,18 +19,14 @@ VrGLThread::VrGLThread(
const base::WeakPtr<VrShell>& weak_vr_shell,
scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner,
gvr_context* gvr_api,
bool initially_web_vr,
bool web_vr_autopresentation_expected,
bool in_cct,
const vr::UiInitialState& ui_initial_state,
bool reprojected_rendering,
bool daydream_support)
: base::android::JavaHandlerThread("VrShellGL"),
weak_vr_shell_(weak_vr_shell),
main_thread_task_runner_(std::move(main_thread_task_runner)),
gvr_api_(gvr_api),
initially_web_vr_(initially_web_vr),
web_vr_autopresentation_expected_(web_vr_autopresentation_expected),
in_cct_(in_cct),
ui_initial_state_(ui_initial_state),
reprojected_rendering_(reprojected_rendering),
daydream_support_(daydream_support) {}

Expand All @@ -44,14 +39,8 @@ base::WeakPtr<VrShellGl> VrGLThread::GetVrShellGl() {
}

void VrGLThread::Init() {
vr::UiInitialState ui_initial_state;
ui_initial_state.in_cct = in_cct_;
ui_initial_state.in_web_vr = initially_web_vr_;
ui_initial_state.web_vr_autopresentation_expected =
web_vr_autopresentation_expected_;

vr_shell_gl_ =
base::MakeUnique<VrShellGl>(this, this, ui_initial_state, gvr_api_,
base::MakeUnique<VrShellGl>(this, this, ui_initial_state_, gvr_api_,
reprojected_rendering_, daydream_support_);

browser_ui_ = vr_shell_gl_->GetBrowserUiWeakPtr();
Expand Down
9 changes: 3 additions & 6 deletions chrome/browser/android/vr_shell/vr_gl_thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "base/single_thread_task_runner.h"
#include "chrome/browser/android/vr_shell/gl_browser_interface.h"
#include "chrome/browser/vr/browser_ui_interface.h"
#include "chrome/browser/vr/ui.h"
#include "chrome/browser/vr/ui_browser_interface.h"
#include "chrome/browser/vr/ui_interface.h"
#include "third_party/gvr-android-sdk/src/libraries/headers/vr/gvr/capi/include/gvr_types.h"
Expand All @@ -31,9 +32,7 @@ class VrGLThread : public base::android::JavaHandlerThread,
const base::WeakPtr<VrShell>& weak_vr_shell,
scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner,
gvr_context* gvr_api,
bool initially_web_vr,
bool web_vr_autopresentation_expected,
bool in_cct,
const vr::UiInitialState& ui_initial_state,
bool reprojected_rendering,
bool daydream_support);

Expand Down Expand Up @@ -94,9 +93,7 @@ class VrGLThread : public base::android::JavaHandlerThread,
// This state is used for initializing vr_shell_gl_.
scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_;
gvr_context* gvr_api_;
bool initially_web_vr_;
bool web_vr_autopresentation_expected_;
bool in_cct_;
vr::UiInitialState ui_initial_state_;
bool reprojected_rendering_;
bool daydream_support_;

Expand Down
23 changes: 15 additions & 8 deletions chrome/browser/android/vr_shell/vr_shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,7 @@ void SetIsInVR(content::WebContents* contents, bool is_in_vr) {
VrShell::VrShell(JNIEnv* env,
const JavaParamRef<jobject>& obj,
ui::WindowAndroid* window,
bool for_web_vr,
bool web_vr_autopresentation_expected,
bool in_cct,
const vr::UiInitialState& ui_initial_state,
VrShellDelegate* delegate,
gvr_context* gvr_api,
bool reprojected_rendering,
Expand All @@ -147,15 +145,16 @@ VrShell::VrShell(JNIEnv* env,

gl_thread_ = base::MakeUnique<VrGLThread>(
weak_ptr_factory_.GetWeakPtr(), main_thread_task_runner_, gvr_api,
for_web_vr, web_vr_autopresentation_expected, in_cct,
reprojected_rendering_, HasDaydreamSupport(env));
ui_initial_state, reprojected_rendering_, HasDaydreamSupport(env));
ui_ = gl_thread_.get();
toolbar_ = base::MakeUnique<vr::ToolbarHelper>(ui_, this);

gl_thread_->Start();

if (for_web_vr || web_vr_autopresentation_expected)
UMA_HISTOGRAM_BOOLEAN("VRAutopresentedWebVR", !for_web_vr);
if (ui_initial_state.in_web_vr ||
ui_initial_state.web_vr_autopresentation_expected) {
UMA_HISTOGRAM_BOOLEAN("VRAutopresentedWebVR", !ui_initial_state.in_web_vr);
}
}

void VrShell::Destroy(JNIEnv* env, const JavaParamRef<jobject>& obj) {
Expand Down Expand Up @@ -868,15 +867,23 @@ jlong Init(JNIEnv* env,
jboolean for_web_vr,
jboolean web_vr_autopresentation_expected,
jboolean in_cct,
jboolean browsing_disabled,
jlong gvr_api,
jboolean reprojected_rendering,
jfloat display_width_meters,
jfloat display_height_meters,
jint display_width_pixels,
jint display_pixel_height) {
vr::UiInitialState ui_initial_state;
ui_initial_state.browsing_disabled = browsing_disabled;
ui_initial_state.in_cct = in_cct;
ui_initial_state.in_web_vr = for_web_vr;
ui_initial_state.web_vr_autopresentation_expected =
web_vr_autopresentation_expected;

return reinterpret_cast<intptr_t>(new VrShell(
env, obj, reinterpret_cast<ui::WindowAndroid*>(window_android),
for_web_vr, web_vr_autopresentation_expected, in_cct,
ui_initial_state,
VrShellDelegate::GetNativeVrShellDelegate(env, delegate),
reinterpret_cast<gvr_context*>(gvr_api), reprojected_rendering,
display_width_meters, display_height_meters, display_width_pixels,
Expand Down
5 changes: 2 additions & 3 deletions chrome/browser/android/vr_shell/vr_shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "base/single_thread_task_runner.h"
#include "chrome/browser/ui/toolbar/chrome_toolbar_model_delegate.h"
#include "chrome/browser/vr/exit_vr_prompt_choice.h"
#include "chrome/browser/vr/ui.h"
#include "chrome/browser/vr/ui_unsupported_mode.h"
#include "content/public/browser/web_contents_observer.h"
#include "device/geolocation/public/interfaces/geolocation_config.mojom.h"
Expand Down Expand Up @@ -70,9 +71,7 @@ class VrShell : device::GvrGamepadDataProvider,
VrShell(JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj,
ui::WindowAndroid* window,
bool for_web_vr,
bool web_vr_autopresentation_expected,
bool in_cct,
const vr::UiInitialState& ui_initial_state,
VrShellDelegate* delegate,
gvr_context* gvr_api,
bool reprojected_rendering,
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/vr/ui.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ struct UiInitialState {
bool in_cct = false;
bool in_web_vr = false;
bool web_vr_autopresentation_expected = false;
bool browsing_disabled = false;
};

// This class manages all GLThread owned objects and GL rendering for VrShell.
Expand Down
7 changes: 6 additions & 1 deletion chrome/browser/vr/ui_scene_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ UiSceneManager::UiSceneManager(UiBrowserInterface* browser,
started_for_autopresentation_(
ui_initial_state.web_vr_autopresentation_expected),
showing_web_vr_splash_screen_(
ui_initial_state.web_vr_autopresentation_expected) {
ui_initial_state.web_vr_autopresentation_expected),
browsing_disabled_(ui_initial_state.browsing_disabled) {
Create2dBrowsingSubtreeRoots();
CreateWebVrRoot();
CreateBackground();
Expand Down Expand Up @@ -797,6 +798,10 @@ void UiSceneManager::OnAppButtonClicked() {
if (started_for_autopresentation_)
return;

// If browsing mode is disabled, the app button should no-op.
if (browsing_disabled_)
return;

// App button click exits the WebVR presentation and fullscreen.
browser_->ExitPresent();
browser_->ExitFullscreen();
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/vr/ui_scene_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ class UiSceneManager {
bool showing_web_vr_splash_screen_ = false;
bool prompting_to_exit_ = false;
bool exiting_ = false;
bool browsing_disabled_ = false;

bool fullscreen_ = false;
bool incognito_ = false;
Expand Down

0 comments on commit b2c1eea

Please sign in to comment.