Fix media timeline so that thumb never exceeds buffered data

media/base/clock.cc

Index: media/base/clock.cc
diff --git a/media/base/clock.cc b/media/base/clock.cc
index 4cdc021780e73306afc5143f2361597b3d195c52..4ec1d4859fcc479d66dcf41c01bd93517b9f5b8c 100644
--- a/media/base/clock.cc
+++ b/media/base/clock.cc
@@ -10,9 +10,8 @@
 namespace media {
 
 Clock::Clock(TimeProvider* time_provider)
-    : time_provider_(time_provider),
-      playing_(false),
-      playback_rate_(1.0f) {
+    : time_provider_(time_provider) {
+  Reset();
 }
 
 Clock::~Clock() {}
@@ -23,44 +22,64 @@ bool Clock::IsPlaying() const {
 
 base::TimeDelta Clock::Play() {
   DCHECK(!playing_);
-  reference_ = GetTimeFromProvider();
+  UpdateReferencePoints();
   playing_ = true;
   return media_time_;
 }
 
 base::TimeDelta Clock::Pause() {
   DCHECK(playing_);
-  // Save our new accumulated amount of media time.
-  media_time_ = Elapsed();
+  UpdateReferencePoints();
   playing_ = false;
   return media_time_;
 }
 
 void Clock::SetPlaybackRate(float playback_rate) {
-  if (playing_) {
-    base::Time time = GetTimeFromProvider();
-    media_time_ = ElapsedViaProvidedTime(time);
-    reference_ = time;
-  }
+  UpdateReferencePoints();
   playback_rate_ = playback_rate;
 }
 
-void Clock::SetTime(const base::TimeDelta& time) {
-  if (time == kNoTimestamp()) {
+void Clock::SetTime(base::TimeDelta current_time, base::TimeDelta max_time) {
+  DCHECK(current_time <= max_time);
+  if (current_time == kNoTimestamp()) {

[acolwell GONE FROM CHROMIUM, 2012/01/24 18:13:59]

Should we convert this into a DCHECK? Any idea why this is here or why it was
added?

[vrk (LEFT CHROMIUM), 2012/01/25 00:09:32]

Introduced in this CL: http://codereview.chromium.org/173072

It was originally a DCHECK, then there was conversation to make it an if
instead. http://codereview.chromium.org/173072/diff/21/23#newcode39

But yeah, judging by the code, it looks like having a current_time ==
kNoTimestamp() is an indication of programmer error. Changing it to a DCHECK.

     NOTREACHED();
     return;
   }
-  if (playing_) {
-    reference_ = GetTimeFromProvider();
-  }
-  media_time_ = time;
+  underflow_ = false;
+  UpdateReferencePoints(current_time);
+  SetMaxTime(max_time);
 }
 
-base::TimeDelta Clock::Elapsed() const {
-  if (!playing_) {
+base::TimeDelta Clock::Elapsed() {
+  if (duration_ == kNoTimestamp())
+    return base::TimeDelta();
+
+  if (!IsTimeProgressing())
     return media_time_;
+
+  base::TimeDelta elapsed = EstimatedElapsedTime();
+  if (max_time_ != kNoTimestamp() && elapsed > max_time_) {
+    UpdateReferencePoints(max_time_);
+    underflow_ = true;
+    elapsed = max_time_;
   }
-  return ElapsedViaProvidedTime(GetTimeFromProvider());
+
+  return elapsed;
+}
+
+void Clock::SetMaxTime(base::TimeDelta max_time) {
+  DCHECK(max_time != kNoTimestamp());
+  UpdateReferencePoints();
+  max_time_ = ClampToValidTimeRange(max_time);
+
+  DCHECK(media_time_ <= max_time_);
+  underflow_ = false;
+}
+
+void Clock::SetDuration(base::TimeDelta duration) {
+  DCHECK(duration_ == kNoTimestamp());
+  DCHECK(duration > base::TimeDelta());
+  duration_ = duration;
 }
 
 base::TimeDelta Clock::ElapsedViaProvidedTime(const base::Time& time) const {
@@ -71,10 +90,58 @@ base::TimeDelta Clock::ElapsedViaProvidedTime(const base::Time& time) const {
 }
 
 base::Time Clock::GetTimeFromProvider() const {
-  if (time_provider_) {
+  if (time_provider_)
     return time_provider_();
-  }
   return base::Time();
 }
 
+base::TimeDelta Clock::ClampToValidTimeRange(base::TimeDelta time) const {
+  if (duration_ == kNoTimestamp())
+    return base::TimeDelta();
+  return std::max(std::min(time, duration_), base::TimeDelta());
+}
+
+void Clock::EndOfStream() {
+  Pause();
+  SetTime(Duration(), Duration());
+}
+
+base::TimeDelta Clock::Duration() const {
+  if (duration_ == kNoTimestamp())
+    return base::TimeDelta();
+  return duration_;
+}
+
+bool Clock::IsTimeProgressing() const {
+  return playing_ && !underflow_;
+}
+
+void Clock::UpdateReferencePoints() {
+  if (IsTimeProgressing())

[acolwell GONE FROM CHROMIUM, 2012/01/24 18:13:59]

I think this function body can be replaced with
UpdateReferencePoints(Elapsed()). If so just inline IsTimeProgressing() in
Elapsed() since that's the only place it is used.

[vrk (LEFT CHROMIUM), 2012/01/25 00:09:32]

Yes, you are right! Done.

+    media_time_ = Elapsed();
+  else
+    DCHECK(media_time_ == Elapsed());
+  reference_ = GetTimeFromProvider();
+}
+
+void Clock::UpdateReferencePoints(base::TimeDelta current_time) {
+  media_time_ = ClampToValidTimeRange(current_time);
+  reference_ = GetTimeFromProvider();
+}
+
+base::TimeDelta Clock::EstimatedElapsedTime() {
+  return ClampToValidTimeRange(
+      ElapsedViaProvidedTime(GetTimeFromProvider()));
+}
+
+void Clock::Reset() {
+  playing_ = false;
+  playback_rate_ = 1.0f;
+  max_time_ = kNoTimestamp();
+  duration_ = kNoTimestamp();
+  media_time_ = base::TimeDelta();
+  reference_ = base::Time();
+  underflow_ = false;
+}
+
 }  // namespace media