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..9e7cf6bb540a00d4fe9b2143f4f3ee3886e2cd62 100644
--- a/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
+++ b/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
@@ -151,6 +151,14 @@ VREyeParameters* VRDisplay::getEyeParameters(const String& whichEye) {
   }
 }
 
+void VRDisplay::doGetVSync() {
+  if (!m_displayBlurred && !m_pendingVsync) {
+    m_pendingVsync = true;
+    m_vrVSyncProvider->GetVSync(convertToBaseCallback(
+        WTF::bind(&VRDisplay::OnVSync, wrapWeakPersistent(this))));
+  }
+}
+
 int VRDisplay::requestAnimationFrame(FrameRequestCallback* callback) {
   Document* doc = this->document();
   if (!doc)
@@ -159,9 +167,15 @@ int VRDisplay::requestAnimationFrame(FrameRequestCallback* callback) {
   if (!m_vrVSyncProvider.is_bound()) {
     ConnectVSyncProvider();
   } else if (!m_displayBlurred && !m_pendingVsync) {
-    m_pendingVsync = true;
-    m_vrVSyncProvider->GetVSync(convertToBaseCallback(
-        WTF::bind(&VRDisplay::OnVSync, wrapWeakPersistent(this))));
+    // Schedule a task to fetch a new VSync after the current
+    // execution context finishes. If we call GetVSync inline here, 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,
+    // causing extreme input delay. See crbug.com/701444.
+    Platform::current()->currentThread()->getWebTaskRunner()->postTask(

[mthiesse, 2017/03/15 15:17:35]

Two comments:
1. This shouldn't be necessary, this sounds like a mojo bug - it shouldn't be
starving out other IPC.

2. You should move the postTask to VRDisplay::OnVSync, because I think we want
to request the VSync early, since the process hops will add a bunch of delay and
we would prefer to do useful things during that delay.

[klausw, 2017/03/15 15:42:39]

Yes, this was unexpected. It's not just starving IPC, it seems as if it
processes all received calls on its incoming mojo queue in a loop before
yielding so nothing else gets to execute that's not a direct result of this mojo
processing. I suspect this behavior gets triggered by the
waitForIncomingMethodCall which ends up moving a received VSync to the "ready to
execute now" queue. (This is a hypothesis, not sure if it's really how it works
internally.)
You mean moving the serviceScriptedAnimations call there to a posted task? I'll
try if that works, if yes it would indeed be cleaner. If not, I can try posting
the GetVSync request a bit earlier from submitFrame if I switch to the alternate
timing where it waits for transfer at the start of the next frame.

Another alternative might be to have VrShellGl actively send a VSync without
waiting for the client to request it, with throttling to let at most one be
outstanding. That could help get timing more consistent, but it's a larger
change since it doesn't fit with the current callback-based logic.

[klausw, 2017/03/15 15:50:59]

To clarify, I think that executing the next GetVSync early from OnVSync would
still run into the same problem we have now, the issue seems to be that we don't
want the VSync callback being already available for processing when it finishes
current work. Specifically (if my hypothesis is right) it mustn't arrive before
or while we're doing a waitForIncomingMethodCall. I think that posting a
deferred GetVSync from OnVSync would be equivalent to doing so in rAF since in
both cases it would be executed after the current serviceScriptedAnimations call
executes, I think there's no yielding happening in between.

[klausw, 2017/03/15 17:39:25]

The deferred serviceScriptedAnimations works, going with that.

+        BLINK_FROM_HERE,
+        WTF::bind(&VRDisplay::doGetVSync, wrapWeakPersistent(this)));
   }
   callback->m_useLegacyTimeBase = false;
   return ensureScriptedAnimationController(doc).registerCallback(callback);
@@ -715,6 +729,8 @@ void VRDisplay::OnVSync(device::mojom::blink::VRPosePtr pose,
   m_framePose = std::move(pose);
   m_vrFrameId = frameId;
   m_pendingRaf = false;
+  TRACE_EVENT1("gpu", "VRDisplay::OnVSync", "frame", frameId);
+
   m_scriptedAnimationController->serviceScriptedAnimations(
       m_timebase + timeDelta.InSecondsF());
 }