Factoring presentation-time callback buffering out of LayerTreeHostImpl.

When a LayerTreeHostImpl is building a compositor frame, it adopts
presentation-time callbacks registered with the active tree. These
callbacks let users of LayerTreeHostImpl (a.k.a. LayerTreeHost) listen
for timing feedback once a relevant frame swap finishes. Until the frame
swap ocurrs, however, the callbacks need to be buffered.

This change adds a class to handle buffering and retrieving the
presentation-time callbacks.

This is preperatory work to move towards support for callbacks that run
on the compositor thread instead of the renderer main thread.

Bug: 922980
Change-Id: Iaca39ee8ee5b709a74cc24369e896f59ee6dec3b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1553915
Commit-Queue: Tom McKee <tommckee@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663247}
diff --git a/cc/BUILD.gn b/cc/BUILD.gn
index 5f89773..ff70057 100644
--- a/cc/BUILD.gn
+++ b/cc/BUILD.gn
@@ -319,6 +319,8 @@
     "trees/occlusion.h",
     "trees/occlusion_tracker.cc",
     "trees/occlusion_tracker.h",
+    "trees/presentation_time_callback_buffer.cc",
+    "trees/presentation_time_callback_buffer.h",
     "trees/property_animation_state.cc",
     "trees/property_animation_state.h",
     "trees/property_tree.cc",
@@ -697,6 +699,7 @@
     "trees/layer_tree_impl_unittest.cc",
     "trees/occlusion_tracker_unittest.cc",
     "trees/occlusion_unittest.cc",
+    "trees/presentation_time_callback_buffer_unittest.cc",
     "trees/property_tree_unittest.cc",
     "trees/swap_promise_manager_unittest.cc",
     "trees/tree_synchronizer_unittest.cc",
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index 8757ed6..2da634e 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -9,10 +9,7 @@
 
 #include <algorithm>
 #include <limits>
-#include <map>
-#include <set>
-#include <unordered_map>
-#include <utility>
+#include <list>
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
@@ -82,6 +79,7 @@
 #include "cc/trees/layer_tree_host_common.h"
 #include "cc/trees/layer_tree_impl.h"
 #include "cc/trees/mutator_host.h"
+#include "cc/trees/presentation_time_callback_buffer.h"
 #include "cc/trees/render_frame_metadata.h"
 #include "cc/trees/render_frame_metadata_observer.h"
 #include "cc/trees/scroll_node.h"
@@ -1798,38 +1796,22 @@
   client_->DidReceiveCompositorFrameAckOnImplThread();
 }
 
-LayerTreeHostImpl::FrameTokenInfo::FrameTokenInfo(
-    uint32_t token,
-    base::TimeTicks cc_frame_time,
-    std::vector<LayerTreeHost::PresentationTimeCallback> callbacks)
-    : token(token),
-      cc_frame_time(cc_frame_time),
-      callbacks(std::move(callbacks)) {}
-
-LayerTreeHostImpl::FrameTokenInfo::FrameTokenInfo(FrameTokenInfo&&) = default;
-LayerTreeHostImpl::FrameTokenInfo::~FrameTokenInfo() = default;
-
 void LayerTreeHostImpl::DidPresentCompositorFrame(
     uint32_t frame_token,
     const gfx::PresentationFeedback& feedback) {
-  std::vector<LayerTreeHost::PresentationTimeCallback> all_callbacks;
-  while (!frame_token_infos_.empty()) {
-    auto info = frame_token_infos_.begin();
-    if (viz::FrameTokenGT(info->token, frame_token))
-      break;
+  PresentationTimeCallbackBuffer::PendingCallbacks activated =
+      presentation_time_callbacks_.PopPendingCallbacks(frame_token);
 
-    // Update compositor frame latency and smoothness stats only for frames
-    // that caused on-screen damage.
-    if (info->token == frame_token)
-      frame_metrics_.AddFrameDisplayed(info->cc_frame_time, feedback.timestamp);
-
-    std::copy(std::make_move_iterator(info->callbacks.begin()),
-              std::make_move_iterator(info->callbacks.end()),
-              std::back_inserter(all_callbacks));
-    frame_token_infos_.erase(info);
+  // Update compositor frame latency and smoothness stats only for frames
+  // that caused on-screen damage.
+  if (!activated.frame_time.is_null()) {
+    frame_metrics_.AddFrameDisplayed(activated.frame_time, feedback.timestamp);
   }
+
+  // Send all the main-thread callbacks to the client in one batch. The client
+  // is in charge of posting them to the main thread.
   client_->DidPresentCompositorFrameOnImplThread(
-      frame_token, std::move(all_callbacks), feedback);
+      frame_token, std::move(activated.main_thread_callbacks), feedback);
 }
 
 void LayerTreeHostImpl::DidNotNeedBeginFrame() {
@@ -1929,11 +1911,10 @@
   metadata.content_source_id = active_tree_->content_source_id();
 
   if (active_tree_->has_presentation_callbacks()) {
-    frame_token_infos_.emplace_back(metadata.frame_token,
-                                    CurrentBeginFrameArgs().frame_time,
-                                    active_tree_->TakePresentationCallbacks());
-
-    DCHECK_LE(frame_token_infos_.size(), 25u);
+    presentation_time_callbacks_.RegisterMainThreadPresentationCallbacks(
+        metadata.frame_token, active_tree_->TakePresentationCallbacks());
+    presentation_time_callbacks_.RegisterFrameTime(
+        metadata.frame_token, CurrentBeginFrameArgs().frame_time);
   }
 
   if (GetDrawMode() == DRAW_MODE_RESOURCELESS_SOFTWARE) {
diff --git a/cc/trees/layer_tree_host_impl.h b/cc/trees/layer_tree_host_impl.h
index d4f125eb..ac92ea1 100644
--- a/cc/trees/layer_tree_host_impl.h
+++ b/cc/trees/layer_tree_host_impl.h
@@ -7,15 +7,13 @@
 
 #include <stddef.h>
 
-#include <bitset>
 #include <memory>
 #include <set>
-#include <string>
 #include <unordered_map>
+#include <utility>
 #include <vector>
 
 #include "base/callback.h"
-#include "base/containers/circular_deque.h"
 #include "base/containers/flat_map.h"
 #include "base/memory/memory_pressure_listener.h"
 #include "base/memory/shared_memory_mapping.h"
@@ -43,6 +41,7 @@
 #include "cc/trees/layer_tree_settings.h"
 #include "cc/trees/managed_memory_policy.h"
 #include "cc/trees/mutator_host_client.h"
+#include "cc/trees/presentation_time_callback_buffer.h"
 #include "cc/trees/render_frame_metadata.h"
 #include "cc/trees/task_runner_provider.h"
 #include "cc/trees/ukm_manager.h"
@@ -1159,30 +1158,7 @@
 
   std::unique_ptr<base::MemoryPressureListener> memory_pressure_listener_;
 
-  // Stores information needed once we get a response for a particular
-  // presentation token.
-  struct FrameTokenInfo {
-    FrameTokenInfo(
-        uint32_t token,
-        base::TimeTicks cc_frame_time,
-        std::vector<LayerTreeHost::PresentationTimeCallback> callbacks);
-    FrameTokenInfo(const FrameTokenInfo&) = delete;
-    FrameTokenInfo(FrameTokenInfo&&);
-    ~FrameTokenInfo();
-
-    FrameTokenInfo& operator=(const FrameTokenInfo&) = delete;
-    FrameTokenInfo& operator=(FrameTokenInfo&&) = default;
-
-    uint32_t token;
-
-    // The compositor frame time used to produce the frame.
-    base::TimeTicks cc_frame_time;
-
-    // The callbacks to send back to the main thread.
-    std::vector<LayerTreeHost::PresentationTimeCallback> callbacks;
-  };
-
-  base::circular_deque<FrameTokenInfo> frame_token_infos_;
+  PresentationTimeCallbackBuffer presentation_time_callbacks_;
   ui::FrameMetrics frame_metrics_;
   ui::SkippedFrameTracker skipped_frame_tracker_;
   bool is_animating_for_snap_;
diff --git a/cc/trees/presentation_time_callback_buffer.cc b/cc/trees/presentation_time_callback_buffer.cc
new file mode 100644
index 0000000..5274d5b
--- /dev/null
+++ b/cc/trees/presentation_time_callback_buffer.cc
@@ -0,0 +1,117 @@
+// Copyright 2019 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 "cc/trees/presentation_time_callback_buffer.h"
+
+#include <utility>
+#include <vector>
+
+namespace cc {
+
+PresentationTimeCallbackBuffer::PresentationTimeCallbackBuffer() = default;
+
+PresentationTimeCallbackBuffer::PresentationTimeCallbackBuffer(
+    PresentationTimeCallbackBuffer&& other)
+    : frame_token_infos_(std::move(other.frame_token_infos_)) {}
+
+PresentationTimeCallbackBuffer& PresentationTimeCallbackBuffer::operator=(
+    PresentationTimeCallbackBuffer&& other) {
+  if (this != &other) {
+    frame_token_infos_ = std::move(other.frame_token_infos_);
+  }
+  return *this;
+}
+
+PresentationTimeCallbackBuffer::~PresentationTimeCallbackBuffer() = default;
+
+PresentationTimeCallbackBuffer::FrameTokenInfo::FrameTokenInfo(uint32_t token)
+    : token(token) {}
+
+PresentationTimeCallbackBuffer::FrameTokenInfo::FrameTokenInfo(
+    FrameTokenInfo&&) = default;
+PresentationTimeCallbackBuffer::FrameTokenInfo&
+PresentationTimeCallbackBuffer::FrameTokenInfo::operator=(FrameTokenInfo&&) =
+    default;
+PresentationTimeCallbackBuffer::FrameTokenInfo::~FrameTokenInfo() = default;
+
+void PresentationTimeCallbackBuffer::RegisterMainThreadPresentationCallbacks(
+    uint32_t frame_token,
+    std::vector<CallbackType> callbacks) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  FrameTokenInfo& frame_info = GetOrMakeRegistration(frame_token);
+
+  // Splice the given |callbacks| onto the vector of existing callbacks.
+  auto& sink = frame_info.main_thread_callbacks;
+  sink.reserve(sink.size() + callbacks.size());
+  std::move(callbacks.begin(), callbacks.end(), std::back_inserter(sink));
+
+  DCHECK_LE(frame_token_infos_.size(), 25u);
+}
+
+void PresentationTimeCallbackBuffer::RegisterFrameTime(
+    uint32_t frame_token,
+    base::TimeTicks frame_time) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  FrameTokenInfo& frame_info = GetOrMakeRegistration(frame_token);
+
+  // Protect against clobbering previously registered frame times.
+  DCHECK(frame_info.frame_time.is_null() ||
+         frame_info.frame_time == frame_time);
+  frame_info.frame_time = frame_time;
+
+  DCHECK_LE(frame_token_infos_.size(), 25u);
+}
+
+PresentationTimeCallbackBuffer::PendingCallbacks::PendingCallbacks() = default;
+PresentationTimeCallbackBuffer::PendingCallbacks::PendingCallbacks(
+    PendingCallbacks&&) = default;
+PresentationTimeCallbackBuffer::PendingCallbacks&
+PresentationTimeCallbackBuffer::PendingCallbacks::operator=(
+    PendingCallbacks&&) = default;
+PresentationTimeCallbackBuffer::PendingCallbacks::~PendingCallbacks() = default;
+
+PresentationTimeCallbackBuffer::PendingCallbacks
+PresentationTimeCallbackBuffer::PopPendingCallbacks(uint32_t frame_token) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  PendingCallbacks result;
+
+  while (!frame_token_infos_.empty()) {
+    auto info = frame_token_infos_.begin();
+    if (viz::FrameTokenGT(info->token, frame_token))
+      break;
+
+    // Forward compositor frame timings on exact frame token matches. These
+    // timings correspond to frames that caused on-screen damage.
+    if (info->token == frame_token && !info->frame_time.is_null())
+      result.frame_time = info->frame_time;
+
+    // Collect the main-thread callbacks. It's the caller's job to post them to
+    // the main thread.
+    std::move(info->main_thread_callbacks.begin(),
+              info->main_thread_callbacks.end(),
+              std::back_inserter(result.main_thread_callbacks));
+
+    frame_token_infos_.erase(info);
+  }
+
+  return result;
+}
+
+PresentationTimeCallbackBuffer::FrameTokenInfo&
+PresentationTimeCallbackBuffer::GetOrMakeRegistration(uint32_t frame_token) {
+  // If the freshest registration is for an earlier frame token, add a new
+  // entry to the queue.
+  if (frame_token_infos_.empty() ||
+      viz::FrameTokenGT(frame_token, frame_token_infos_.back().token)) {
+    frame_token_infos_.emplace_back(frame_token);
+  }
+
+  // Registrations should use monotonically increasing frame tokens.
+  DCHECK_EQ(frame_token_infos_.back().token, frame_token);
+
+  return frame_token_infos_.back();
+}
+
+}  // namespace cc
diff --git a/cc/trees/presentation_time_callback_buffer.h b/cc/trees/presentation_time_callback_buffer.h
new file mode 100644
index 0000000..cdebf90
--- /dev/null
+++ b/cc/trees/presentation_time_callback_buffer.h
@@ -0,0 +1,132 @@
+// Copyright 2019 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 CC_TREES_PRESENTATION_TIME_CALLBACK_BUFFER_H_
+#define CC_TREES_PRESENTATION_TIME_CALLBACK_BUFFER_H_
+
+#include <vector>
+
+#include "base/containers/circular_deque.h"
+#include "base/sequence_checker.h"
+#include "cc/trees/layer_tree_host.h"
+
+namespace cc {
+
+// Maintains a queue of callbacks and compositor frame times that we want to
+// buffer until a relevant frame is presented.
+//
+// Callbacks are queued through |RegisterMainThreadPresentationCallbacks|.
+//
+// Once a frame is presented, users of this class can call
+// |PopPendingCallbacks| to get their callbacks back. This class never runs
+// callbacks itself so it's up to calling code to |PostTask| as needed.
+//
+// This class is thread unsafe so concurrent access would require external
+// synchronization. In practice, however, instances of this class are only used
+// on the compositor thread even though the buffered callbacks are intended to
+// be run on the renderer main thread.
+//
+// CC_EXPORT is only needed for testing.
+class CC_EXPORT PresentationTimeCallbackBuffer {
+ public:
+  using CallbackType = LayerTreeHost::PresentationTimeCallback;
+
+  PresentationTimeCallbackBuffer();
+
+  PresentationTimeCallbackBuffer(const PresentationTimeCallbackBuffer&) =
+      delete;
+  PresentationTimeCallbackBuffer(PresentationTimeCallbackBuffer&&);
+
+  PresentationTimeCallbackBuffer& operator=(
+      const PresentationTimeCallbackBuffer&) = delete;
+  PresentationTimeCallbackBuffer& operator=(PresentationTimeCallbackBuffer&&);
+
+  ~PresentationTimeCallbackBuffer();
+
+  // Buffers the given |callbacks| in preparation for a GPU frame swap at or
+  // after the given |frame_token|. Calling code posts these callbacks to the
+  // main thread once they're popped.
+  void RegisterMainThreadPresentationCallbacks(
+      uint32_t frame_token,
+      std::vector<CallbackType> callbacks);
+
+  // The given |frame_time| is associated with the given |frame_token| and will
+  // be exposed through |PopPendingCallbacks| if there is an exact frame token
+  // match. Note that it is an error to register distinct |frame_time|s against
+  // the same |frame_token|.
+  void RegisterFrameTime(uint32_t frame_token, base::TimeTicks frame_time);
+
+  // Structured return value for |PopPendingCallbacks|. CC_EXPORT is only
+  // needed for testing.
+  struct CC_EXPORT PendingCallbacks {
+    PendingCallbacks();
+
+    PendingCallbacks(const PendingCallbacks&) = delete;
+    PendingCallbacks(PendingCallbacks&&);
+
+    PendingCallbacks& operator=(const PendingCallbacks&) = delete;
+    PendingCallbacks& operator=(PendingCallbacks&&);
+
+    ~PendingCallbacks();
+
+    // Holds callbacks registered through
+    // |RegisterMainThreadPresentationCallbacks|.
+    std::vector<CallbackType> main_thread_callbacks;
+
+    // Note: calling code needs to test against frame_time.is_null() because
+    // frame_time is not always defined. See |PopPendingCallbacks|.
+    base::TimeTicks frame_time;
+  };
+
+  // Call this once the presentation for the given |frame_token| has completed.
+  // Yields any pending callbacks that were registered against a frame token
+  // that was less than or equal to the given |frame_token|. It is the caller's
+  // responsibility to run the callbacks on the right threads/sequences. When
+  // the given |frame_token| is an exact match to a registered entry,
+  // |frame_time| will be set to the frame time supplied through
+  // |RegisterFrameTime|. Otherwise, |frame_time| will be default constructed
+  // and should not be used. Calling code can assume |frame_time| is meaningful
+  // iff frame_time.is_null() returns false.
+  PendingCallbacks PopPendingCallbacks(uint32_t frame_token);
+
+ private:
+  // Stores information needed once we get a response for a particular
+  // presentation token.
+  struct FrameTokenInfo {
+    explicit FrameTokenInfo(uint32_t token);
+    FrameTokenInfo(const FrameTokenInfo&) = delete;
+    FrameTokenInfo(FrameTokenInfo&&);
+    FrameTokenInfo& operator=(const FrameTokenInfo&) = delete;
+    FrameTokenInfo& operator=(FrameTokenInfo&&);
+    ~FrameTokenInfo();
+
+    // A |CompositorFrameMetadata::frame_token| that we use to associate
+    // presentation feedback with the relevant compositor frame.
+    uint32_t token;
+
+    // A copy of the |frame_time| from the |BeginFrameArgs| associated with
+    // frame. Useful for tracking latency between frame requests and frame
+    // presentations.
+    base::TimeTicks frame_time;
+
+    // The callbacks to send back to the main thread.
+    std::vector<CallbackType> main_thread_callbacks;
+  };
+
+  // Returns a reference to a |FrameTokenInfo| with the given |frame_token|.
+  // The instance is created if necessary and occupies the appropriate place in
+  // |frame_token_infos_|.
+  FrameTokenInfo& GetOrMakeRegistration(uint32_t frame_token);
+
+  // Queue of pending registrations ordered by |token|. We can use a deque
+  // because we require callers to use non-decreasing tokens when registering.
+  base::circular_deque<FrameTokenInfo> frame_token_infos_;
+
+  // When DCHECK is enabled, check that instances of this class aren't being
+  // used concurrently.
+  SEQUENCE_CHECKER(sequence_checker_);
+};
+
+}  // namespace cc
+
+#endif  // CC_TREES_PRESENTATION_TIME_CALLBACK_BUFFER_H_
diff --git a/cc/trees/presentation_time_callback_buffer_unittest.cc b/cc/trees/presentation_time_callback_buffer_unittest.cc
new file mode 100644
index 0000000..2d6b36f
--- /dev/null
+++ b/cc/trees/presentation_time_callback_buffer_unittest.cc
@@ -0,0 +1,159 @@
+// Copyright 2019 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 "cc/trees/presentation_time_callback_buffer.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+std::vector<cc::PresentationTimeCallbackBuffer::CallbackType> GenerateCallbacks(
+    int num_callbacks) {
+  std::vector<cc::PresentationTimeCallbackBuffer::CallbackType> result;
+
+  while (num_callbacks-- > 0) {
+    // PresentationTimeCallbackBuffer isn't supposed to invoke any callbacks.
+    // We can check for that by passing callbacks which cause test failure.
+    result.emplace_back(base::BindOnce([](const gfx::PresentationFeedback&) {
+      FAIL() << "Callbacks should not be directly invoked by "
+                "PresentationTimeCallbackBuffer";
+    }));
+  }
+
+  return result;
+}
+
+base::TimeTicks MakeTicks(uint64_t us) {
+  return base::TimeTicks() + base::TimeDelta::FromMicroseconds(us);
+}
+
+constexpr uint32_t kFrameToken1 = 234;
+constexpr uint32_t kFrameToken2 = 345;
+constexpr uint32_t kFrameToken3 = 456;
+constexpr uint32_t kFrameToken4 = 567;
+
+}  // namespace
+
+namespace cc {
+
+TEST(PresentationTimeCallbackBufferTest, TestNoCallbacks) {
+  PresentationTimeCallbackBuffer buffer;
+
+  auto result = buffer.PopPendingCallbacks(kFrameToken1);
+
+  EXPECT_TRUE(result.main_thread_callbacks.empty());
+  EXPECT_TRUE(result.frame_time.is_null());
+}
+
+TEST(PresentationTimeCallbackBufferTest, TestOneMainThreadCallback) {
+  PresentationTimeCallbackBuffer buffer;
+
+  buffer.RegisterMainThreadPresentationCallbacks(kFrameToken2,
+                                                 GenerateCallbacks(1));
+
+  // Make sure that popping early frame tokens doesn't return irrelevant
+  // entries.
+  {
+    auto result = buffer.PopPendingCallbacks(kFrameToken1);
+    EXPECT_TRUE(result.main_thread_callbacks.empty());
+    EXPECT_TRUE(result.frame_time.is_null());
+  }
+
+  {
+    auto result = buffer.PopPendingCallbacks(kFrameToken2);
+    EXPECT_EQ(result.main_thread_callbacks.size(), 1ull);
+    EXPECT_TRUE(result.frame_time.is_null());
+  }
+
+  // Make sure that the buffer has removed the registration since the "pop".
+  {
+    auto result = buffer.PopPendingCallbacks(kFrameToken2);
+    EXPECT_TRUE(result.main_thread_callbacks.empty());
+    EXPECT_TRUE(result.frame_time.is_null());
+  }
+}
+
+TEST(PresentationTimeCallbackBufferTest, TestFrameTimeRegistration) {
+  PresentationTimeCallbackBuffer buffer;
+
+  base::TimeTicks frame_time = MakeTicks(234);
+  buffer.RegisterFrameTime(kFrameToken2, frame_time);
+
+  // Make sure that popping early frame tokens doesn't return irrelevant
+  // entries.
+  {
+    auto result = buffer.PopPendingCallbacks(kFrameToken1);
+    EXPECT_TRUE(result.main_thread_callbacks.empty());
+    EXPECT_TRUE(result.frame_time.is_null());
+  }
+
+  {
+    auto result = buffer.PopPendingCallbacks(kFrameToken2);
+    EXPECT_TRUE(result.main_thread_callbacks.empty());
+    EXPECT_FALSE(result.frame_time.is_null());
+    EXPECT_EQ(result.frame_time, frame_time);
+  }
+
+  // Make sure that the buffer has removed the registration since the "pop".
+  {
+    auto result = buffer.PopPendingCallbacks(kFrameToken2);
+    EXPECT_TRUE(result.main_thread_callbacks.empty());
+    EXPECT_TRUE(result.frame_time.is_null());
+  }
+}
+
+TEST(PresentationTimeCallbackBufferTest, TestCallbackBatchingNoFrameTime) {
+  PresentationTimeCallbackBuffer buffer;
+
+  base::TimeTicks frame_time1 = MakeTicks(123);
+  base::TimeTicks frame_time2 = MakeTicks(234);
+  base::TimeTicks frame_time4 = MakeTicks(456);
+
+  // Register one callback for frame1, two for frame2 and two for frame4.
+  buffer.RegisterMainThreadPresentationCallbacks(kFrameToken1,
+                                                 GenerateCallbacks(1));
+  buffer.RegisterFrameTime(kFrameToken1, frame_time1);
+  buffer.RegisterMainThreadPresentationCallbacks(kFrameToken2,
+                                                 GenerateCallbacks(2));
+  buffer.RegisterFrameTime(kFrameToken2, frame_time2);
+  buffer.RegisterMainThreadPresentationCallbacks(kFrameToken4,
+                                                 GenerateCallbacks(2));
+  buffer.RegisterFrameTime(kFrameToken4, frame_time4);
+
+  // Pop callbacks up to and including frame3. Should be three in total; one
+  // from frame1 and two from frame2. There shouldn't be a frame time because
+  // no frame time was registered against exactly kFrameToken3.
+  {
+    auto result = buffer.PopPendingCallbacks(kFrameToken3);
+    EXPECT_EQ(result.main_thread_callbacks.size(), 3ull);
+    EXPECT_TRUE(result.frame_time.is_null());
+  }
+}
+
+TEST(PresentationTimeCallbackBufferTest, TestCallbackBatchingWithFrameTime) {
+  PresentationTimeCallbackBuffer buffer;
+
+  base::TimeTicks frame_time3 = MakeTicks(345);
+
+  // Register one callback for frame1, two for frame2 and two for frame4.
+  buffer.RegisterMainThreadPresentationCallbacks(kFrameToken1,
+                                                 GenerateCallbacks(1));
+  buffer.RegisterMainThreadPresentationCallbacks(kFrameToken2,
+                                                 GenerateCallbacks(2));
+  buffer.RegisterFrameTime(kFrameToken3, frame_time3);
+  buffer.RegisterMainThreadPresentationCallbacks(kFrameToken4,
+                                                 GenerateCallbacks(2));
+
+  // Pop callbacks up to and including frame3. Should be three in total; one
+  // from frame1 and two from frame2. There should be a frame time because
+  // a frame time was registered against exactly kFrameToken3.
+  {
+    auto result = buffer.PopPendingCallbacks(kFrameToken3);
+    EXPECT_EQ(result.main_thread_callbacks.size(), 3ull);
+    EXPECT_FALSE(result.frame_time.is_null());
+    EXPECT_EQ(result.frame_time, frame_time3);
+  }
+}
+
+}  // namespace cc