ui: Add SkippedFrameTracker for FrameMetrics

Sources that run on BeginFrames can delegate their skipped
frame logic to SkippedFrameTracker.

SkippedFrameTracker handles the following corner cases:
1) when non-consecutive BeginFrames are received.
2) when the producer is not trying to produce.

Bug: 790759
Change-Id: I99d4067d4eab79f220b892256d8d3d49d1af1209
Reviewed-on: https://chromium-review.googlesource.com/1070994
Commit-Queue: Brian Anderson <brianderson@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565459}

ui/latency/BUILD.gn

@@ -18,6 +18,8 @@ jumbo_source_set("latency") {
     "latency_info.h",
     "latency_tracker.cc",
     "latency_tracker.h",
+    "skipped_frame_tracker.cc",
+    "skipped_frame_tracker.h",
     "stream_analyzer.cc",
     "stream_analyzer.h",
     "windowed_analyzer.cc",
@@ -53,6 +55,7 @@ test("latency_unittests") {
     "frame_metrics_unittest.cc",
     "histograms_unittest.cc",
     "latency_info_unittest.cc",
+    "skipped_frame_tracker_unittest.cc",
     "stream_analyzer_unittest.cc",
     "windowed_analyzer_unittest.cc",
   ]

ui/latency/frame_metrics.h

@@ -13,6 +13,7 @@
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "base/trace_event/trace_event_argument.h"
+#include "ui/latency/skipped_frame_tracker.h"
 
 namespace ui {
 namespace frame_metrics {
@@ -92,19 +93,21 @@ struct FrameMetricsSettings {
 // Statistics will be reported automatically. Either periodically, based
 // on the client interface, or on destruction if any samples were added since
 // the last call to StartNewReportPeriod.
-class FrameMetrics {
+class FrameMetrics : public SkippedFrameTracker::Client {
  public:
   explicit FrameMetrics(FrameMetricsSettings settings);
-  virtual ~FrameMetrics();
+  ~FrameMetrics() override;
 
   // Resets all data and history as if the class were just created.
   void Reset();
 
   // AddFrameProduced should be called every time a source produces a frame.
   // The information added here affects the number of frames skipped.
+  // Note: If the FrameMetrics class is hooked up to an optional
+  //   SkippedFrameTracker, the client should not call this directly.
   void AddFrameProduced(base::TimeTicks source_timestamp,
                         base::TimeDelta amount_produced,
-                        base::TimeDelta amount_skipped);
+                        base::TimeDelta amount_skipped) override;
 
   // AddFrameDisplayed should be called whenever a frame causes damage and
   // we know when the result became visible on the display.

ui/latency/skipped_frame_tracker.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 "ui/latency/skipped_frame_tracker.h"
+
+#include <cmath>
+#include "ui/latency/frame_metrics.h"
+
+namespace ui {
+
+SkippedFrameTracker::SkippedFrameTracker(Client* client) : client_(client) {}
+
+void SkippedFrameTracker::BeginFrame(base::TimeTicks frame_time,
+                                     base::TimeDelta interval) {
+  DCHECK(!inside_begin_frame_);
+  inside_begin_frame_ = true;
+  did_produce_this_frame_ = false;
+  frame_time_ = frame_time;
+  interval_ = interval;
+
+  // On our first frame of activity, we may need to initialize
+  // will_produce_frame_time_.
+  if (active_state_ == ActiveState::WillProduceFirst &&
+      will_produce_frame_time_.is_null()) {
+    will_produce_frame_time_ = frame_time_;
+  }
+}
+
+void SkippedFrameTracker::FinishFrame() {
+  DCHECK(inside_begin_frame_);
+  inside_begin_frame_ = false;
+
+  // Assume the source is idle if it hasn't attempted to produce for an entire
+  // BeginFrame.
+  if (!did_produce_this_frame_ && active_state_ == ActiveState::WasActive) {
+    will_produce_frame_time_ = base::TimeTicks();
+    active_state_ = ActiveState::Idle;
+  }
+}
+
+void SkippedFrameTracker::WillProduceFrame() {
+  // Make sure we don't transition out of the WillProduceFirst state until
+  // we've actually produced the first frame.
+  if (active_state_ == ActiveState::WillProduceFirst)
+    return;
+
+  // This is our first frame of activity.
+  if (active_state_ == ActiveState::Idle) {
+    active_state_ = ActiveState::WillProduceFirst;
+    // If we're already inside a BeginFrame when we first become active,
+    // we can initialize will_produce_frame_time_.
+    if (inside_begin_frame_)
+      will_produce_frame_time_ = frame_time_;
+    return;
+  }
+
+  active_state_ = ActiveState::WillProduce;
+}
+
+void SkippedFrameTracker::DidProduceFrame() {
+  // Ignore duplicate calls to DidProduceFrame.
+  if (did_produce_this_frame_)
+    return;
+
+  // Return early if frame was pulled by sink.
+  bool frame_was_pushed_by_source =
+      (active_state_ == ActiveState::WillProduceFirst &&
+       !will_produce_frame_time_.is_null()) ||
+      active_state_ == ActiveState::WillProduce;
+  if (!frame_was_pushed_by_source)
+    return;
+
+  DCHECK(!will_produce_frame_time_.is_null());
+
+  // Clamp the amount of time skipped to a positive value, since negative
+  // values aren't meaningful.
+  base::TimeDelta skipped_clamped =
+      std::max(base::TimeDelta(), (frame_time_ - will_produce_frame_time_));
+
+  // Snap the amount of time skipped to whole intervals in order to filter
+  // out jitter in the timing received by the BeginFrame source.
+  int skipped_intervals = (skipped_clamped + (interval_ / 2)) / interval_;
+  base::TimeDelta skipped_snapped = skipped_intervals * interval_;
+
+  DCHECK_GE(skipped_snapped, base::TimeDelta());
+  client_->AddFrameProduced(frame_time_, interval_, skipped_snapped);
+
+  // Predict the next BeginFrame's frame time, so we can detect if it gets
+  // dropped.
+  will_produce_frame_time_ = frame_time_ + interval_;
+  active_state_ = ActiveState::WasActive;
+  did_produce_this_frame_ = true;
+}
+
+}  // namespace ui

ui/latency/skipped_frame_tracker.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 UI_LATENCY_SKIPPED_FRAME_TRACKER_H_
+#define UI_LATENCY_SKIPPED_FRAME_TRACKER_H_
+
+#include "base/macros.h"
+#include "base/time/time.h"
+
+namespace ui {
+
+// SkippedFrameTracker tracks skipped BeginFrames. It can be used by sources
+// attempting to produce at the display rate. It properly handles
+// non-consecutive BeginFrames and tracks when the source is actualy trying to
+// produce, rather than passively receiving BeginFrames.
+class SkippedFrameTracker {
+ public:
+  // SkippedFrameTracker calls Client::AddFrameProduced from FinishFrame
+  // when necessary and with the correct values.
+  class Client {
+   public:
+    virtual ~Client() = default;
+    virtual void AddFrameProduced(base::TimeTicks source_timestamp,
+                                  base::TimeDelta amount_produced,
+                                  base::TimeDelta amount_skipped) = 0;
+  };
+
+  // SkippedFrameTracker will call |client|->AddFrameProduced
+  // with the appropriate info automatically as frames are produced.
+  explicit SkippedFrameTracker(Client* client);
+
+  // BeginFrame and FinishFrame must be called for each BeginFrame received.
+  // In order for this class to detect idle periods properly, the source must
+  // call Begin+FinishFrame without calling WillProduceFrame before going
+  // idle. This is necessary since there is otherwise no way to tell if a
+  // non-consecutive BeginFrame occured a) because we were slow or b) because
+  // we weren't trying to produce a frame.
+  void BeginFrame(base::TimeTicks frame_time, base::TimeDelta interval);
+  void FinishFrame();
+
+  // WillProduceFrame should be called when the source knows it wants to
+  // produce a frame. DidProduceFrame should be called when the source has
+  // actually submitted the frame.
+  // It is okay for DidProduceFrame to be called without WillProduceFrame,
+  // which can happen in cases where a frame is "pulled" from later in the
+  // pipeline rather than pushed from the source. Such calls to DidProduceFrame
+  // will be ignored.
+  void WillProduceFrame();
+  void DidProduceFrame();
+
+ protected:
+  Client* client_;
+
+  bool inside_begin_frame_ = false;
+  base::TimeTicks frame_time_;
+  base::TimeDelta interval_;
+  bool did_produce_this_frame_ = false;
+
+  base::TimeTicks will_produce_frame_time_;
+
+  enum class ActiveState {
+    // Idle: The initial and idle state.
+    // Goto WillProduceFirst on 1st call to WillProduceFrame.
+    Idle,
+    // WillProduceFirst: Producing the first frame out of idle.
+    // Goto WasActive on first FinishFrame after a DidProduceFrame.
+    // Counts missing BeginFrames as skipped: NO.
+    WillProduceFirst,
+    // WillProduce: Producing the (N > 1)'th frame of constant activity.
+    // Goto WasActive on first FinishFrame after a DidProduceFrame.
+    // Counts missing BeginFrames as skipped: YES.
+    WillProduce,
+    // WasActive: An intermediate state to determine if we are idle or not.
+    // Goto WillProduce on WillProduceFrame.
+    // Otherwise, goto Idle on next FinishFrame.
+    WasActive,
+  };
+  ActiveState active_state_ = ActiveState::Idle;
+
+  DISALLOW_COPY_AND_ASSIGN(SkippedFrameTracker);
+};
+
+}  // namespace ui
+
+#endif  // UI_LATENCY_SKIPPED_FRAME_TRACKER_H_

ui/latency/skipped_frame_tracker_unittest.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 "ui/latency/skipped_frame_tracker.h"
+
+#include "base/bind.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace ui {
+namespace {
+
+// TestClient observes calls to AddFrameProduced so tests can verify
+// those calls.
+class TestClient : public SkippedFrameTracker::Client {
+ public:
+  TestClient() = default;
+  ~TestClient() override = default;
+
+  void AddFrameProduced(base::TimeTicks source_timestamp,
+                        base::TimeDelta amount_produced,
+                        base::TimeDelta amount_skipped) override {
+    source_timestamp_ = source_timestamp.since_origin().InMicroseconds();
+    amount_produced_ = amount_produced.InMicroseconds();
+    amount_skipped_ = amount_skipped.InMicroseconds();
+    call_count_++;
+  }
+
+  int GetAndResetCallCount() {
+    int result = call_count_;
+    call_count_ = 0;
+    return result;
+  }
+
+  int call_count_ = 0;
+  int source_timestamp_ = 0;
+  int amount_produced_ = 0;
+  int amount_skipped_ = 0;
+};
+
+// TestSkippedFrameTracker let's us verify the active state from tests.
+class TestSkippedFrameTracker : public SkippedFrameTracker {
+ public:
+  TestSkippedFrameTracker(Client* client) : SkippedFrameTracker(client) {}
+
+  bool IsActive() {
+    switch (active_state_) {
+      case ActiveState::Idle:
+        return false;
+      case ActiveState::WillProduce:
+      case ActiveState::WillProduceFirst:
+      case ActiveState::WasActive:
+        break;
+    }
+    return true;
+  }
+};
+
+// SkippedFrameTrackerTest is the test fixture used by all tests in this file.
+class SkippedFrameTrackerTest : public testing::Test {
+ public:
+  SkippedFrameTrackerTest() : tracker_(&client_) {}
+
+  ::testing::AssertionResult BeginFrame(int timestamp, int interval) {
+    int call_count = client_.call_count_;
+    tracker_.BeginFrame(
+        base::TimeTicks() + base::TimeDelta::FromMicroseconds(timestamp),
+        base::TimeDelta::FromMicroseconds(interval));
+    return MaybeCallCountFailure(call_count);
+  }
+
+  ::testing::AssertionResult FinishFrame() {
+    int call_count = client_.call_count_;
+    tracker_.FinishFrame();
+    return MaybeCallCountFailure(call_count);
+  }
+
+  ::testing::AssertionResult WillProduceFrame() {
+    int call_count = client_.call_count_;
+    tracker_.WillProduceFrame();
+    return MaybeCallCountFailure(call_count);
+  }
+
+  ::testing::AssertionResult DidProduceFrame() {
+    int call_count = client_.call_count_;
+    tracker_.DidProduceFrame();
+    return MaybeCallCountFailure(call_count);
+  }
+
+ protected:
+  static ::testing::AssertionResult MaybeCallCountFailure(int count) {
+    if (count == 0)
+      return ::testing::AssertionSuccess();
+    return ::testing::AssertionFailure()
+           << count << " unverified calls to AddFrameProduced.";
+  }
+
+  TestClient client_;
+  TestSkippedFrameTracker tracker_;
+};
+
+#define VERIFY_ADD_PRODUCED_CALLED(timestamp, produced, skipped) \
+  EXPECT_EQ(1, client_.GetAndResetCallCount());                  \
+  EXPECT_EQ(timestamp, client_.source_timestamp_);               \
+  EXPECT_EQ(produced, client_.amount_produced_);                 \
+  EXPECT_EQ(skipped, client_.amount_skipped_);
+
+// Producing a frame entirely within a BeginFrame works.
+TEST_F(SkippedFrameTrackerTest, NoSkips_BeginThenWill) {
+  EXPECT_TRUE(BeginFrame(100, 10));
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(100, 10, 0);
+  EXPECT_TRUE(FinishFrame());
+}
+
+// Starting to produce a frame before receiving the BeginFrame works.
+TEST_F(SkippedFrameTrackerTest, NoSkips_WillThenBegin) {
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(BeginFrame(100, 10));
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(100, 10, 0);
+  EXPECT_TRUE(FinishFrame());
+}
+
+// A (WillProduceFrame, DidProduceFrame) that spans multiple BeginFrames
+// is registered properly.
+TEST_F(SkippedFrameTrackerTest, Skips_ProducedOverMultipleBeginFrames) {
+  EXPECT_TRUE(BeginFrame(100, 10));
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(FinishFrame());
+  EXPECT_TRUE(BeginFrame(110, 10));
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(110, 10, 10);
+  EXPECT_TRUE(FinishFrame());
+}
+
+// An unexpected jump in the frame timestamp, compared to the interval,
+// is registered as skipped time.
+TEST_F(SkippedFrameTrackerTest, Skips_DroppedBeginFrames) {
+  EXPECT_TRUE(BeginFrame(100, 10));
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(100, 10, 0);
+  EXPECT_TRUE(FinishFrame());
+
+  EXPECT_TRUE(BeginFrame(200, 10));
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(200, 10, 90);
+  EXPECT_TRUE(FinishFrame());
+}
+
+// Jitter just below the interval midpoint rounds down the number of dropped
+// BeginFrames detected.
+TEST_F(SkippedFrameTrackerTest, Skips_DroppedBeginFrames_JitterRoundsDown) {
+  EXPECT_TRUE(BeginFrame(100, 10));
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(FinishFrame());
+  EXPECT_TRUE(BeginFrame(114, 10));
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(114, 10, 10);
+  EXPECT_TRUE(FinishFrame());
+}
+
+// Jitter just above the interval midpoint rounds up the number of dropped
+// BeginFrames detected.
+TEST_F(SkippedFrameTrackerTest, Skips_DroppedBeginFrames_JitterRoundsUp) {
+  EXPECT_TRUE(BeginFrame(100, 10));
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(FinishFrame());
+  EXPECT_TRUE(BeginFrame(116, 10));
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(116, 10, 20);
+  EXPECT_TRUE(FinishFrame());
+}
+
+// Active, idle, then active again.
+// In second active period, start to produce frame first.
+TEST_F(SkippedFrameTrackerTest, NoSkips_ActiveIdleActive_WillThenBegin) {
+  // Active
+  EXPECT_TRUE(BeginFrame(100, 10));
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(100, 10, 0);
+  EXPECT_TRUE(FinishFrame());
+
+  // Idle
+  EXPECT_TRUE(BeginFrame(110, 10));
+  EXPECT_TRUE(FinishFrame());
+
+  // Active
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(BeginFrame(120, 10));
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(120, 10, 0);
+  EXPECT_TRUE(FinishFrame());
+}
+
+// Active, idle, then active again.
+// In second active period, BeginFrame first.
+TEST_F(SkippedFrameTrackerTest, NoSkips_ActiveIdleActive_BeginThenWill) {
+  EXPECT_TRUE(BeginFrame(100, 10));
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(100, 10, 0);
+  EXPECT_TRUE(FinishFrame());
+  EXPECT_TRUE(tracker_.IsActive());
+
+  EXPECT_TRUE(BeginFrame(110, 10));
+  EXPECT_TRUE(FinishFrame());
+  EXPECT_FALSE(tracker_.IsActive());
+
+  EXPECT_TRUE(BeginFrame(120, 10));
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(120, 10, 0);
+  EXPECT_TRUE(FinishFrame());
+  EXPECT_TRUE(tracker_.IsActive());
+}
+
+// Active, idle, then active again.
+// Dropped BeginFrames during idle period shouldn't register as skipped.
+TEST_F(SkippedFrameTrackerTest, NoSkips_ActiveIdleActive_JumpInIdle) {
+  EXPECT_TRUE(BeginFrame(100, 10));
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(100, 10, 0);
+  EXPECT_TRUE(FinishFrame());
+  EXPECT_TRUE(tracker_.IsActive());
+
+  EXPECT_TRUE(BeginFrame(110, 10));
+  EXPECT_TRUE(FinishFrame());
+  EXPECT_FALSE(tracker_.IsActive());
+
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(BeginFrame(200, 10));
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(200, 10, 0);
+  EXPECT_TRUE(FinishFrame());
+  EXPECT_TRUE(tracker_.IsActive());
+}
+
+// If frames are pulled from later in the pipeline when the source hasn't tried
+// to create a new frame, it should not be recorded as a frame produced
+// by the source.
+TEST_F(SkippedFrameTrackerTest, PulledFramesNotRecorded) {
+  EXPECT_TRUE(BeginFrame(100, 10));
+  // WillProduceFrame intentionally not called here impliles
+  // next call to DidProduceFrame was "pulled" not "pushed".
+  EXPECT_TRUE(DidProduceFrame());
+  EXPECT_TRUE(FinishFrame());
+
+  // Even though BeginFrames might've been dropped since the pulled frame,
+  // act as if we should behanve just like the produce is coming out of an
+  // idle period.
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(BeginFrame(200, 10));
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(200, 10, 0);
+  EXPECT_TRUE(FinishFrame());
+}
+
+// Multiple calls to WillProduceFrame are legal and should behave as if only
+// the first call was made.
+TEST_F(SkippedFrameTrackerTest, MultipleWillProduceBeforeDidProduce) {
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(BeginFrame(100, 10));
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(100, 10, 0);
+  EXPECT_TRUE(FinishFrame());
+}
+
+// Frame pulled before BeginFrame doesn't count.
+TEST_F(SkippedFrameTrackerTest, NoSkips_ActiveIdleActive_FramePulledBeforeBF) {
+  EXPECT_TRUE(BeginFrame(100, 10));
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(100, 10, 0);
+  EXPECT_TRUE(FinishFrame());
+  EXPECT_TRUE(tracker_.IsActive());
+
+  EXPECT_TRUE(BeginFrame(110, 10));
+  EXPECT_TRUE(FinishFrame());
+  EXPECT_FALSE(tracker_.IsActive());
+
+  EXPECT_TRUE(WillProduceFrame());
+  // Consider frame pulled since it came before the BeginFrame.
+  EXPECT_TRUE(DidProduceFrame());
+  // Make sure we are immune to multiple pulled frames.
+  EXPECT_TRUE(DidProduceFrame());
+
+  EXPECT_TRUE(BeginFrame(120, 10));
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(120, 10, 0);
+  EXPECT_TRUE(FinishFrame());
+  EXPECT_TRUE(tracker_.IsActive());
+}
+
+// Frame pulled just after a push doesn't count.
+TEST_F(SkippedFrameTrackerTest, NoSkips_ActiveIdleActive_FramePulledAfterPush) {
+  EXPECT_TRUE(BeginFrame(100, 10));
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(100, 10, 0);
+  EXPECT_TRUE(FinishFrame());
+  EXPECT_TRUE(tracker_.IsActive());
+
+  EXPECT_TRUE(BeginFrame(110, 10));
+  EXPECT_TRUE(FinishFrame());
+  EXPECT_FALSE(tracker_.IsActive());
+
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(BeginFrame(120, 10));
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(120, 10, 0);
+  // Consider frame pulled since we aleady pushed one this frame.
+  EXPECT_TRUE(DidProduceFrame());
+  // Make sure we are immune to multiple pulled frames.
+  EXPECT_TRUE(DidProduceFrame());
+  EXPECT_TRUE(FinishFrame());
+  EXPECT_TRUE(tracker_.IsActive());
+}
+
+// Frame pulled while attempting to push counts.
+TEST_F(SkippedFrameTrackerTest, NoSkips_ActiveIdleActive_FramePulledIsPush) {
+  EXPECT_TRUE(BeginFrame(100, 10));
+  EXPECT_TRUE(WillProduceFrame());
+  EXPECT_TRUE(FinishFrame());
+  EXPECT_TRUE(tracker_.IsActive());
+
+  // Consider frame pushed, even if we are outside the BeginFrame, since we
+  // were trying to push.
+  EXPECT_TRUE(DidProduceFrame());
+  VERIFY_ADD_PRODUCED_CALLED(100, 10, 0);
+  // A second pulled frame shouldn't count though.
+  EXPECT_TRUE(DidProduceFrame());
+
+  EXPECT_TRUE(BeginFrame(110, 10));
+  EXPECT_TRUE(FinishFrame());
+  EXPECT_FALSE(tracker_.IsActive());
+}
+
+}  // namespace
+}  // namespace ui