Isolate a presenting VR device from pages other than the one presenting.

device/vr/vr_device_manager.cc

Index: device/vr/vr_device_manager.cc
diff --git a/device/vr/vr_device_manager.cc b/device/vr/vr_device_manager.cc
index b774ad248b762c7018e36f3040eded2d48ba6e7d..a16d035e83dd62a6af7ce1850adbca9d86822f72 100644
--- a/device/vr/vr_device_manager.cc
+++ b/device/vr/vr_device_manager.cc
@@ -22,7 +22,11 @@ VRDeviceManager* g_vr_device_manager = nullptr;
 }
 
 VRDeviceManager::VRDeviceManager()
-    : vr_initialized_(false), keep_alive_(false), has_scheduled_poll_(false) {
+    : vr_initialized_(false),
+      presenting_service_(nullptr),
+      presenting_device_(nullptr),
+      keep_alive_(false),
+      has_scheduled_poll_(false) {
 // Register VRDeviceProviders for the current platform
 #if defined(OS_ANDROID)
   RegisterProvider(base::WrapUnique(new GvrDeviceProvider()));
@@ -30,7 +34,11 @@ VRDeviceManager::VRDeviceManager()
 }
 
 VRDeviceManager::VRDeviceManager(std::unique_ptr<VRDeviceProvider> provider)
-    : vr_initialized_(false), keep_alive_(true), has_scheduled_poll_(false) {
+    : vr_initialized_(false),
+      presenting_service_(nullptr),
+      presenting_device_(nullptr),
+      keep_alive_(true),
+      has_scheduled_poll_(false) {
   thread_checker_.DetachFromThread();
   RegisterProvider(std::move(provider));
   SetInstance(this);
@@ -48,6 +56,26 @@ VRDeviceManager* VRDeviceManager::GetInstance() {
   return g_vr_device_manager;
 }
 
+// Returns the requested device with the requested id if the specified service
+// is allowed to access it.
+VRDevice* VRDeviceManager::GetAllowedDevice(VRServiceImpl* service,
+                                            unsigned int index) {
+  VRDeviceManager* device_manager = GetInstance();
+
+  // If another service is presenting to the requested device don't allow other
+  // services to access it. That could potentially allow them to spy on
+  // where the user is looking on another page, spam another application with
+  // pose resets, etc.
+  if (device_manager->presenting_service_ &&
+      device_manager->presenting_service_ != service) {
+    if (device_manager->presenting_device_ &&
+        device_manager->presenting_device_->id() == index)
+      return nullptr;
+  }
+
+  return device_manager->GetDevice(index);
+}
+
 void VRDeviceManager::SetInstance(VRDeviceManager* instance) {
   // Unit tests can create multiple instances but only one should exist at any
   // given time so g_vr_device_manager should only go from nullptr to
@@ -72,6 +100,13 @@ void VRDeviceManager::RemoveService(VRServiceImpl* service) {
   services_.erase(std::remove(services_.begin(), services_.end(), service),
                   services_.end());
 
+  if (service == presenting_service_) {

[mthiesse, 2016/09/13 18:46:15]

With these checks peppered all over the place, it might make sense to check this
at GetInstance() instead, and not allow the wrong service to get an instance of
this class at all. That would probably be much easier to maintain in the long
term.

[bajones, 2016/09/13 19:42:57]

I see your point (and I already did something similar with GetAllowedDevice) but
there's valid reasons for a service to access this class even if it's not
presenting. (Removing itself from the manager when the associated page closes is
one example.)

[mthiesse, 2016/09/13 20:01:16]

Is there something preventing you from only restricting access at GetInstance()
if somebody is already presenting?

Something like:
if (presenting_service_) {
  if (service != presenting_service_)
    return nullptr;
}
return instance;


This obviously won't work if clients can hold onto instances of this class and
call its functions at some point in the future, but it looks like that isn't the
case?

+    presenting_device_->ExitPresent();
+
+    presenting_service_ = nullptr;
+    presenting_device_ = nullptr;
+  }
+
   if (services_.empty() && !keep_alive_) {
     // Delete the device manager when it has no active connections.
     delete g_vr_device_manager;
@@ -127,6 +162,81 @@ void VRDeviceManager::OnDeviceChanged(VRDisplayPtr device) {
     service->client()->OnDisplayChanged(device.Clone());
 }
 
+bool VRDeviceManager::RequestPresent(VRServiceImpl* service,
+                                     unsigned int index) {
+  // Is anything presenting currently?
+  if (presenting_service_) {
+    // Fail if the currently presenting service is not the one making the
+    // request.
+    if (presenting_service_ != service)
+      return false;
+
+    // If we are switching presentation from the currently presenting service to
+    // a new device stop presening to the previous one.
+    if (presenting_device_ && presenting_device_->id() != index) {

[mthiesse, 2016/09/13 18:46:15]

Is it possible for there to be a presenting service but no presenting device?
Maybe DCHECK for this?

[bajones, 2016/09/13 19:42:57]

Good Call. Done.

+      // Tell the device to stop presenting.
+      presenting_device_->ExitPresent();
+
+      // Only the presenting service needs to be notified that presentation is
+      // ending on the previous device.
+      presenting_service_->client()->OnExitPresent(presenting_device_->id());
+      presenting_device_ = nullptr;
+    }
+
+    presenting_service_ = nullptr;
+  }
+
+  VRDevice* requested_device = GetDevice(index);
+  // Can't present to a device that doesn't exist.
+  if (!requested_device)
+    return false;
+
+  // Attempt to begin presenting to this device. This could fail for any number
+  // of device-specific reasons.
+  if (!requested_device->RequestPresent())
+    return false;
+
+  // Successfully began presenting!
+  presenting_service_ = service;
+  presenting_device_ = requested_device;
+
+  return true;
+}
+
+void VRDeviceManager::ExitPresent(VRServiceImpl* service, unsigned int index) {
+  // Don't allow services other than the currently presenting one to exit
+  // presentation.
+  if (presenting_service_ != service)
+    return;
+
+  // Fail if the specified device is not currently presenting.
+  if (!presenting_device_ || presenting_device_->id() != index)
+    return;
+
+  // Tell the device to stop presenting.
+  presenting_device_->ExitPresent();
+  presenting_service_->client()->OnExitPresent(index);
+
+  // Clear the presenting service and device.
+  presenting_service_ = nullptr;
+  presenting_device_ = nullptr;
+}
+
+void VRDeviceManager::SubmitFrame(VRServiceImpl* service,
+                                  unsigned int index,
+                                  VRPosePtr pose) {
+  // Don't allow services other than the currently presenting one to submit any
+  // frames.
+  if (presenting_service_ != service)
+    return;
+
+  // Don't submit frames to devices other than the currently presenting one.
+  if (!presenting_device_ || presenting_device_->id() != index)
+    return;
+
+  presenting_device_->SubmitFrame(std::move(pose));
+}
+
 void VRDeviceManager::InitializeProviders() {
   if (vr_initialized_) {
     return;