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

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "modules/vr/VRDisplay.h"
 
 #include "core/css/StylePropertySet.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/DocumentUserGestureToken.h"
 #include "core/dom/FrameRequestCallback.h"
@@ skipping 133 matching lines @@
   switch (stringToVREye(whichEye)) {
     case VREyeLeft:
       return m_eyeParametersLeft;
     case VREyeRight:
       return m_eyeParametersRight;
     default:
       return nullptr;
   }
 }
 
+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)
     return 0;
   m_pendingRaf = true;
   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);
 }
 
 void VRDisplay::cancelAnimationFrame(int id) {
   if (!m_scriptedAnimationController)
     return;
   m_scriptedAnimationController->cancelCallback(id);
 }
@@ skipping 533 matching lines @@
       WTF::TimeDelta::FromMicroseconds(time->microseconds);
   // Ensure a consistent timebase with document rAF.
   if (m_timebase < 0) {
     m_timebase = WTF::monotonicallyIncreasingTime() - timeDelta.InSecondsF();
   }
 
   AutoReset<bool> animating(&m_inAnimationFrame, true);
   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());
 }
 
 void VRDisplay::ConnectVSyncProvider() {
   if (!m_navigatorVR->isFocused() || m_vrVSyncProvider.is_bound())
     return;
   m_display->GetVRVSyncProvider(mojo::MakeRequest(&m_vrVSyncProvider));
   m_vrVSyncProvider.set_connection_error_handler(convertToBaseCallback(
       WTF::bind(&VRDisplay::OnVSyncConnectionError, wrapWeakPersistent(this))));
@@ skipping 55 matching lines @@
   visitor->trace(m_stageParameters);
   visitor->trace(m_eyeParametersLeft);
   visitor->trace(m_eyeParametersRight);
   visitor->trace(m_layer);
   visitor->trace(m_renderingContext);
   visitor->trace(m_scriptedAnimationController);
   visitor->trace(m_pendingPresentResolvers);
 }
 
 }  // namespace blink