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

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/FrameRequestCallback.h"
 #include "core/dom/ScriptedAnimationController.h"
@@ skipping 111 matching lines @@
   switch (StringToVREye(which_eye)) {
     case kVREyeLeft:
       return eye_parameters_left_;
     case kVREyeRight:
       return eye_parameters_right_;
     default:
       return nullptr;
   }
 }
 
-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_) {
     pending_vsync_ = true;
     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);
 }
 
 void VRDisplay::cancelAnimationFrame(int id) {
   DVLOG(2) << __FUNCTION__;
   if (!scripted_animation_controller_)
     return;
   scripted_animation_controller_->CancelCallback(id);
 }
@@ skipping 444 matching lines @@
       }
     }
   }
 
   pending_previous_frame_render_ = true;
   pending_submit_frame_ = true;
   display_->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
   // presenting, so run the responsible code directly.
   rendering_context_->MarkCompositedAndClearBackbufferIfNeeded();
 
   // If we're not deferring the wait for transferring the mailbox,
   // we need to wait for it now to prevent the image going out of
   // scope before its mailbox is retrieved.
   if (!wait_for_previous_transfer_to_finish) {
@@ skipping 130 matching lines @@
   }
 
   TRACE_EVENT1("gpu", "VRDisplay::OnVSync", "frame", vr_frame_id_);
 
   if (pending_vrdisplay_raf_ && scripted_animation_controller_) {
     // Run the callback, making sure that in_animation_frame_ is only
     // true for the vrDisplay rAF and not for a legacy window rAF
     // 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.
   // Trigger any callbacks on window.rAF manually so that they run after
   // completing the vrDisplay.rAF processing.
   if (is_presenting_ && !capabilities_->hasExternalDisplay()) {
     Platform::Current()->CurrentThread()->GetWebTaskRunner()->PostTask(
         BLINK_FROM_HERE, WTF::Bind(&VRDisplay::ProcessScheduledWindowAnimations,
                                    WrapWeakPersistent(this), timestamp));
   }
 }
 
@@ skipping 109 matching lines @@
   visitor->Trace(stage_parameters_);
   visitor->Trace(eye_parameters_left_);
   visitor->Trace(eye_parameters_right_);
   visitor->Trace(layer_);
   visitor->Trace(rendering_context_);
   visitor->Trace(scripted_animation_controller_);
   visitor->Trace(pending_present_resolvers_);
 }
 
 }  // namespace blink