Remove Sync GetPose VRService call, implement VRVSyncProvider

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 b2d9678e3df6bc57f8254ec94587ab0119655b2b..d276fa6d22301cd8c866d516d7b0730ffa545e7e 100644
--- a/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
+++ b/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
@@ -47,33 +47,6 @@ VREye stringToVREye(const String& whichEye) {
   return VREyeNone;
 }
 
-class VRDisplayFrameRequestCallback : public FrameRequestCallback {
- public:
-  VRDisplayFrameRequestCallback(VRDisplay* vrDisplay) : m_vrDisplay(vrDisplay) {
-    m_useLegacyTimeBase = true;
-  }
-  ~VRDisplayFrameRequestCallback() override {}
-  void handleEvent(double highResTimeMs) override {
-    Document* doc = m_vrDisplay->document();
-    if (!doc)
-      return;
-
-    // Need to divide by 1000 here because serviceScriptedAnimations expects
-    // time to be given in seconds.
-    m_vrDisplay->serviceScriptedAnimations(
-        doc->loader()->timing().pseudoWallTimeToMonotonicTime(highResTimeMs /
-                                                              1000.0));
-  }
-
-  DEFINE_INLINE_VIRTUAL_TRACE() {
-    visitor->trace(m_vrDisplay);
-
-    FrameRequestCallback::trace(visitor);
-  }
-
-  Member<VRDisplay> m_vrDisplay;
-};
-
 }  // namespace
 
 VRDisplay::VRDisplay(NavigatorVR* navigatorVR,
@@ -81,21 +54,12 @@ VRDisplay::VRDisplay(NavigatorVR* navigatorVR,
                      device::mojom::blink::VRDisplayClientRequest request)
     : ContextLifecycleObserver(navigatorVR->document()),
       m_navigatorVR(navigatorVR),
-      m_isConnected(false),
-      m_isPresenting(false),
-      m_isValidDeviceForPresenting(true),
-      m_canUpdateFramePose(true),
       m_capabilities(new VRDisplayCapabilities()),
       m_eyeParametersLeft(new VREyeParameters()),
       m_eyeParametersRight(new VREyeParameters()),
-      m_depthNear(0.01),
-      m_depthFar(10000.0),
       m_fullscreenCheckTimer(this, &VRDisplay::onFullscreenCheck),
-      m_contextGL(nullptr),
-      m_animationCallbackRequested(false),
-      m_inAnimationFrame(false),
       m_display(std::move(display)),
-      m_binding(this, std::move(request)) {}
+      m_displayClientBinding(this, std::move(request)) {}
 
 VRDisplay::~VRDisplay() {}
 
@@ -144,9 +108,7 @@ void VRDisplay::disconnected() {
 }
 
 bool VRDisplay::getFrameData(VRFrameData* frameData) {
-  updatePose();
-
-  if (!m_framePose)
+  if (!m_framePose || m_displayBlurred)
     return false;
 
   if (!frameData)
@@ -160,9 +122,7 @@ bool VRDisplay::getFrameData(VRFrameData* frameData) {
 }
 
 VRPose* VRDisplay::getPose() {
-  updatePose();
-
-  if (!m_framePose)
+  if (!m_framePose || m_displayBlurred)
     return nullptr;
 
   VRPose* pose = VRPose::create();
@@ -170,23 +130,6 @@ VRPose* VRDisplay::getPose() {
   return pose;
 }
 
-void VRDisplay::updatePose() {
-  if (m_displayBlurred) {
-    // WebVR spec says to return a null pose when the display is blurred.
-    m_framePose = nullptr;
-    return;
-  }
-  if (m_canUpdateFramePose) {
-    if (!m_display)
-      return;
-    device::mojom::blink::VRPosePtr pose;
-    m_display->GetPose(&pose);
-    m_framePose = std::move(pose);
-    if (m_isPresenting)
-      m_canUpdateFramePose = false;
-  }
-}
-
 void VRDisplay::resetPose() {
   if (!m_display)
     return;
@@ -209,12 +152,9 @@ int VRDisplay::requestAnimationFrame(FrameRequestCallback* callback) {
   Document* doc = this->document();
   if (!doc)
     return 0;
-
-  if (!m_animationCallbackRequested) {
-    doc->requestAnimationFrame(new VRDisplayFrameRequestCallback(this));
-    m_animationCallbackRequested = true;
+  if (!m_vrVSyncProvider.is_bound() && !m_displayBlurred) {
+    ConnectVSyncProvider();
   }
-
   callback->m_useLegacyTimeBase = false;
   return ensureScriptedAnimationController(doc).registerCallback(callback);
 }
@@ -227,44 +167,18 @@ void VRDisplay::cancelAnimationFrame(int id) {
 
 void VRDisplay::OnBlur() {
   m_displayBlurred = true;
-
+  m_vrVSyncProvider.reset();
   m_navigatorVR->enqueueVREvent(VRDisplayEvent::create(
       EventTypeNames::vrdisplayblur, true, false, this, ""));
 }
 
 void VRDisplay::OnFocus() {
   m_displayBlurred = false;
-  // Restart our internal doc requestAnimationFrame callback, if it fired while
-  // the display was blurred.
-  // TODO(bajones): Don't use doc->requestAnimationFrame() at all. Animation
-  // frames should be tied to the presenting VR display (e.g. should be serviced
-  // by GVR library callbacks on Android), and not the doc frame rate.
-  if (!m_animationCallbackRequested) {
-    Document* doc = this->document();
-    if (!doc)
-      return;
-    doc->requestAnimationFrame(new VRDisplayFrameRequestCallback(this));
-  }
+  ConnectVSyncProvider();
   m_navigatorVR->enqueueVREvent(VRDisplayEvent::create(
       EventTypeNames::vrdisplayfocus, true, false, this, ""));
 }
 
-void VRDisplay::serviceScriptedAnimations(double monotonicAnimationStartTime) {
-  if (!m_scriptedAnimationController)
-    return;
-  AutoReset<bool> animating(&m_inAnimationFrame, true);
-  m_animationCallbackRequested = false;
-
-  // We use an internal rAF callback to run the animation loop at the display
-  // speed, and run the user's callback after our internal callback fires.
-  // However, when the display is blurred, we want to pause the animation loop,
-  // so we don't fire the user's callback until the display is focused.
-  if (m_displayBlurred)
-    return;
-  m_scriptedAnimationController->serviceScriptedAnimations(
-      monotonicAnimationStartTime);
-}
-
 void ReportPresentationResult(PresentationResult result) {
   // Note that this is called twice for each call to requestPresent -
   // one to declare that requestPresent was called, and one for the
@@ -694,6 +608,53 @@ void VRDisplay::OnDeactivate(
       EventTypeNames::vrdisplaydeactivate, true, false, this, reason));
 }
 
+void VRDisplay::OnVSync(device::mojom::blink::VRPosePtr pose,
+                        double timeSeconds) {
+  if (m_vrVSyncProvider.is_bound()) {
+    m_vrVSyncProvider->GetVSync(convertToBaseCallback(
+        WTF::bind(&VRDisplay::OnVSync, wrapWeakPersistent(this))));
+  }
+  // The VSync provider cannot shut down before replying to pending callbacks,
+  // so it will send a null pose with no timestamp to be ignored.
+  if (pose.is_null() && timeSeconds == 0)
+    return;
+  if (m_displayBlurred)
+    return;
+  if (!m_scriptedAnimationController)
+    return;
+  Document* doc = this->document();
+  if (!doc)
+    return;
+
+  // Ensure a consistent timebase with document rAF.
+  if (m_timebase < 0) {
+    m_timebase = WTF::monotonicallyIncreasingTime() - timeSeconds;
+  }
+
+  AutoReset<bool> animating(&m_inAnimationFrame, true);
+  m_framePose = std::move(pose);
+  m_canUpdateFramePose = false;
+  m_scriptedAnimationController->serviceScriptedAnimations(m_timebase +
+                                                           timeSeconds);
+}
+
+void VRDisplay::OnVSyncProviderDisconnected() {
+  TaskRunnerHelper::get(TaskType::UnspecedTimer, document())
+      ->postDelayedTask(
+          BLINK_FROM_HERE,
+          WTF::bind(&VRDisplay::ConnectVSyncProvider, wrapWeakPersistent(this)),
+          100);  // Short delay to throttle requests.

[dcheng, 2017/01/12 09:56:56]

My understanding is the impl side is hosted in the browser process. If this pipe
was ever unexpectedly disconnected, that would indicate the browser process is
shutting down (in which case this would never succeed). Is it necessary to try
to reconnect?

[mthiesse, 2017/01/12 15:49:03]

No, this pipe shutting down doesn't indicate the browser process is shutting
down. It indicates that we're switching from one VRVSyncProvider to another.
Rather than send a new message saying "Hey, your VSync provider is gone but
another one is available", we use the disconnected error to signal this. It's
assumed that there's always a VRVSyncProvider available.

Is there a cleaner approach here? Can we move a binding to another thread
without breaking it? I would assume not...

[dcheng, 2017/01/14 11:22:53]

You can use mojo::Binding<T>::Unbind() to get an InterfaceRequest which can be
passed around and bound somewhere else.

[mthiesse, 2017/01/16 20:52:57]

Ah thanks, that's exactly what I was looking for and makes things so much
cleaner.

+}
+
+void VRDisplay::ConnectVSyncProvider() {
+  m_display->GetVRVSyncProvider(mojo::MakeRequest(&m_vrVSyncProvider));
+  m_vrVSyncProvider.set_connection_error_handler(
+      convertToBaseCallback(WTF::bind(&VRDisplay::OnVSyncProviderDisconnected,
+                                      wrapWeakPersistent(this))));
+  m_vrVSyncProvider->GetVSync(convertToBaseCallback(
+      WTF::bind(&VRDisplay::OnVSync, wrapWeakPersistent(this))));
+}
+
 void VRDisplay::onFullscreenCheck(TimerBase*) {
   if (!m_isPresenting) {
     m_fullscreenCheckTimer.stop();
@@ -739,7 +700,8 @@ ScriptedAnimationController& VRDisplay::ensureScriptedAnimationController(
 }
 
 void VRDisplay::dispose() {
-  m_binding.Close();
+  m_displayClientBinding.Close();
+  m_vrVSyncProvider.reset();
 }
 
 ExecutionContext* VRDisplay::getExecutionContext() const {