WebVR: Defer GetVSync calls until the current frame is submitted.

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 5122c99d42d313daa98f7a7ad95c259908966e61..a98b7a8de59704317c8f0ffc8d6b0042e7a75bde 100644
--- a/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
+++ b/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
@@ -129,12 +129,10 @@ VREyeParameters* VRDisplay::getEyeParameters(const String& which_eye) {
   }
 }
 
-int VRDisplay::requestAnimationFrame(FrameRequestCallback* callback) {
-  DVLOG(2) << __FUNCTION__;
-  Document* doc = this->GetDocument();
-  if (!doc)
-    return 0;
-  pending_vrdisplay_raf_ = true;
+void VRDisplay::RequestVSync() {
+  if (pending_vsync_ || !pending_vrdisplay_raf_)
+    return;
+
   if (!vr_v_sync_provider_.is_bound()) {
     ConnectVSyncProvider();
   } else if (!display_blurred_ && !pending_vsync_) {
@@ -142,6 +140,17 @@ int VRDisplay::requestAnimationFrame(FrameRequestCallback* callback) {
     vr_v_sync_provider_->GetVSync(ConvertToBaseCallback(
         WTF::Bind(&VRDisplay::OnVSync, WrapWeakPersistent(this))));
   }
+}
+
+int VRDisplay::requestAnimationFrame(FrameRequestCallback* callback) {
+  DVLOG(2) << __FUNCTION__;
+  Document* doc = this->GetDocument();
+  if (!doc)
+    return 0;
+  pending_vrdisplay_raf_ = true;
+  if (did_submit_this_frame_) {
+    RequestVSync();
+  }

[bajones, 2017/05/18 15:59:18]

I'm not sure what this call site provides? Seems like it can't be called unless
the frame has been submitted, which calls RequestVSync anyway?

[mthiesse, 2017/05/18 16:09:01]

This code is all really subtle and kind of scary.

We need this here in case the page calls rAF after calling SubmitFrame, because
then RequestVSync() inside SubmitFrame will do nothing.

Klaus please add a comment here explaining how all possible cases are covered
and we won't fail to request a VSync in response to rAF.

[klausw, 2017/05/18 22:22:29]

I've added a detailed explanation in RequestVSync, pointing to it from here.
Also added a layout test that covers all four cases.

   callback->use_legacy_time_base_ = false;
   return EnsureScriptedAnimationController(doc).RegisterCallback(callback);
 }
@@ -606,6 +615,8 @@ void VRDisplay::submitFrame() {
       vr_frame_id_,
       gpu::MailboxHolder(static_image->GetMailbox(),
                          static_image->GetSyncToken(), GL_TEXTURE_2D));
+  did_submit_this_frame_ = true;
+  RequestVSync();
 
   // If preserveDrawingBuffer is false, must clear now. Normally this
   // happens as part of compositing, but that's not active while
@@ -756,7 +767,12 @@ void VRDisplay::ProcessScheduledAnimations(double timestamp) {
     // that may be called later.
     AutoReset<bool> animating(&in_animation_frame_, true);
     pending_vrdisplay_raf_ = false;
+    did_submit_this_frame_ = false;
     scripted_animation_controller_->ServiceScriptedAnimations(timestamp);
+    if (!did_submit_this_frame_) {
+      DVLOG(2) << __FUNCTION__ << ": vrDisplay.rAF did not submit a frame";
+      RequestVSync();
+    }
   }
 

[mthiesse, 2017/05/18 16:09:01]

Add a DCHECK here that if pending_vrdisplay_raf_ is true, that either the VSync
has been requested, or that the vsync provider isn't connected.

[klausw, 2017/05/18 22:22:29]

Done.

   // For GVR, we shut down normal vsync processing during VR presentation.