Skip to content

Commit

Permalink
VR: Check focus state browser-side before sending input to renderer.
Browse files Browse the repository at this point in the history
Unfortunately we have to check focus twice before sending it to a VrDisplay,
once in the browser to do view-level focus, then again in the renderer to do
frame-level focus.

This CL ensures browser-side focus before sending displayactivate events
or head pose information to the WebVR page.

This CL adds an argument to OnWebContentsFocused to let observers know
which RenderWidgetHost gained/lost focus. It also implements GotFocus for
Android, meaning OnWebContentsFocused will now be called on Android where
it previously wasn't.

LostFocus will now be called significantly less, as I've made it symmetric
with GotFocus (which involved adding the fullscreen restriction to
WebContentsImpl::RenderWidgetLostFocus that's present in
WebContentsImpl::RenderWidgetGotFocus)

Bug: 687411, 738239
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Ibeee427d602d7fef8ce533669b45d9cc5f8ffa03
Reviewed-on: https://chromium-review.googlesource.com/565760
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488300}
  • Loading branch information
Michael Thiessen authored and Commit Bot committed Jul 20, 2017
1 parent 2cda996 commit 896405d
Show file tree
Hide file tree
Showing 55 changed files with 430 additions and 189 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1552,6 +1552,17 @@ public void setFeedbackFrequencyForTesting(int frequency) {
mFeedbackFrequency = frequency;
}

@VisibleForTesting
public boolean isListeningForWebVrActivate() {
return mListeningForWebVrActivate;
}

@VisibleForTesting
public boolean isClearActivatePending() {
assert mNativeVrShellDelegate != 0;
return nativeIsClearActivatePending(mNativeVrShellDelegate);
}

/**
* @return Pointer to the native VrShellDelegate object.
*/
Expand Down Expand Up @@ -1585,5 +1596,6 @@ private native void nativeUpdateVSyncInterval(
private native void nativeOnPause(long nativeVrShellDelegate);
private native void nativeOnResume(long nativeVrShellDelegate);
private native void nativeUpdateNonPresentingContext(long nativeVrShellDelegate, long context);
private native boolean nativeIsClearActivatePending(long nativeVrShellDelegate);
private native void nativeDestroy(long nativeVrShellDelegate);
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
package org.chromium.chrome.browser.vr_shell;

import static org.chromium.chrome.browser.vr_shell.VrTestRule.PAGE_LOAD_TIMEOUT_S;
import static org.chromium.chrome.browser.vr_shell.VrTestRule.POLL_CHECK_INTERVAL_SHORT_MS;
import static org.chromium.chrome.browser.vr_shell.VrTestRule.POLL_TIMEOUT_LONG_MS;
import static org.chromium.chrome.browser.vr_shell.VrTestRule.POLL_TIMEOUT_SHORT_MS;
import static org.chromium.chrome.test.util.ChromeRestriction.RESTRICTION_TYPE_VIEWER_DAYDREAM;
import static org.chromium.chrome.test.util.ChromeRestriction.RESTRICTION_TYPE_VIEWER_NON_DAYDREAM;
Expand All @@ -17,6 +19,7 @@
import org.junit.Test;
import org.junit.runner.RunWith;

import org.chromium.base.ThreadUtils;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.DisableIf;
import org.chromium.base.test.util.MinAndroidSdkLevel;
Expand All @@ -27,6 +30,8 @@
import org.chromium.chrome.browser.vr_shell.util.VrTransitionUtils;
import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.content.browser.test.util.Criteria;
import org.chromium.content.browser.test.util.CriteriaHelper;

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -151,4 +156,30 @@ public void testAppButtonExitsPresentation() throws InterruptedException {
mVrTestRule.pollJavaScriptBoolean("!vrDisplay.isPresenting", POLL_TIMEOUT_SHORT_MS,
mVrTestRule.getFirstTabWebContents()));
}

/**
* Tests that focus loss updates synchronously.
*/
@Test
@MediumTest
@Restriction(RESTRICTION_TYPE_VIEWER_DAYDREAM)
public void testFocusUpdatesSynchronously() throws InterruptedException {
mVrTestRule.loadUrlAndAwaitInitialization(
VrTestRule.getHtmlTestFile("generic_webvr_page_with_activate_listener"),
PAGE_LOAD_TIMEOUT_S);

CriteriaHelper.pollUiThread(new Criteria("DisplayActivate was never registered.") {
@Override
public boolean isSatisfied() {
return VrShellDelegate.getInstanceForTesting().isListeningForWebVrActivate();
}
}, POLL_TIMEOUT_LONG_MS, POLL_CHECK_INTERVAL_SHORT_MS);
ThreadUtils.runOnUiThreadBlocking(new Runnable() {
@Override
public void run() {
mVrTestRule.getActivity().getCurrentContentViewCore().onPause();
Assert.assertTrue(VrShellDelegate.getInstanceForTesting().isClearActivatePending());
}
});
}
}
133 changes: 129 additions & 4 deletions chrome/browser/android/vr_shell/vr_shell_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,16 @@

#include "base/android/jni_android.h"
#include "base/callback_helpers.h"
#include "base/memory/ptr_util.h"
#include "chrome/browser/android/vr_shell/vr_metrics_util.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_widget_host.h"
#include "content/public/browser/render_widget_host_view.h"
#include "content/public/browser/web_contents_observer.h"
#include "device/vr/android/gvr/gvr_delegate.h"
#include "device/vr/android/gvr/gvr_device.h"
#include "device/vr/android/gvr/gvr_device_provider.h"
#include "device/vr/vr_display_impl.h"
#include "jni/VrShellDelegate_jni.h"
#include "third_party/gvr-android-sdk/src/libraries/headers/vr/gvr/capi/include/gvr.h"

Expand All @@ -21,8 +27,35 @@ using base::android::ScopedJavaLocalRef;

namespace vr_shell {

namespace {

content::RenderFrameHost* getHostForDisplay(device::VRDisplayImpl* display) {
return content::RenderFrameHost::FromID(display->ProcessId(),
display->RoutingId());
}

} // namespace

class DelegateWebContentsObserver : public content::WebContentsObserver {
public:
DelegateWebContentsObserver(VrShellDelegate* delegate,
content::WebContents* contents)
: content::WebContentsObserver(contents), delegate_(delegate) {}

private:
void OnWebContentsFocused(content::RenderWidgetHost* host) override {
delegate_->OnWebContentsFocused(host);
}
void OnWebContentsLostFocus(content::RenderWidgetHost* host) override {
delegate_->OnWebContentsLostFocus(host);
}

VrShellDelegate* delegate_;
};

VrShellDelegate::VrShellDelegate(JNIEnv* env, jobject obj)
: weak_ptr_factory_(this) {
: task_runner_(base::ThreadTaskRunnerHandle::Get()),
weak_ptr_factory_(this) {
DVLOG(1) << __FUNCTION__ << "=" << this;
j_vr_shell_delegate_.Reset(env, obj);
}
Expand Down Expand Up @@ -104,8 +137,8 @@ void VrShellDelegate::SetPresentResult(JNIEnv* env,

void VrShellDelegate::DisplayActivate(JNIEnv* env,
const JavaParamRef<jobject>& obj) {
if (device_provider_) {
device_provider_->Device()->OnActivate(
if (activatable_display_) {
activatable_display_->OnActivate(
device::mojom::VRDisplayEventReason::MOUNTED,
base::Bind(&VrShellDelegate::OnActivateDisplayHandled,
weak_ptr_factory_.GetWeakPtr()));
Expand Down Expand Up @@ -147,6 +180,11 @@ void VrShellDelegate::UpdateNonPresentingContext(
gvr_api_ = gvr::GvrApi::WrapNonOwned(reinterpret_cast<gvr_context*>(context));
}

bool VrShellDelegate::IsClearActivatePending(JNIEnv* env,
const JavaParamRef<jobject>& obj) {
return !clear_activate_task_.IsCancelled();
}

void VrShellDelegate::Destroy(JNIEnv* env, const JavaParamRef<jobject>& obj) {
delete this;
}
Expand Down Expand Up @@ -213,15 +251,102 @@ device::GvrDelegate* VrShellDelegate::GetDelegate() {
return gvr_delegate_;
}

void VrShellDelegate::OnDisplayAdded(device::VRDisplayImpl* display) {
content::RenderFrameHost* host = getHostForDisplay(display);
if (host == nullptr)
return;
content::WebContents* web_contents =
content::WebContents::FromRenderFrameHost(host);
CHECK(web_contents);
content::RenderWidgetHost* render_widget_host =
host->GetView()->GetRenderWidgetHost();
displays_.emplace(render_widget_host, display);
observers_.emplace(display, base::MakeUnique<DelegateWebContentsObserver>(
this, web_contents));
if (host->GetView()->HasFocus())
OnWebContentsFocused(render_widget_host);
}

void VrShellDelegate::OnDisplayRemoved(device::VRDisplayImpl* display) {
if (activatable_display_ == display) {
SetListeningForActivate(false);
activatable_display_ = nullptr;
}
for (auto it = displays_.begin(); it != displays_.end();) {
if (it->second == display) {
it = displays_.erase(it);
} else {
++it;
}
}
auto it = observers_.find(display);
if (it != observers_.end())
observers_.erase(it);
}

void VrShellDelegate::OnListeningForActivateChanged(
device::VRDisplayImpl* display) {
content::RenderFrameHost* host = getHostForDisplay(display);
bool has_focus = host != nullptr && host->GetView()->HasFocus();
if (display->ListeningForActivate() && has_focus) {
OnFocusedAndActivatable(display);
} else {
if (activatable_display_ != display)
return;
OnLostFocusedAndActivatable();
}
}

void VrShellDelegate::OnWebContentsFocused(content::RenderWidgetHost* host) {
auto it = displays_.find(host);
if (it == displays_.end())
return;
if (!it->second->ListeningForActivate())
return;
OnFocusedAndActivatable(it->second);
}

void VrShellDelegate::OnWebContentsLostFocus(content::RenderWidgetHost* host) {
auto it = displays_.find(host);
if (it == displays_.end())
return;
if (activatable_display_ != it->second)
return;
if (!it->second->ListeningForActivate())
return;
OnLostFocusedAndActivatable();
}

void VrShellDelegate::OnFocusedAndActivatable(device::VRDisplayImpl* display) {
activatable_display_ = display;
SetListeningForActivate(true);
clear_activate_task_.Cancel();
}

void VrShellDelegate::OnLostFocusedAndActivatable() {
// We post here to ensure that this runs after Android finishes running all
// onPause handlers. This allows us to capture the pre-paused state during
// onPause in java, so we know that the pause is the cause of the focus loss,
// and that the page is still listening for activate.
clear_activate_task_.Reset(
base::Bind(&VrShellDelegate::SetListeningForActivate,
weak_ptr_factory_.GetWeakPtr(), false));
task_runner_->PostTask(FROM_HERE, clear_activate_task_.callback());
}

void VrShellDelegate::SetListeningForActivate(bool listening) {
clear_activate_task_.Cancel();
JNIEnv* env = AttachCurrentThread();
Java_VrShellDelegate_setListeningForWebVrActivate(
env, j_vr_shell_delegate_.obj(), listening);
}

void VrShellDelegate::GetNextMagicWindowPose(
device::VRDisplayImpl* display,
device::mojom::VRDisplay::GetNextMagicWindowPoseCallback callback) {
if (!gvr_api_ || gvr_delegate_) {
content::RenderFrameHost* host = getHostForDisplay(display);
if (!gvr_api_ || gvr_delegate_ || host == nullptr ||
!host->GetView()->HasFocus()) {
std::move(callback).Run(nullptr);
return;
}
Expand Down
28 changes: 27 additions & 1 deletion chrome/browser/android/vr_shell/vr_shell_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,32 @@

#include <jni.h>

#include <map>
#include <memory>

#include "base/android/jni_weak_ref.h"
#include "base/callback.h"
#include "base/cancelable_callback.h"
#include "base/macros.h"
#include "chrome/browser/android/vr_shell/vr_core_info.h"
#include "chrome/browser/android/vr_shell/vr_usage_monitor.h"
#include "device/vr/android/gvr/gvr_delegate_provider.h"
#include "device/vr/vr_service.mojom.h"
#include "third_party/gvr-android-sdk/src/libraries/headers/vr/gvr/capi/include/gvr_types.h"

namespace content {
class RenderWidgetHost;
}

namespace device {
class GvrDelegate;
class GvrDeviceProvider;
}

namespace vr_shell {

class DelegateWebContentsObserver;

class VrShellDelegate : public device::GvrDelegateProvider {
public:
VrShellDelegate(JNIEnv* env, jobject obj);
Expand Down Expand Up @@ -53,10 +61,15 @@ class VrShellDelegate : public device::GvrDelegateProvider {
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj,
jlong context);
bool IsClearActivatePending(JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
void Destroy(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);

device::GvrDeviceProvider* device_provider() { return device_provider_; }

void OnWebContentsFocused(content::RenderWidgetHost* host);
void OnWebContentsLostFocus(content::RenderWidgetHost* host);

// device::GvrDelegateProvider implementation.
void ExitWebVRPresent() override;
void CreateVRDisplayInfo(
Expand All @@ -71,12 +84,18 @@ class VrShellDelegate : public device::GvrDelegateProvider {
device::mojom::VRPresentationProviderRequest request,
const base::Callback<void(bool)>& callback) override;
device::GvrDelegate* GetDelegate() override;
void SetListeningForActivate(bool listening) override;
void OnDisplayAdded(device::VRDisplayImpl* display) override;
void OnDisplayRemoved(device::VRDisplayImpl* display) override;
void OnListeningForActivateChanged(device::VRDisplayImpl* display) override;
void GetNextMagicWindowPose(
device::VRDisplayImpl* display,
device::mojom::VRDisplay::GetNextMagicWindowPoseCallback callback)
override;

void OnActivateDisplayHandled(bool will_not_present);
void OnFocusedAndActivatable(device::VRDisplayImpl* display);
void OnLostFocusedAndActivatable();
void SetListeningForActivate(bool listening);

std::unique_ptr<VrCoreInfo> MakeVrCoreInfo(JNIEnv* env);

Expand All @@ -91,6 +110,13 @@ class VrShellDelegate : public device::GvrDelegateProvider {
bool pending_successful_present_request_ = false;
std::unique_ptr<gvr::GvrApi> gvr_api_;

std::map<content::RenderWidgetHost*, device::VRDisplayImpl*> displays_;
std::map<device::VRDisplayImpl*, std::unique_ptr<DelegateWebContentsObserver>>
observers_;
device::VRDisplayImpl* activatable_display_ = nullptr;
base::CancelableClosure clear_activate_task_;
scoped_refptr<base::SingleThreadTaskRunner> task_runner_;

base::WeakPtrFactory<VrShellDelegate> weak_ptr_factory_;

DISALLOW_COPY_AND_ASSIGN(VrShellDelegate);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<!doctype html>
<!--
Tests that scanning the Daydream View NFC tag on supported devices fires the
vrdisplayactivate event
-->
<html>
<head>
<link rel="stylesheet" type="text/css" href="../resources/webvr_e2e.css">
</head>
<body>
<canvas id="webgl-canvas"></canvas>
<script src="../../../../../../third_party/WebKit/LayoutTests/resources/testharness.js"></script>
<script src="../resources/webvr_e2e.js"></script>
<script>
window.addEventListener("vrdisplayactivate", () => {}, false);
</script>
</body>
</html>
4 changes: 2 additions & 2 deletions content/browser/android/content_view_core.cc
Original file line number Diff line number Diff line change
Expand Up @@ -739,9 +739,9 @@ void ContentViewCore::SetFocusInternal(bool focused) {
return;

if (focused)
GetRenderWidgetHostViewAndroid()->Focus();
GetRenderWidgetHostViewAndroid()->GotFocus();
else
GetRenderWidgetHostViewAndroid()->Blur();
GetRenderWidgetHostViewAndroid()->LostFocus();
}

void ContentViewCore::SendOrientationChangeEvent(
Expand Down
Loading

0 comments on commit 896405d

Please sign in to comment.