Skip to content

Commit

Permalink
Send vrdisplayactivate to the most recently focused navigator listeni…
Browse files Browse the repository at this point in the history
…ng for displayactivate.

Since focus changes are reported late in the Android lifecycle, we can end up in a situation where we send vrdisplayactivate, but no webVR frames are considered focused yet.

This change makes it so that even if no webVR frame is currently focused, we still send displayactivate to the most recently focused frame (which has the added benefit of ensuring only one frame can call requestPresent in response). This is safe because we only fire displayactivate at all if we are sure some frame was listening for displayactivate before the Android VR DON flow paused chrome and unfocused the frame.

In the future we should track focus fully browser-side to more securely solve this issue.

This CL also prevents a possible reconnect loop if the page mistakenly thinks it has focus when it doesn't by detecting repeated connection failures.

BUG=706472

Review-Url: https://codereview.chromium.org/2783993002
Cr-Commit-Position: refs/heads/master@{#460558}
  • Loading branch information
mthiesse authored and Commit bot committed Mar 29, 2017
1 parent 8c2a4e8 commit b6d4321
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 10 deletions.
4 changes: 0 additions & 4 deletions device/vr/android/gvr/gvr_device_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,6 @@ device::GvrDelegateProvider* GvrDeviceProvider::GetDelegateProvider() {
}

void GvrDeviceProvider::Initialize() {
// TODO(mthiesse): Clean up how we connect the GvrDelegateProvider to the
// GvrDeviceProvider so we don't have to call this function multiple times.
// Ideally the DelegateProvider would always be available, and GetInstance()
// would create it.
Initialize(device::GvrDelegateProvider::GetInstance());
}

Expand Down
16 changes: 14 additions & 2 deletions device/vr/vr_device_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ void VRDeviceManager::AddService(VRServiceImpl* service) {
void VRDeviceManager::RemoveService(VRServiceImpl* service) {

if (service->listening_for_activate()) {
ListeningForActivateChanged(false);
ListeningForActivateChanged(false, service);
}

services_.erase(service);
Expand All @@ -112,9 +112,14 @@ unsigned int VRDeviceManager::GetNumberOfConnectedDevices() {
return static_cast<unsigned int>(devices_.size());
}

void VRDeviceManager::ListeningForActivateChanged(bool listening) {
void VRDeviceManager::ListeningForActivateChanged(bool listening,
VRServiceImpl* service) {
DCHECK(thread_checker_.CalledOnValidThread());

if (listening) {
most_recently_listening_for_activate_ = service;
}

bool activate_listeners = listening;
if (!activate_listeners) {
for (auto* service : services_) {
Expand All @@ -133,6 +138,13 @@ void VRDeviceManager::ListeningForActivateChanged(bool listening) {
}
}

bool VRDeviceManager::IsMostRecentlyListeningForActivate(
VRServiceImpl* service) {
if (!service)
return false;
return service == most_recently_listening_for_activate_;
}

VRDevice* VRDeviceManager::GetDevice(unsigned int index) {
DCHECK(thread_checker_.CalledOnValidThread());

Expand Down
5 changes: 4 additions & 1 deletion device/vr/vr_device_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ class VRDeviceManager {

DEVICE_VR_EXPORT unsigned int GetNumberOfConnectedDevices();

void ListeningForActivateChanged(bool listening);
void ListeningForActivateChanged(bool listening, VRServiceImpl* service);

bool IsMostRecentlyListeningForActivate(VRServiceImpl* service);

private:
friend class VRDeviceManagerTest;
Expand Down Expand Up @@ -77,6 +79,7 @@ class VRDeviceManager {
bool vr_initialized_;

std::set<VRServiceImpl*> services_;
VRServiceImpl* most_recently_listening_for_activate_ = nullptr;

// For testing. If true will not delete self when consumer count reaches 0.
bool keep_alive_;
Expand Down
4 changes: 4 additions & 0 deletions device/vr/vr_display_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "base/bind.h"
#include "device/vr/vr_device.h"
#include "device/vr/vr_device_manager.h"
#include "device/vr/vr_service_impl.h"

namespace device {
Expand Down Expand Up @@ -45,6 +46,9 @@ void VRDisplayImpl::OnFocus() {
}

void VRDisplayImpl::OnActivate(mojom::VRDisplayEventReason reason) {
VRDeviceManager* manager = VRDeviceManager::GetInstance();
if (!manager->IsMostRecentlyListeningForActivate(service_))
return;
client_->OnActivate(reason);
}

Expand Down
2 changes: 1 addition & 1 deletion device/vr/vr_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ void VRServiceImpl::ConnectDevice(VRDevice* device) {
void VRServiceImpl::SetListeningForActivate(bool listening) {
listening_for_activate_ = listening;
VRDeviceManager* device_manager = VRDeviceManager::GetInstance();
device_manager->ListeningForActivateChanged(listening);
device_manager->ListeningForActivateChanged(listening, this);
}

// Creates a VRDisplayPtr unique to this service so that the associated page can
Expand Down
6 changes: 4 additions & 2 deletions third_party/WebKit/Source/modules/vr/VRDisplay.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -681,8 +681,6 @@ void VRDisplay::stopPresenting() {
}

void VRDisplay::OnActivate(device::mojom::blink::VRDisplayEventReason reason) {
if (!m_navigatorVR->isFocused() || m_displayBlurred)
return;
AutoReset<bool> activating(&m_inDisplayActivate, true);
m_navigatorVR->dispatchVREvent(VRDisplayEvent::create(
EventTypeNames::vrdisplayactivate, true, false, this, reason));
Expand Down Expand Up @@ -713,6 +711,7 @@ void VRDisplay::OnVSync(device::mojom::blink::VRPosePtr pose,
mojo::common::mojom::blink::TimeDeltaPtr time,
int16_t frameId,
device::mojom::blink::VRVSyncProvider::Status error) {
m_VSyncConnectionFailed = false;
switch (error) {
case device::mojom::blink::VRVSyncProvider::Status::SUCCESS:
break;
Expand Down Expand Up @@ -760,7 +759,10 @@ void VRDisplay::ConnectVSyncProvider() {

void VRDisplay::OnVSyncConnectionError() {
m_vrVSyncProvider.reset();
if (m_VSyncConnectionFailed)
return;
ConnectVSyncProvider();
m_VSyncConnectionFailed = true;
}

ScriptedAnimationController& VRDisplay::ensureScriptedAnimationController(
Expand Down
1 change: 1 addition & 0 deletions third_party/WebKit/Source/modules/vr/VRDisplay.h
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ class VRDisplay final : public EventTargetWithInlineData,
double m_timebase = -1;
bool m_pendingPreviousFrameRender = false;
bool m_pendingSubmitFrame = false;
bool m_VSyncConnectionFailed = false;

device::mojom::blink::VRDisplayPtr m_display;

Expand Down

0 comments on commit b6d4321

Please sign in to comment.