Ensure inline rAFs are not run if an immersive session was created

Because a task is queued to call XRSession::OnFrame for any inline
sessions during XRFrameProvider::ProcessScheduledFrame, it is possible
for an immersive session to be created while we are waiting for the
frame to be served.

This creates an "interceptor" method "OnPreDispatchInlineFrame", which
can ensure that there is still no immersive session when providing the
frame to the XRSession. This "interceptor" method is needed, rather than
simply adding the check in XRSession for two reasons:

1) It should not be the responsibility of the XRSession to verify that
it should have received the frame.
2) Since the frame is being blocked, the session needs to be added back
into the requesting_sessions_ list to prevent the frame loop from timing
out.

Fixed: 1116261
Change-Id: I61699b65ccb073f709321cbd9f812cf6931bd72b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2363704Reviewed-by: Brandon Jones <bajones@chromium.org>
Commit-Queue: Alexander Cooper <alcooper@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799637}

chrome/test/data/xr/e2e_test_files/html/test_non_immersive_stops_during_immersive.html

@@ -16,7 +16,6 @@ is active, but resumes afterwards.
       setup({single_test: true});
 
       let counter = 0;
-      let nonImmersiveDuringImmersiveCounter = 0;
 
       function stepBeforeImmersive() {
         onMagicWindowXRFrameCallback = function() {
@@ -24,17 +23,8 @@ is active, but resumes afterwards.
           // are not done while there is an immersive session.
           onMagicWindowXRFrameCallback = function() {
             if (sessionInfos[sessionTypes.IMMERSIVE].currentSession !== null) {
-              // TODO(https://crbug.com/1116261): Investigate a proper product
-              // fix so that this can be removed.
-              // The inline rAF loop should not be started back again until we
-              // have been notified that the immersive session ended; however,
-              // a non-immersive rAF may have already been scheduled and not be
-              // cancellable once the immersive session returns, leading to a
-              // race. To this end, we will allow for at most one non-immersive
-              // frame to be called while we have an immersive session.
-              nonImmersiveDuringImmersiveCounter++;
-              assert_less_than_equal(nonImmersiveDuringImmersiveCounter, 1,
-                  "Non-immersive rAF called during immersive session more than allowed");
+              assert_unreached(
+                  "Non-immersive rAF called during immersive session");
             }
           }
           finishJavaScriptStep();

third_party/blink/renderer/modules/xr/xr_frame_provider.cc

@@ -539,14 +539,42 @@ void XRFrameProvider::ProcessScheduledFrame(
       // Run session->OnFrame() in a posted task to ensure that createAnchor
       // promises get a chance to run - the presentation frame state is already
       // updated.
+      // Note that rather than call session->OnFrame() directly, we dispatch to
+      // a helper method who can determine if the state requirements are still
+      // met that would allow the frame to be served.
       frame->GetTaskRunner(blink::TaskType::kInternalMedia)
-          ->PostTask(FROM_HERE,
-                     WTF::Bind(&XRSession::OnFrame, WrapWeakPersistent(session),
-                               high_res_now_ms, base::nullopt, base::nullopt));
+          ->PostTask(
+              FROM_HERE,
+              WTF::Bind(&XRFrameProvider::OnPreDispatchInlineFrame,
+                        WrapWeakPersistent(this), WrapWeakPersistent(session),
+                        high_res_now_ms, base::nullopt, base::nullopt));
     }
   }
 }
 
+void XRFrameProvider::OnPreDispatchInlineFrame(
+    XRSession* session,
+    double timestamp,
+    const base::Optional<gpu::MailboxHolder>& output_mailbox_holder,
+    const base::Optional<gpu::MailboxHolder>& camera_image_mailbox_holder) {
+  // Do nothing if the session was cleaned up or ended before we were schedueld.
+  if (!session || session->ended())
+    return;
+
+  // If we have an immersive session, we shouldn't serve frames to the inline
+  // session; however, we need to ensure that we don't stall out its frame loop,
+  // so add a new frame request to get served after the immersive session exits.
+  if (immersive_session_) {
+    RequestFrame(session);
+    return;
+  }
+
+  // If we still have the session and don't have an immersive session, then we
+  // should serve the frame.
+  session->OnFrame(timestamp, output_mailbox_holder,
+                   camera_image_mailbox_holder);
+}
+
 void XRFrameProvider::SubmitWebGLLayer(XRWebGLLayer* layer, bool was_changed) {
   DCHECK(layer);
   DCHECK(immersive_session_);

third_party/blink/renderer/modules/xr/xr_frame_provider.h

@@ -81,6 +81,16 @@ class XRFrameProvider final : public GarbageCollected<XRFrameProvider> {
   void ProcessScheduledFrame(device::mojom::blink::XRFrameDataPtr frame_data,
                              double high_res_now_ms);
 
+  // Called before dispatching a frame to an inline session. This method ensures
+  // that inline session frame calls can be scheduled and that they are neither
+  // served nor dropped if an immersive session is started while the inline
+  // session was waiting to be served.
+  void OnPreDispatchInlineFrame(
+      XRSession* session,
+      double timestamp,
+      const base::Optional<gpu::MailboxHolder>& output_mailbox_holder,
+      const base::Optional<gpu::MailboxHolder>& camera_image_mailbox_holder);
+
   const Member<XRSystem> xr_;
 
   // Immersive session state