Make disabling magic window a cross-platform concept + add regression test.

Moves the concept of disabling magic window poses into VRDeviceBase.

Bug: 777868
Change-Id: I5ab317faf5ad04abb47558e041446cfc1a484da0
Reviewed-on: https://chromium-review.googlesource.com/776165
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: David Dorwin <ddorwin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518369}

chrome/browser/android/vr_shell/vr_shell_delegate.cc

@@ -79,9 +79,12 @@ VrShellDelegate* VrShellDelegate::GetNativeVrShellDelegate(
 void VrShellDelegate::SetDelegate(VrShell* vr_shell,
                                   gvr::ViewerType viewer_type) {
   vr_shell_ = vr_shell;
-  device::GvrDevice* device = static_cast<device::GvrDevice*>(GetDevice());
+  device::VRDevice* device = GetDevice();
+  // When VrShell is created, we disable magic window mode as the user is inside
+  // the headset. As currently implemented, orientation-based magic window
+  // doesn't make sense when the window is fixed and the user is moving.
   if (device)
-    device->SetInBrowsingMode(true);
+    device->SetMagicWindowEnabled(false);
 
   if (pending_successful_present_request_) {
     CHECK(!present_callback_.is_null());
@@ -94,9 +97,9 @@ void VrShellDelegate::SetDelegate(VrShell* vr_shell,
 
 void VrShellDelegate::RemoveDelegate() {
   vr_shell_ = nullptr;
-  device::GvrDevice* device = static_cast<device::GvrDevice*>(GetDevice());
+  device::VRDevice* device = GetDevice();
   if (device) {
-    device->SetInBrowsingMode(false);
+    device->SetMagicWindowEnabled(true);
     device->OnExitPresent();
   }
 }
@@ -177,9 +180,12 @@ void VrShellDelegate::Destroy(JNIEnv* env, const JavaParamRef<jobject>& obj) {
 void VrShellDelegate::SetDeviceId(unsigned int device_id) {
   device_id_ = device_id;
   if (vr_shell_) {
-    device::GvrDevice* device = static_cast<device::GvrDevice*>(GetDevice());
+    device::VRDevice* device = GetDevice();
+    // See comment in VrShellDelegate::SetDelegate. This handles the case where
+    // VrShell is created before the device/ code is initialized (like when
+    // entering VR browsing on a non-webVR page).
     if (device)
-      device->SetInBrowsingMode(true);
+      device->SetMagicWindowEnabled(false);
   }
 }
 

device/vr/android/gvr/gvr_device.cc

@@ -180,12 +180,8 @@ void GvrDevice::ExitPresent() {
   OnExitPresent();
 }
 
-void GvrDevice::GetPose(
+void GvrDevice::OnMagicWindowPoseRequest(
     mojom::VRMagicWindowProvider::GetPoseCallback callback) {
-  if (in_browsing_mode_) {
-    std::move(callback).Run(nullptr);
-    return;
-  }
   std::move(callback).Run(
       GvrDelegate::GetVRPosePtrWithNeckModel(gvr_api_.get(), nullptr));
 }

device/vr/android/gvr/gvr_device.h

@@ -32,7 +32,6 @@ class DEVICE_VR_EXPORT GvrDevice : public VRDeviceBase {
       mojom::VRPresentationProviderRequest request,
       mojom::VRDisplayHost::RequestPresentCallback callback) override;
   void ExitPresent() override;
-  void GetPose(mojom::VRMagicWindowProvider::GetPoseCallback callback) override;
   void PauseTracking() override;
   void ResumeTracking() override;
 
@@ -42,13 +41,12 @@ class DEVICE_VR_EXPORT GvrDevice : public VRDeviceBase {
   void Activate(mojom::VRDisplayEventReason reason,
                 base::Callback<void(bool)> on_handled);
 
-  // TODO(mthiesse): Make this functionality cross-platform.
-  void SetInBrowsingMode(bool in_browsing_mode) {
-    in_browsing_mode_ = in_browsing_mode;
-  }
-
  private:
+  // VRDeviceBase
   void OnListeningForActivate(bool listening) override;
+  void OnMagicWindowPoseRequest(
+      mojom::VRMagicWindowProvider::GetPoseCallback callback) override;
+
   void OnRequestPresentResult(
       mojom::VRDisplayHost::RequestPresentCallback callback,
       VRDisplayImpl* display,
@@ -59,7 +57,6 @@ class DEVICE_VR_EXPORT GvrDevice : public VRDeviceBase {
 
   base::android::ScopedJavaGlobalRef<jobject> non_presenting_context_;
   std::unique_ptr<gvr::GvrApi> gvr_api_;
-  bool in_browsing_mode_ = false;
 
   base::WeakPtrFactory<GvrDevice> weak_ptr_factory_;
 

device/vr/openvr/openvr_device.cc

@@ -189,7 +189,7 @@ void OpenVRDevice::ExitPresent() {
       base::Bind(&OpenVRRenderLoop::ExitPresent, render_loop_->GetWeakPtr()));
 }
 
-void OpenVRDevice::GetPose(
+void OpenVRDevice::OnMagicWindowPoseRequest(
     mojom::VRMagicWindowProvider::GetPoseCallback callback) {
   vr::TrackedDevicePose_t rendering_poses[vr::k_unMaxTrackedDeviceCount];
   vr_system_->GetDeviceToAbsoluteTrackingPose(vr::TrackingUniverseSeated, 0.03f,

device/vr/openvr/openvr_device.h

@@ -33,7 +33,6 @@ class OpenVRDevice : public VRDeviceBase {
       mojom::VRPresentationProviderRequest request,
       mojom::VRDisplayHost::RequestPresentCallback callback) override;
   void ExitPresent() override;
-  void GetPose(mojom::VRMagicWindowProvider::GetPoseCallback callback) override;
 
   void OnPollingEvents();
 
@@ -42,6 +41,10 @@ class OpenVRDevice : public VRDeviceBase {
       bool result);
 
  private:
+  // VRDeviceBase
+  void OnMagicWindowPoseRequest(
+      mojom::VRMagicWindowProvider::GetPoseCallback callback) override;
+
   // TODO (BillOrr): This should not be a unique_ptr because the render_loop_
   // binds to VRVSyncProvider requests, so its lifetime should be tied to the
   // lifetime of that binding.

device/vr/test/fake_vr_device.cc

@@ -62,4 +62,9 @@ void FakeVRDevice::ExitPresent() {
   OnExitPresent();
 }
 
+void FakeVRDevice::OnMagicWindowPoseRequest(
+    mojom::VRMagicWindowProvider::GetPoseCallback callback) {
+  std::move(callback).Run(pose_.Clone());
+}
+
 }  // namespace device

device/vr/test/fake_vr_device.h

@@ -26,10 +26,17 @@ class DEVICE_VR_EXPORT FakeVRDevice : public VRDeviceBase {
       mojom::VRDisplayHost::RequestPresentCallback callback) override;
   void ExitPresent() override;
 
+  void SetPose(mojom::VRPosePtr pose) { pose_ = std::move(pose); }
+
  private:
+  void OnMagicWindowPoseRequest(
+      mojom::VRMagicWindowProvider::GetPoseCallback callback) override;
+
   mojom::VRDisplayInfoPtr InitBasicDevice();
   mojom::VREyeParametersPtr InitEye(float fov, float offset, uint32_t size);
 
+  mojom::VRPosePtr pose_;
+
   DISALLOW_COPY_AND_ASSIGN(FakeVRDevice);
 };
 

device/vr/vr_device.h

@@ -27,6 +27,7 @@ class DEVICE_VR_EXPORT VRDevice {
   virtual void Blur() = 0;
   virtual void Focus() = 0;
   virtual mojom::VRDisplayInfoPtr GetVRDisplayInfo() = 0;
+  virtual void SetMagicWindowEnabled(bool enabled) = 0;
 
   // TODO(mthiesse): The browser should handle browser-side exiting of
   // presentation before device/ is even aware presentation is being exited.

device/vr/vr_device_base.cc

@@ -63,9 +63,18 @@ void VRDeviceBase::ExitPresent() {
   NOTREACHED();
 }
 
-void VRDeviceBase::GetPose(
+void VRDeviceBase::SetMagicWindowEnabled(bool enabled) {
+  magic_window_enabled_ = enabled;
+}
+
+void VRDeviceBase::GetMagicWindowPose(
     mojom::VRMagicWindowProvider::GetPoseCallback callback) {
+  if (!magic_window_enabled_) {
     std::move(callback).Run(nullptr);
+    return;
+  }
+
+  OnMagicWindowPoseRequest(std::move(callback));
 }
 
 void VRDeviceBase::AddDisplay(VRDisplayImpl* display) {
@@ -132,6 +141,11 @@ void VRDeviceBase::OnActivate(mojom::VRDisplayEventReason reason,
 
 void VRDeviceBase::OnListeningForActivate(bool listening) {}
 
+void VRDeviceBase::OnMagicWindowPoseRequest(
+    mojom::VRMagicWindowProvider::GetPoseCallback callback) {
+  std::move(callback).Run(nullptr);
+}
+
 void VRDeviceBase::UpdateListeningForActivate(VRDisplayImpl* display) {
   if (display->ListeningForActivate() && display->InFocusedFrame()) {
     bool was_listening = !!listening_for_activate_diplay_;

device/vr/vr_device_base.h

@@ -31,6 +31,7 @@ class DEVICE_VR_EXPORT VRDeviceBase : public VRDevice {
   void Focus() override;
   void OnExitPresent() override;
   mojom::VRDisplayInfoPtr GetVRDisplayInfo() final;
+  void SetMagicWindowEnabled(bool enabled) final;
 
   virtual void RequestPresent(
       VRDisplayImpl* display,
@@ -38,7 +39,6 @@ class DEVICE_VR_EXPORT VRDeviceBase : public VRDevice {
       mojom::VRPresentationProviderRequest request,
       mojom::VRDisplayHost::RequestPresentCallback callback);
   virtual void ExitPresent();
-  virtual void GetPose(mojom::VRMagicWindowProvider::GetPoseCallback callback);
 
   void AddDisplay(VRDisplayImpl* display);
   void RemoveDisplay(VRDisplayImpl* display);
@@ -46,6 +46,8 @@ class DEVICE_VR_EXPORT VRDeviceBase : public VRDevice {
   bool CheckPresentingDisplay(VRDisplayImpl* display);
   void OnListeningForActivateChanged(VRDisplayImpl* display);
   void OnFrameFocusChanged(VRDisplayImpl* display);
+  void GetMagicWindowPose(
+      mojom::VRMagicWindowProvider::GetPoseCallback callback);
 
   VRDisplayImpl* GetPresentingDisplay() { return presenting_display_; }
 
@@ -55,10 +57,11 @@ class DEVICE_VR_EXPORT VRDeviceBase : public VRDevice {
   void OnActivate(mojom::VRDisplayEventReason reason,
                   base::Callback<void(bool)> on_handled);
 
-  virtual void OnListeningForActivate(bool listening);
-
  private:
   void UpdateListeningForActivate(VRDisplayImpl* display);
+  virtual void OnListeningForActivate(bool listening);
+  virtual void OnMagicWindowPoseRequest(
+      mojom::VRMagicWindowProvider::GetPoseCallback callback);
 
   std::set<VRDisplayImpl*> displays_;
 
@@ -81,8 +84,8 @@ class DEVICE_VR_EXPORT VRDeviceBase : public VRDevice {
   mojom::VRDisplayInfoPtr display_info_;
 
   unsigned int id_;
-
   static unsigned int next_id_;
+  bool magic_window_enabled_ = true;
 
   DISALLOW_COPY_AND_ASSIGN(VRDeviceBase);
 };

device/vr/vr_device_base_unittest.cc

@@ -8,6 +8,7 @@
 
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
+#include "device/vr/test/fake_vr_device.h"
 #include "device/vr/test/fake_vr_service_client.h"
 #include "device/vr/test/mock_vr_display_impl.h"
 #include "device/vr/vr_device_base.h"
@@ -155,4 +156,15 @@ TEST_F(VRDeviceTest, DisplayActivateRegsitered) {
   device->FireDisplayActivate();
 }
 
+TEST_F(VRDeviceTest, NoMagicWindowPosesWhileBrowsing) {
+  auto device = std::make_unique<FakeVRDevice>();
+  device->SetPose(mojom::VRPose::New());
+
+  device->GetMagicWindowPose(
+      base::BindOnce([](mojom::VRPosePtr pose) { EXPECT_TRUE(pose); }));
+  device->SetMagicWindowEnabled(false);
+  device->GetMagicWindowPose(
+      base::BindOnce([](mojom::VRPosePtr pose) { EXPECT_FALSE(pose); }));
+}
+
 }  // namespace device

device/vr/vr_display_impl.cc

@@ -74,12 +74,13 @@ void VRDisplayImpl::ExitPresent() {
     device_->ExitPresent();
 }
 
+// Gets a pose for magic window sessions.
 void VRDisplayImpl::GetPose(GetPoseCallback callback) {
   if (!device_->IsAccessAllowed(this)) {
     std::move(callback).Run(nullptr);
     return;
   }
-  device_->GetPose(std::move(callback));
+  device_->GetMagicWindowPose(std::move(callback));
 }
 
 void VRDisplayImpl::SetListeningForActivate(bool listening) {

device/vr/vr_display_impl.h

@@ -52,6 +52,7 @@ class DEVICE_VR_EXPORT VRDisplayImpl : public mojom::VRMagicWindowProvider {
   void ExitPresent();
 
  private:
+  // mojom::VRMagicWindowProvider
   void GetPose(GetPoseCallback callback) override;
 
   mojo::Binding<mojom::VRMagicWindowProvider> binding_;