cc: Add RingBuffer::Iterator to simplify buffer iteration

This change adds the inner class Iterator to the RingBuffer, which allows for
easier iteration of the buffer. The Iterator gets retrieved from the buffer by
either calling RingBuffer::Begin() or RingBuffer::End(). This change switches
buffer iterations to using the new Iterator and also cleans up data retrieval
on the HudLayer by providing Iterator accessors in FrameRateCounter,
PainTimeCounter and MemoryHistory.


Review URL: https://chromiumcodereview.appspot.com/12225150

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@183451 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 99 insertions, 73 deletions

diff --git a/cc/frame_rate_counter.cc b/cc/frame_rate_counter.cc
index 3628d78..8cfb598 100644
--- a/cc/frame_rate_counter.cc
+++ b/cc/frame_rate_counter.cc
@@ -20,9 +20,10 @@ scoped_ptr<FrameRateCounter> FrameRateCounter::create(bool hasImplThread) {
   return make_scoped_ptr(new FrameRateCounter(hasImplThread));
 }
 
-inline base::TimeDelta FrameRateCounter::recentFrameInterval(size_t n) const
+base::TimeDelta FrameRateCounter::recentFrameInterval(size_t n) const
 {
     DCHECK(n > 0);
+    DCHECK(n < m_ringBuffer.BufferSize());
     return m_ringBuffer.ReadBuffer(n) - m_ringBuffer.ReadBuffer(n - 1);
 }
 
@@ -67,8 +68,8 @@ void FrameRateCounter::getMinAndMaxFPS(double& minFPS, double& maxFPS) const
     minFPS = std::numeric_limits<double>::max();
     maxFPS = 0;
 
-    for (size_t i = m_ringBuffer.BufferSize() - 1; i > 0 && m_ringBuffer.IsFilledIndex(i - 1); --i) {
-        base::TimeDelta delta = recentFrameInterval(i);
+    for (RingBufferType::Iterator it = --m_ringBuffer.End(); it; --it) {
+        base::TimeDelta delta = recentFrameInterval(it.index() + 1);
 
         if (isBadFrameInterval(delta))
             continue;
@@ -101,8 +102,8 @@ double FrameRateCounter::getAverageFPS() const
     //
     // isBadFrameInterval encapsulates the frame too slow/frame too fast logic.
 
-    for (size_t i = m_ringBuffer.BufferSize() - 1; i > 0 && m_ringBuffer.IsFilledIndex(i - 1) && frameTimesTotal < 1.0; --i) {
-        base::TimeDelta delta = recentFrameInterval(i);
+    for (RingBufferType::Iterator it = --m_ringBuffer.End(); it && frameTimesTotal < 1.0; --it) {
+        base::TimeDelta delta = recentFrameInterval(it.index() + 1);
 
         if (!isBadFrameInterval(delta)) {
             frameCount++;
@@ -119,14 +120,4 @@ double FrameRateCounter::getAverageFPS() const
     return averageFPS;
 }
 
-base::TimeTicks FrameRateCounter::timeStampOfRecentFrame(size_t n) const
-{
-    DCHECK(n < m_ringBuffer.BufferSize());
-
-    if (m_ringBuffer.IsFilledIndex(n))
-        return m_ringBuffer.ReadBuffer(n);
-
-    return base::TimeTicks();
-}
-
 }  // namespace cc
diff --git a/cc/frame_rate_counter.h b/cc/frame_rate_counter.h
index 413500a..ade2004 100644
--- a/cc/frame_rate_counter.h
+++ b/cc/frame_rate_counter.h
@@ -24,9 +24,9 @@ public:
 
     void saveTimeStamp(base::TimeTicks timestamp);
 
-    // n = 0 returns the oldest frame retained in the history,
-    // while n = timeStampHistorySize() - 1 returns the timestamp most recent frame.
-    base::TimeTicks timeStampOfRecentFrame(size_t n) const;
+    // n = 0 returns the oldest frame interval retained in the history,
+    // while n = timeStampHistorySize() - 1 returns the most recent frame interval.
+    base::TimeDelta recentFrameInterval(size_t n) const;
 
     // This is a heuristic that can be used to ignore frames in a reasonable way. Returns
     // true if the given frame interval is too fast or too slow, based on constant thresholds.
@@ -35,11 +35,13 @@ public:
     void getMinAndMaxFPS(double& minFPS, double& maxFPS) const;
     double getAverageFPS() const;
 
+    typedef RingBuffer<base::TimeTicks, 136> RingBufferType;
+    RingBufferType::Iterator begin() const { return m_ringBuffer.Begin(); }
+    RingBufferType::Iterator end() const { return m_ringBuffer.End(); }
+
 private:
     explicit FrameRateCounter(bool hasImplThread);
 
-    base::TimeDelta recentFrameInterval(size_t n) const;
-
     // Two thresholds (measured in seconds) that describe what is considered to be a "no-op frame" that should not be counted.
     // - if the frame is too fast, then given our compositor implementation, the frame probably was a no-op and did not draw.
     // - if the frame is too slow, then there is probably not animating content, so we should not pollute the average.
@@ -51,7 +53,7 @@ private:
     // FIXME: Determine this threshold based on monitor refresh rate, crbug.com/138642.
     static const double kDroppedFrameTime;
 
-    RingBuffer<base::TimeTicks, 136> m_ringBuffer;
+    RingBufferType m_ringBuffer;
 
     bool m_hasImplThread;
     int m_droppedFrameCount;
diff --git a/cc/heads_up_display_layer_impl.cc b/cc/heads_up_display_layer_impl.cc
index 9d664e1..661fb8a 100644
--- a/cc/heads_up_display_layer_impl.cc
+++ b/cc/heads_up_display_layer_impl.cc
@@ -193,7 +193,8 @@ void HeadsUpDisplayLayerImpl::drawHudContents(SkCanvas* canvas)
             fpsCounter->getMinAndMaxFPS(m_fpsGraph.min, m_fpsGraph.max);
 
             base::TimeDelta latest, min, max;
-            latest = paintTimeCounter->GetPaintTimeOfRecentFrame(paintTimeCounter->HistorySize() - 1);
+            if (paintTimeCounter->End())
+                latest = paintTimeCounter->End()->total_time();
             paintTimeCounter->GetMinAndMaxPaintTime(&min, &max);
 
             m_paintTimeGraph.value = latest.InMillisecondsF();
@@ -305,15 +306,14 @@ int HeadsUpDisplayLayerImpl::drawFPSDisplay(SkCanvas* canvas, FrameRateCounter*
     drawGraphLines(canvas, &paint, graphBounds, m_fpsGraph);
 
     // Collect graph and histogram data.
-    int x = 0;
     SkPath path;
 
     const int histogramSize = 20;
     double histogram[histogramSize] = {0};
     double maxBucketValue = 0;
 
-    for (size_t i = 0; i < fpsCounter->timeStampHistorySize() - 1; ++i) {
-        base::TimeDelta delta = fpsCounter->timeStampOfRecentFrame(i + 1) - fpsCounter->timeStampOfRecentFrame(i);
+    for (FrameRateCounter::RingBufferType::Iterator it = --fpsCounter->end(); it; --it) {
+        base::TimeDelta delta = fpsCounter->recentFrameInterval(it.index() + 1);
 
         // Skip this particular instantaneous frame rate if it is not likely to have been valid.
         if (!fpsCounter->isBadFrameInterval(delta)) {
@@ -326,7 +326,7 @@ int HeadsUpDisplayLayerImpl::drawFPSDisplay(SkCanvas* canvas, FrameRateCounter*
                 p = 1;
 
             // Plot this data point.
-            SkPoint cur = SkPoint::Make(graphBounds.left() + x, graphBounds.bottom() - p * graphBounds.height());
+            SkPoint cur = SkPoint::Make(graphBounds.left() + it.index(), graphBounds.bottom() - p * graphBounds.height());
             if (path.isEmpty())
                 path.moveTo(cur);
             else
@@ -339,8 +339,6 @@ int HeadsUpDisplayLayerImpl::drawFPSDisplay(SkCanvas* canvas, FrameRateCounter*
             histogram[bucketIndex] += delta.InSecondsF();
             maxBucketValue = std::max(histogram[bucketIndex], maxBucketValue);
         }
-
-        x++;
     }
 
     // Draw FPS histogram.
@@ -370,13 +368,12 @@ int HeadsUpDisplayLayerImpl::drawFPSDisplay(SkCanvas* canvas, FrameRateCounter*
 
 int HeadsUpDisplayLayerImpl::drawMemoryDisplay(SkCanvas* canvas, MemoryHistory* memoryHistory, const int& initial_top)
 {
-    MemoryHistory::Entry curEntry = memoryHistory->GetEntry(
-        memoryHistory->HistorySize() - 1);
-
     // Don't draw the display if there is no data in it.
-    if (curEntry.bytes_total() == 0)
+    if (!memoryHistory->End())
         return initial_top;
 
+    const MemoryHistory::Entry curEntry = **memoryHistory->End();
+
     // Move up by 2 to create no gap between us and previous counter.
     const int top = initial_top - 2;
     const int padding = 4;
@@ -448,8 +445,8 @@ int HeadsUpDisplayLayerImpl::drawPaintTimeDisplay(SkCanvas* canvas, PaintTimeCou
     drawGraphLines(canvas, &paint, graphBounds, m_paintTimeGraph);
 
     paint.setColor(DebugColors::PaintTimeDisplayTextAndGraphColor());
-    for (size_t i = 0; i < paintTimeCounter->HistorySize(); ++i) {
-        double pt = paintTimeCounter->GetPaintTimeOfRecentFrame(i).InMillisecondsF();
+    for (PaintTimeCounter::RingBufferType::Iterator it = paintTimeCounter->End(); it; --it) {
+        double pt = it->total_time().InMillisecondsF();
 
         if (pt == 0.0)
             continue;
@@ -458,7 +455,7 @@ int HeadsUpDisplayLayerImpl::drawPaintTimeDisplay(SkCanvas* canvas, PaintTimeCou
         if (p > 1)
             p = 1;
 
-        canvas->drawRect(SkRect::MakeXYWH(graphBounds.left() + i * 2, graphBounds.bottom() - p * graphBounds.height(), 1, p * graphBounds.height()), paint);
+        canvas->drawRect(SkRect::MakeXYWH(graphBounds.left() + it.index() * 2, graphBounds.bottom() - p * graphBounds.height(), 1, p * graphBounds.height()), paint);
     }
 
     return top + height + 2;
diff --git a/cc/memory_history.cc b/cc/memory_history.cc
index 6934f7f..0638810 100644
--- a/cc/memory_history.cc
+++ b/cc/memory_history.cc
@@ -14,15 +14,6 @@ scoped_ptr<MemoryHistory> MemoryHistory::create() {
 MemoryHistory::MemoryHistory() {
 }
 
-MemoryHistory::Entry MemoryHistory::GetEntry(const size_t& n) const {
-  DCHECK(n < ring_buffer_.BufferSize());
-
-  if (ring_buffer_.IsFilledIndex(n))
-    return ring_buffer_.ReadBuffer(n);
-
-  return MemoryHistory::Entry();
-}
-
 void MemoryHistory::SaveEntry(const MemoryHistory::Entry& entry) {
   ring_buffer_.SaveToBuffer(entry);
 }
@@ -31,11 +22,8 @@ void MemoryHistory::GetMinAndMax(size_t* min, size_t* max) const {
   *min = std::numeric_limits<size_t>::max();
   *max = 0;
 
-  for (size_t i = 0; i < ring_buffer_.BufferSize(); i++) {
-    if (!ring_buffer_.IsFilledIndex(i))
-        continue;
-    const Entry entry = ring_buffer_.ReadBuffer(i);
-    size_t bytes_total = entry.bytes_total();
+  for (RingBufferType::Iterator it = ring_buffer_.Begin(); it; ++it) {
+    size_t bytes_total = it->bytes_total();
 
     if (bytes_total < *min)
       *min = bytes_total;
diff --git a/cc/memory_history.h b/cc/memory_history.h
index c46a29f..ddf6a0f 100644
--- a/cc/memory_history.h
+++ b/cc/memory_history.h
@@ -37,17 +37,17 @@ class MemoryHistory {
       }
   };
 
-  // n = 0 returns the oldest and
-  // n = HistorySize() - 1 the most recent paint time.
-  Entry GetEntry(const size_t& n) const;
-
   void SaveEntry(const Entry& entry);
   void GetMinAndMax(size_t* min, size_t* max) const;
 
+  typedef RingBuffer<Entry, 80> RingBufferType;
+  RingBufferType::Iterator Begin() const { return ring_buffer_.Begin(); }
+  RingBufferType::Iterator End() const { return ring_buffer_.End(); }
+
  private:
   MemoryHistory();
 
-  RingBuffer<Entry, 80> ring_buffer_;
+  RingBufferType ring_buffer_;
 
   DISALLOW_COPY_AND_ASSIGN(MemoryHistory);
 };
diff --git a/cc/paint_time_counter.cc b/cc/paint_time_counter.cc
index 7749cf7..f7feb01 100644
--- a/cc/paint_time_counter.cc
+++ b/cc/paint_time_counter.cc
@@ -16,16 +16,6 @@ PaintTimeCounter::PaintTimeCounter()
     can_save_rasterize_time_delta_(false) {
 }
 
-base::TimeDelta PaintTimeCounter::GetPaintTimeOfRecentFrame(
-    const size_t& n) const {
-  DCHECK(n < ring_buffer_.BufferSize());
-
-  if (ring_buffer_.IsFilledIndex(n))
-    return ring_buffer_.ReadBuffer(n).total_time();
-
-  return base::TimeDelta();
-}
-
 void PaintTimeCounter::SavePaintTime(const base::TimeDelta& total_paint_time,
                                      int commit_number) {
   if (can_save_paint_time_delta_) {
@@ -57,15 +47,13 @@ void PaintTimeCounter::GetMinAndMaxPaintTime(base::TimeDelta* min,
   *min = base::TimeDelta::FromDays(1);
   *max = base::TimeDelta();
 
-  for (size_t i = 0; i < ring_buffer_.BufferSize(); i++) {
-    if (ring_buffer_.IsFilledIndex(i)) {
-      base::TimeDelta paint_time = ring_buffer_.ReadBuffer(i).total_time();
+  for (RingBufferType::Iterator it = ring_buffer_.Begin(); it; ++it) {
+    const base::TimeDelta paint_time = it->total_time();
 
-      if (paint_time < *min)
-        *min = paint_time;
-      if (paint_time > *max)
-        *max = paint_time;
-    }
+    if (paint_time < *min)
+      *min = paint_time;
+    if (paint_time > *max)
+      *max = paint_time;
   }
 
   if (*min > *max)
diff --git a/cc/paint_time_counter.h b/cc/paint_time_counter.h
index b110fc8..b9c6a6c 100644
--- a/cc/paint_time_counter.h
+++ b/cc/paint_time_counter.h
@@ -44,10 +44,14 @@ class PaintTimeCounter {
 
   void ClearHistory();
 
+  typedef RingBuffer<Entry, 90> RingBufferType;
+  RingBufferType::Iterator Begin() const { return ring_buffer_.Begin(); }
+  RingBufferType::Iterator End() const { return ring_buffer_.End(); }
+
  private:
   PaintTimeCounter();
 
-  RingBuffer<Entry, 90> ring_buffer_;
+  RingBufferType ring_buffer_;
 
   base::TimeDelta last_total_paint_time_;
   base::TimeDelta last_total_rasterize_time_;
diff --git a/cc/ring_buffer.h b/cc/ring_buffer.h
index 5e015fe..6452e2f 100644
--- a/cc/ring_buffer.h
+++ b/cc/ring_buffer.h
@@ -50,6 +50,62 @@ class RingBuffer {
     current_index_ = 0;
   }
 
+  // Iterator has const access to the RingBuffer it got retrieved from.
+  class Iterator {
+   public:
+    size_t index() const { return index_; }
+
+    const T* operator->() const { return &buffer_.ReadBuffer(index_); }
+    const T* operator*() const { return &buffer_.ReadBuffer(index_); }
+
+    Iterator& operator++() {
+      index_++;
+      if (index_ == size)
+        out_of_range_ = true;
+      return *this;
+    }
+
+    Iterator& operator--() {
+      if (index_ == 0)
+        out_of_range_ = true;
+      index_--;
+      return *this;
+    }
+
+    operator bool() const {
+      return buffer_.IsFilledIndex(index_) && !out_of_range_;
+    }
+
+   private:
+    Iterator(const RingBuffer<T, size>& buffer, size_t index)
+      : buffer_(buffer),
+        index_(index),
+        out_of_range_(false) {
+    }
+
+    const RingBuffer<T, size>& buffer_;
+    size_t index_;
+    bool out_of_range_;
+
+    friend class RingBuffer<T, size>;
+  };
+
+  // Returns an Iterator pointing to the oldest value in the buffer.
+  // Example usage (iterate from oldest to newest value):
+  //  for (RingBuffer<T, size>::Iterator it = ring_buffer.Begin(); it; ++it) ...
+  Iterator Begin() const {
+    if (current_index_ < size)
+      return Iterator(*this, size - current_index_);
+    return Iterator(*this, 0);
+  }
+
+  // Returns an Iterator pointing to the newest value in the buffer.
+  // Example usage (iterate backwards from newest to oldest value):
+  //  for (RingBuffer<T, size>::Iterator it = ring_buffer.End(); it; --it) ...
+  Iterator End() const {
+    return Iterator(*this, size - 1);
+  }
+
  private:
   inline size_t BufferIndex(size_t n) const {
     return (current_index_ + n) % size;