Refactor XRVisibilityState plumbing

Ensure that newly created sessions get the current XRVisibilityState,
even if they were created after that visibility state may have changed.

Bug: 1011566
Change-Id: I5eac2aac73f74a7e4b4426be3b5e6603097db39b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1841246Reviewed-by: Klaus Weidner <klausw@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Commit-Queue: Alexander Cooper <alcooper@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703365}

chrome/browser/vr/service/vr_service_impl.cc

@@ -299,6 +299,7 @@ void VRServiceImpl::OnSessionCreated(
     session->enabled_features.push_back(feature);
   }
 
+  client->OnVisibilityStateChanged(visibility_state_);
   session_clients_.Add(std::move(client));
 
   std::move(callback).Run(
@@ -581,6 +582,7 @@ void VRServiceImpl::OnExitPresent() {
 
 void VRServiceImpl::OnVisibilityStateChanged(
     device::mojom::XRVisibilityState visiblity_state) {
+  visibility_state_ = visiblity_state;
   for (auto& client : session_clients_)
     client->OnVisibilityStateChanged(visiblity_state);
 }

chrome/browser/vr/service/vr_service_impl.h

@@ -157,6 +157,8 @@ class VR_EXPORT VRServiceImpl : public device::mojom::VRService,
   content::RenderFrameHost* render_frame_host_;
   mojo::SelfOwnedReceiverRef<VRService> receiver_;
   InterfaceSet<device::mojom::XRSessionControllerPtr> magic_window_controllers_;
+  device::mojom::XRVisibilityState visibility_state_ =
+      device::mojom::XRVisibilityState::VISIBLE;
 
   // List of callbacks to run when initialization is completed.
   std::vector<base::OnceCallback<void()>> pending_requests_;

device/vr/windows/compositor_base.cc

@@ -215,6 +215,14 @@ void XRCompositorCommon::RequestSession(
 
   on_visibility_state_changed_ = std::move(on_visibility_state_changed);
 
+  // Queue up a notification to the requester of the current visibility state,
+  // so that it can be initialized to the right value.
+  if (on_visibility_state_changed_) {
+    main_thread_task_runner_->PostTask(
+        FROM_HERE,
+        base::BindOnce(on_visibility_state_changed_, visibility_state_));
+  }
+
   device::mojom::XRPresentationTransportOptionsPtr transport_options =
       device::mojom::XRPresentationTransportOptions::New();
   transport_options->transport_method =
@@ -274,12 +282,15 @@ void XRCompositorCommon::ExitPresent() {
   }
 }
 
-void XRCompositorCommon::VisibilityStateChanged(
+void XRCompositorCommon::SetVisibilityState(
     mojom::XRVisibilityState visibility_state) {
-  if (on_visibility_state_changed_) {
-    main_thread_task_runner_->PostTask(
-        FROM_HERE,
-        base::BindOnce(on_visibility_state_changed_, visibility_state));
+  if (visibility_state_ != visibility_state) {
+    visibility_state_ = visibility_state;
+    if (on_visibility_state_changed_) {
+      main_thread_task_runner_->PostTask(
+          FROM_HERE,
+          base::BindOnce(on_visibility_state_changed_, visibility_state));
+    }
   }
 }
 
@@ -385,9 +396,17 @@ void XRCompositorCommon::GetControllerDataAndSendFrameData(
   // Update gamepad controllers.
   UpdateControllerState();
 
+  // This method represents a call from the renderer process. If our visibility
+  // state is hidden, we should avoid handing "sensitive" information, like the
+  // pose back up to the renderer. Note that this check is done here as other
+  // methods (RequestNextOverlayPose) represent a call from the browser process,
+  // which should receive the pose.
+  bool is_visible =
+      (visibility_state_ != device::mojom::XRVisibilityState::HIDDEN);
+
   // We have posted a message to allow other calls to get through, and now state
   // may have changed.  WebXR may not be presenting any more, or may be hidden.
-  std::move(callback).Run(is_presenting_ &&
+  std::move(callback).Run(is_presenting_ && is_visible &&
                                   (webxr_visible_ || on_webxr_submitted_)
                               ? std::move(frame_data)
                               : mojom::XRFrameData::New());

device/vr/windows/compositor_base.h

@@ -62,7 +62,6 @@ class XRCompositorCommon : public base::Thread,
                       mojom::XRRuntimeSessionOptionsPtr options,
                       RequestSessionCallback callback);
   void ExitPresent();
-  void VisibilityStateChanged(mojom::XRVisibilityState visibility_state);
 
   void GetFrameData(mojom::XRFrameDataRequestOptionsPtr options,
                     XRFrameDataProvider::GetFrameDataCallback callback) final;
@@ -82,6 +81,7 @@ class XRCompositorCommon : public base::Thread,
 
  protected:
   virtual bool UsesInputEventing();
+  void SetVisibilityState(mojom::XRVisibilityState visibility_state);
 #if defined(OS_WIN)
   D3D11TextureHelper texture_helper_;
 #endif
@@ -183,6 +183,8 @@ class XRCompositorCommon : public base::Thread,
   mojo::Receiver<mojom::XRFrameDataProvider> frame_data_receiver_{this};
   mojo::Binding<mojom::IsolatedXRGamepadProvider> gamepad_provider_;
   mojo::Binding<mojom::ImmersiveOverlay> overlay_binding_;
+  mojom::XRVisibilityState visibility_state_ =
+      mojom::XRVisibilityState::VISIBLE;
 
   DISALLOW_COPY_AND_ASSIGN(XRCompositorCommon);
 };

device/vr/windows_mixed_reality/mixed_reality_renderloop.cc

@@ -231,7 +231,7 @@ bool MixedRealityRenderLoop::StartRuntime() {
       holographic_space_->AddUserPresenceChangedCallback(
           base::BindRepeating(&MixedRealityRenderLoop::OnUserPresenceChanged,
                               base::Unretained(this)));
-  UpdateVisiblityState();
+  UpdateVisibilityState();
 
   input_helper_ = std::make_unique<MixedRealityInputHelper>(
       window_->hwnd(), weak_ptr_factory_.GetWeakPtr());
@@ -410,39 +410,30 @@ void MixedRealityRenderLoop::OnUserPresenceChanged() {
   task_runner()->PostTask(FROM_HERE,
                           base::BindOnce(
                               [](MixedRealityRenderLoop* render_loop) {
-                                render_loop->UpdateVisiblityState();
+                                render_loop->UpdateVisibilityState();
                               },
                               base::Unretained(this)));
 }
 
-void MixedRealityRenderLoop::UpdateVisiblityState() {
-  HolographicSpaceUserPresence user_presence =
-      holographic_space_->UserPresence();
-
-  device::mojom::XRVisibilityState new_state;
-  switch (user_presence) {
+void MixedRealityRenderLoop::UpdateVisibilityState() {
+  switch (holographic_space_->UserPresence()) {
     // Indicates that the browsers immersive content is visible in the headset
     // receiving input, and the headset is being worn.
     case HolographicSpaceUserPresence::
         HolographicSpaceUserPresence_PresentActive:
-      new_state = device::mojom::XRVisibilityState::VISIBLE;
-      break;
+      SetVisibilityState(device::mojom::XRVisibilityState::VISIBLE);
+      return;
     // Indicates that the browsers immersive content is visible in the headset
     // and the headset is being worn, but a modal dialog is capturing input.
     case HolographicSpaceUserPresence::
         HolographicSpaceUserPresence_PresentPassive:
-      new_state = device::mojom::XRVisibilityState::VISIBLE_BLURRED;
-      break;
+      SetVisibilityState(device::mojom::XRVisibilityState::VISIBLE_BLURRED);
+      return;
     // Indicates that the browsers immersive content is not visible in the
     // headset or the user is not wearing the headset.
     case HolographicSpaceUserPresence::HolographicSpaceUserPresence_Absent:
-      new_state = device::mojom::XRVisibilityState::HIDDEN;
-      break;
-  }
-
-  if (visibility_state != new_state) {
-    visibility_state = new_state;
-    VisibilityStateChanged(visibility_state);
+      SetVisibilityState(device::mojom::XRVisibilityState::HIDDEN);
+      return;
   }
 }
 
@@ -809,12 +800,6 @@ mojom::XRFrameDataPtr MixedRealityRenderLoop::GetNextFrameData() {
   mojom::XRFrameDataPtr ret =
       CreateDefaultFrameData(timestamp_.get(), next_frame_id_);
 
-  // If the device isn't currently showing content from this render loop, don't
-  // deliver complete frame data.
-  if (visibility_state == device::mojom::XRVisibilityState::HIDDEN) {
-    return ret;
-  }
-
   if ((!attached_ && !anchor_origin_) || !pose_) {
     TRACE_EVENT_INSTANT0("xr", "No origin or no pose",
                          TRACE_EVENT_SCOPE_THREAD);

device/vr/windows_mixed_reality/mixed_reality_renderloop.h

@@ -80,7 +80,7 @@ class MixedRealityRenderLoop : public XRCompositorCommon {
   void OnCurrentStageChanged();
 
   void OnUserPresenceChanged();
-  void UpdateVisiblityState();
+  void UpdateVisibilityState();
 
   // Will try to update the stage bounds if the following are true:
   // 1) We have a spatial_stage.
@@ -128,9 +128,6 @@ class MixedRealityRenderLoop : public XRCompositorCommon {
   std::vector<gfx::Point3F> bounds_;
   bool bounds_updated_ = false;
 
-  device::mojom::XRVisibilityState visibility_state =
-      device::mojom::XRVisibilityState::HIDDEN;
-
   // This must be the last member
   base::WeakPtrFactory<MixedRealityRenderLoop> weak_ptr_factory_{this};