Fix InProcessCommandBuffer handling of buffer presented.

There was some incorrect logic in InProcessCommandBuffer. Presentation
feedback was getting sent to the GpuControlClient even if it didn't
request it. Worse yet, vsync timing parameters were getting updated if
presentation feedback was requested, even if the presentation feedback
didn't contain accurate vsync information.

On mac with OOP-D this was causing the real vsync timebase provided by
the browser to get overridden with junk. Mac doesn't have accurate
presentation feedback, instead it's just base::TimeTicks::Now() for when
swap buffers completed.

Share the existing logic found in GLES2CommandBufferStub and
CommandBufferProxyImpl with InProcessCommandBuffer to avoid future
problems.

Bug: 900564
Change-Id: Ib1bdb822b2357a905a923a537d4da7bf01ad0a25
Reviewed-on: https://chromium-review.googlesource.com/c/1337638
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Jonathan Backer <backer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608498}

gpu/command_buffer/common/BUILD.gn

@@ -86,6 +86,8 @@ jumbo_source_set("common_sources") {
     "mailbox.h",
     "mailbox_holder.cc",
     "mailbox_holder.h",
+    "presentation_feedback_utils.cc",
+    "presentation_feedback_utils.h",
     "scheduling_priority.cc",
     "scheduling_priority.h",
     "shared_image_trace_utils.cc",

gpu/command_buffer/common/presentation_feedback_utils.cc

+// Copyright 2018 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 "gpu/command_buffer/common/presentation_feedback_utils.h"
+
+#include "gpu/command_buffer/common/swap_buffers_flags.h"
+#include "ui/gfx/presentation_feedback.h"
+
+namespace gpu {
+
+bool ShouldSendBufferPresented(uint32_t swap_buffer_flags,
+                               uint32_t presentation_feedback_flags) {
+  return swap_buffer_flags & SwapBuffersFlags::kPresentationFeedback ||
+         (swap_buffer_flags & SwapBuffersFlags::kVSyncParams &&
+          presentation_feedback_flags & gfx::PresentationFeedback::kVSync);
+}
+
+bool ShouldUpdateVsyncParams(const gfx::PresentationFeedback& feedback) {
+  return feedback.flags & gfx::PresentationFeedback::kVSync &&
+         feedback.timestamp != base::TimeTicks() &&
+         feedback.interval != base::TimeDelta();
+}
+
+}  // namespace gpu

gpu/command_buffer/common/presentation_feedback_utils.h

+// Copyright 2018 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.
+
+#ifndef GPU_COMMAND_BUFFER_COMMON_PRESENTATION_FEEDBACK_UTILS_H_
+#define GPU_COMMAND_BUFFER_COMMON_PRESENTATION_FEEDBACK_UTILS_H_
+
+#include <cstdint>
+
+#include "gpu/gpu_export.h"
+
+namespace gfx {
+struct PresentationFeedback;
+}
+
+namespace gpu {
+
+// Returns true if command buffer should send buffer presented message to
+// client.
+GPU_EXPORT bool ShouldSendBufferPresented(uint32_t swap_buffer_flags,
+                                          uint32_t presentation_feedback_flags);
+
+// Returns true if command buffer should update vsync timing paramters based on
+// presentation feedback.
+GPU_EXPORT bool ShouldUpdateVsyncParams(
+    const gfx::PresentationFeedback& feedback);
+
+}  // namespace gpu
+
+#endif  // GPU_COMMAND_BUFFER_COMMON_PRESENTATION_FEEDBACK_UTILS_H_

gpu/ipc/client/command_buffer_proxy_impl.cc

@@ -24,6 +24,7 @@
 #include "gpu/command_buffer/common/command_buffer_shared.h"
 #include "gpu/command_buffer/common/gpu_memory_allocation.h"
 #include "gpu/command_buffer/common/gpu_memory_buffer_support.h"
+#include "gpu/command_buffer/common/presentation_feedback_utils.h"
 #include "gpu/command_buffer/common/sync_token.h"
 #include "gpu/ipc/client/gpu_channel_host.h"
 #include "gpu/ipc/common/command_buffer_id.h"
@@ -774,9 +775,7 @@ void CommandBufferProxyImpl::OnBufferPresented(
   if (gpu_control_client_)
     gpu_control_client_->OnSwapBufferPresented(swap_id, feedback);
   if (update_vsync_parameters_completion_callback_ &&
-      feedback.flags & gfx::PresentationFeedback::kVSync &&
-      feedback.timestamp != base::TimeTicks() &&
-      feedback.interval != base::TimeDelta()) {
+      ShouldUpdateVsyncParams(feedback)) {
     update_vsync_parameters_completion_callback_.Run(feedback.timestamp,
                                                      feedback.interval);
   }

gpu/ipc/in_process_command_buffer.cc

@@ -32,6 +32,7 @@
 #include "gpu/command_buffer/client/gpu_memory_buffer_manager.h"
 #include "gpu/command_buffer/client/shared_image_interface.h"
 #include "gpu/command_buffer/common/gpu_memory_buffer_support.h"
+#include "gpu/command_buffer/common/presentation_feedback_utils.h"
 #include "gpu/command_buffer/common/swap_buffers_complete_params.h"
 #include "gpu/command_buffer/common/swap_buffers_flags.h"
 #include "gpu/command_buffer/common/sync_token.h"
@@ -1476,10 +1477,12 @@ void InProcessCommandBuffer::BufferPresented(
   SwapBufferParams params = pending_presented_params_.front();
   pending_presented_params_.pop_front();
 
-  PostOrRunClientCallback(
-      base::BindOnce(&InProcessCommandBuffer::BufferPresentedOnOriginThread,
-                     client_thread_weak_ptr_factory_.GetWeakPtr(),
-                     params.swap_id, params.flags, feedback));
+  if (ShouldSendBufferPresented(params.flags, feedback.flags)) {
+    PostOrRunClientCallback(
+        base::BindOnce(&InProcessCommandBuffer::BufferPresentedOnOriginThread,
+                       client_thread_weak_ptr_factory_.GetWeakPtr(),
+                       params.swap_id, params.flags, feedback));
+  }
 }
 
 void InProcessCommandBuffer::AddFilter(IPC::MessageFilter* message_filter) {
@@ -1505,13 +1508,11 @@ void InProcessCommandBuffer::BufferPresentedOnOriginThread(
   DCHECK_CALLED_ON_VALID_SEQUENCE(client_sequence_checker_);
   if (gpu_control_client_)
     gpu_control_client_->OnSwapBufferPresented(swap_id, feedback);
-  if (flags & gpu::SwapBuffersFlags::kPresentationFeedback ||
-      (flags & gpu::SwapBuffersFlags::kVSyncParams &&
-       feedback.flags & gfx::PresentationFeedback::kVSync)) {
-    if (update_vsync_parameters_completion_callback_ &&
-        feedback.timestamp != base::TimeTicks())
-      update_vsync_parameters_completion_callback_.Run(feedback.timestamp,
-                                                       feedback.interval);
+
+  if (update_vsync_parameters_completion_callback_ &&
+      ShouldUpdateVsyncParams(feedback)) {
+    update_vsync_parameters_completion_callback_.Run(feedback.timestamp,
+                                                     feedback.interval);
   }
 }
 

gpu/ipc/service/gles2_command_buffer_stub.cc

@@ -23,6 +23,7 @@
 #include "gpu/command_buffer/common/constants.h"
 #include "gpu/command_buffer/common/gpu_memory_buffer_support.h"
 #include "gpu/command_buffer/common/mailbox.h"
+#include "gpu/command_buffer/common/presentation_feedback_utils.h"
 #include "gpu/command_buffer/common/swap_buffers_flags.h"
 #include "gpu/command_buffer/service/gl_context_virtual.h"
 #include "gpu/command_buffer/service/gl_state_restorer_impl.h"
@@ -414,9 +415,7 @@ void GLES2CommandBufferStub::BufferPresented(
   SwapBufferParams params = pending_presented_params_.front();
   pending_presented_params_.pop_front();
 
-  if (params.flags & gpu::SwapBuffersFlags::kPresentationFeedback ||
-      (params.flags & gpu::SwapBuffersFlags::kVSyncParams &&
-       feedback.flags & gfx::PresentationFeedback::kVSync)) {
+  if (ShouldSendBufferPresented(params.flags, feedback.flags)) {
     Send(new GpuCommandBufferMsg_BufferPresented(route_id_, params.swap_id,
                                                  feedback));
   }