WebVR: process animations from posted task to yield for other events

third_party/WebKit/Source/modules/vr/VRDisplay.cpp

Index: third_party/WebKit/Source/modules/vr/VRDisplay.cpp
diff --git a/third_party/WebKit/Source/modules/vr/VRDisplay.cpp b/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
index 4fa6fa737b48b1bcd283427ec31cc2e29c010de0..d318bd6c240a918f85743d243d640ee835796ab3 100644
--- a/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
+++ b/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
@@ -689,6 +689,14 @@ void VRDisplay::OnDeactivate(
       EventTypeNames::vrdisplaydeactivate, true, false, this, reason));
 }
 
+void VRDisplay::processScheduledAnimations(double timestamp) {
+  TRACE_EVENT1("gpu", "VRDisplay::OnVSync", "frame", m_vrFrameId);
+
+  AutoReset<bool> animating(&m_inAnimationFrame, true);
+  m_pendingRaf = false;
+  m_scriptedAnimationController->serviceScriptedAnimations(timestamp);
+}
+
 void VRDisplay::OnVSync(device::mojom::blink::VRPosePtr pose,
                         mojo::common::mojom::blink::TimeDeltaPtr time,
                         int16_t frameId,
@@ -711,12 +719,20 @@ void VRDisplay::OnVSync(device::mojom::blink::VRPosePtr pose,
     m_timebase = WTF::monotonicallyIncreasingTime() - timeDelta.InSecondsF();
   }
 
-  AutoReset<bool> animating(&m_inAnimationFrame, true);
   m_framePose = std::move(pose);
   m_vrFrameId = frameId;
-  m_pendingRaf = false;
-  m_scriptedAnimationController->serviceScriptedAnimations(
-      m_timebase + timeDelta.InSecondsF());
+
+  // Post a task to handle scheduled animations after the current
+  // execution context finishes. If we call GetVSync inline here via

[mthiesse, 2017/03/15 17:43:51]

Comment needs to be updated.

[klausw, 2017/03/15 17:55:40]

The comment is accurate, but I'll rephrase it to be less confusing.

I think the sequence was:

while (mojo stuff to do)
  OnVSync
    serviceScriptedAnimations
      JS rAF callback
        rAF
          GetVSync
        ...
        submitFrame
          (VSync arrives, add to mojo queue)
  (keep going since queue isn't empty)

[mthiesse, 2017/03/15 18:05:03]

The problem is the comment says "If we call GetVSync inline here via...", but we
do call GetVSync inline here. We should instead say that we need to inject a
posted task with every frame to prevent starvation of non-mojo IPC, as our
render loop blocks on mojo IPC and ends up taking priority over other posted
tasks as a result.

[klausw, 2017/03/15 18:14:26]

No, we don't call GetVSync inline here. It's now called from the posted task,
the scheduled animation will call rAF from the JS code it's executing.

Anyway, comment rephrased to be less detailed, the bug pointer should be enough
for context.

[mthiesse, 2017/03/15 18:19:01]

We should at least mention in this comment that the starvation is due to
WaitForIncomingMethodCall

[klausw, 2017/03/15 18:21:00]

Done.

+  // serviceScriptedAnimations calling rAF, we can end up with the
+  // next VSync already being ready and scheduled when we exit, and
+  // then we'll process several frames in a row without ever yielding
+  // back to the main event loop. This causes extreme input delay, see
+  // crbug.com/701444.
+  Platform::current()->currentThread()->getWebTaskRunner()->postTask(
+      BLINK_FROM_HERE,
+      WTF::bind(&VRDisplay::processScheduledAnimations,
+                wrapWeakPersistent(this), m_timebase + timeDelta.InSecondsF()));
 }
 
 void VRDisplay::ConnectVSyncProvider() {