Revert current time clamp on ended; clamp paused time instead.

Clamping the current time within Pipeline leads to a race
a race condition while using |renderer_ended_|.  The problem
that code was added for can mostly be fixed by instead clamping
the pause time in WebMediaPlayerImpl.

I've verified the HTMLMediaElement cleanup in another CL still
passes all tests: https://codereview.chromium.org/456343002/

BUG=446637
TEST=layout tests, unittests.

Review URL: https://codereview.chromium.org/843863002

Cr-Commit-Position: refs/heads/master@{#310686}

4 files changed, 21 insertions, 15 deletions

diff --git a/media/base/pipeline.cc b/media/base/pipeline.cc
index 3d210d4..9ce867d 100644
--- a/media/base/pipeline.cc
+++ b/media/base/pipeline.cc
@@ -163,17 +163,8 @@ void Pipeline::SetVolume(float volume) {
 
 TimeDelta Pipeline::GetMediaTime() const {
   base::AutoLock auto_lock(lock_);
-  if (!renderer_)
-    return TimeDelta();
-
-  // TODO(sriram): In some cases GetMediaTime() returns a value few
-  // milliseconds less than duration, even though playback has ended
-  // http://crbug.com/438581
-  TimeDelta media_time = renderer_->GetMediaTime();
-  if (renderer_ended_)
-    return duration_;
-
-  return std::min(media_time, duration_);
+  return renderer_ ? std::min(renderer_->GetMediaTime(), duration_)
+                   : TimeDelta();
 }
 
 Ranges<TimeDelta> Pipeline::GetBufferedTimeRanges() const {
diff --git a/media/base/pipeline_unittest.cc b/media/base/pipeline_unittest.cc
index b5e91bd..68e4590 100644
--- a/media/base/pipeline_unittest.cc
+++ b/media/base/pipeline_unittest.cc
@@ -618,8 +618,8 @@ TEST_F(PipelineTest, EndedCallback) {
   message_loop_.RunUntilIdle();
 
   EXPECT_CALL(callbacks_, OnEnded());
-  // There are cases where duration is reported wrong initially, so there is
-  // an OnDurationChange event fired again on OnEnded event if required.
+  // Since the |ended_cb_| is manually invoked above, the duration does not
+  // match the expected duration and is updated upon ended.
   EXPECT_CALL(callbacks_, OnDurationChange());
   text_stream()->SendEosNotification();
   message_loop_.RunUntilIdle();
diff --git a/media/blink/webmediaplayer_impl.cc b/media/blink/webmediaplayer_impl.cc
index 4b67c9d..7932443 100644
--- a/media/blink/webmediaplayer_impl.cc
+++ b/media/blink/webmediaplayer_impl.cc
@@ -273,7 +273,7 @@ void WebMediaPlayerImpl::pause() {
   pipeline_.SetPlaybackRate(0.0f);
   if (data_source_)
     data_source_->MediaIsPaused();
-  paused_time_ = pipeline_.GetMediaTime();
+  UpdatePausedTime();
 
   media_log_->AddEvent(media_log_->CreateEvent(MediaLogEvent::PAUSE));
 
@@ -713,7 +713,7 @@ void WebMediaPlayerImpl::OnPipelineSeeked(bool time_changed,
 
   // Update our paused time.
   if (paused_)
-    paused_time_ = pipeline_.GetMediaTime();
+    UpdatePausedTime();
 
   should_notify_time_changed_ = time_changed;
 }
@@ -993,4 +993,15 @@ WebMediaPlayerImpl::GetCurrentFrameFromCompositor() {
   return video_frame;
 }
 
+void WebMediaPlayerImpl::UpdatePausedTime() {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+
+  // pause() may be called after playback has ended and the HTMLMediaElement
+  // requires that currentTime() == duration() after ending.  We want to ensure
+  // |paused_time_| matches currentTime() in this case or a future seek() may
+  // incorrectly discard what it thinks is a seek to the existing time.
+  paused_time_ =
+      ended_ ? pipeline_.GetMediaDuration() : pipeline_.GetMediaTime();
+}
+
 }  // namespace media
diff --git a/media/blink/webmediaplayer_impl.h b/media/blink/webmediaplayer_impl.h
index db540f6..6e8de1f 100644
--- a/media/blink/webmediaplayer_impl.h
+++ b/media/blink/webmediaplayer_impl.h
@@ -218,6 +218,10 @@ class MEDIA_EXPORT WebMediaPlayerImpl
   void OnCdmAttached(blink::WebContentDecryptionModuleResult result,
                      bool success);
 
+  // Updates |paused_time_| to the current media time with consideration for the
+  // |ended_| state by clamping current time to duration upon |ended_|.
+  void UpdatePausedTime();
+
   blink::WebLocalFrame* frame_;
 
   // TODO(hclam): get rid of these members and read from the pipeline directly.