Allow transitioning to HAVE_METADATA before pipeline initialization completes.

webkit/media/webmediaplayer_impl.cc

Index: webkit/media/webmediaplayer_impl.cc
diff --git a/webkit/media/webmediaplayer_impl.cc b/webkit/media/webmediaplayer_impl.cc
index 706ca8411d0ec36febdc81a3acccf6874c5a5df1..2d0dd3990ac4df735d2925cad458cd3dfe41e89b 100644
--- a/webkit/media/webmediaplayer_impl.cc
+++ b/webkit/media/webmediaplayer_impl.cc
@@ -132,7 +132,8 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
       audio_renderer_sink_(audio_renderer_sink),
       is_local_source_(false),
       supports_save_(true),
-      decryptor_(proxy_.get(), client, frame) {
+      decryptor_(proxy_.get(), client, frame),
+      starting_(false) {
   media_log_->AddEvent(
       media_log_->CreateEvent(media::MediaLogEvent::WEBMEDIAPLAYER_CREATED));
 
@@ -313,7 +314,7 @@ bool WebMediaPlayerImpl::supportsSave() const {
 void WebMediaPlayerImpl::seek(float seconds) {
   DCHECK_EQ(main_loop_, MessageLoop::current());
 
-  if (seeking_) {
+  if (starting_ || seeking_) {
     pending_seek_ = true;
     pending_seek_seconds_ = seconds;
     proxy_->DemuxerCancelPendingSeek();
@@ -765,35 +766,9 @@ void WebMediaPlayerImpl::Repaint() {
   GetClient()->repaint();
 }
 
-void WebMediaPlayerImpl::OnPipelineInitialize(PipelineStatus status) {
-  DCHECK_EQ(main_loop_, MessageLoop::current());
-  if (status != media::PIPELINE_OK) {
-    // Any error that occurs before the pipeline can initialize should be
-    // considered a format error.
-    SetNetworkState(WebMediaPlayer::NetworkStateFormatError);
-    // Repaint to trigger UI update.
-    Repaint();
-    return;
-  }
-
-  if (!hasVideo())
-    GetClient()->disableAcceleratedCompositing();
-
-  if (is_local_source_)
-    SetNetworkState(WebMediaPlayer::NetworkStateLoaded);
-
-  SetReadyState(WebMediaPlayer::ReadyStateHaveMetadata);
-  // Fire canplaythrough immediately after playback begins because of
-  // crbug.com/106480.
-  // TODO(vrk): set ready state to HaveFutureData when bug above is fixed.
-  SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);
-
-  // Repaint to trigger UI update.
-  Repaint();
-}
-
 void WebMediaPlayerImpl::OnPipelineSeek(PipelineStatus status) {
   DCHECK_EQ(main_loop_, MessageLoop::current());
+  starting_ = false;
   seeking_ = false;
   if (pending_seek_) {
     pending_seek_ = false;
@@ -824,6 +799,15 @@ void WebMediaPlayerImpl::OnPipelineEnded(PipelineStatus status) {
 
 void WebMediaPlayerImpl::OnPipelineError(PipelineStatus error) {
   DCHECK_EQ(main_loop_, MessageLoop::current());
+
+  if (ready_state_ == WebMediaPlayer::ReadyStateHaveNothing) {
+    // Any error that occurs before reaching ReadyStateHaveMetadata should
+    // be considered a format error.
+    SetNetworkState(WebMediaPlayer::NetworkStateFormatError);

[scherkus (not reviewing), 2012/08/15 23:43:16]

does this impact any layout tests?

do we have layout tests for stuff like this?

[acolwell GONE FROM CHROMIUM, 2012/08/16 00:54:18]

This does not break any existing tests if that is what you mean. This just moves
the logic from OnPipelineInitialize(). We are always in ReadyStateHaveNothing
when that method is called.
Yes. there are several tests that check for MEDIA_ERROR_SRC_NOT_SUPPORTED which
is what this error eventually turns into.

+    Repaint();
+    return;
+  }
+
   switch (error) {
     case media::PIPELINE_OK:
       NOTREACHED() << "PIPELINE_OK isn't an error!";
@@ -871,6 +855,23 @@ void WebMediaPlayerImpl::OnPipelineError(PipelineStatus error) {
   Repaint();
 }
 
+void WebMediaPlayerImpl::OnPipelineReadyState(
+    media::Pipeline::ReadyState readyState) {

[scherkus (not reviewing), 2012/08/15 23:43:16]

unix_hacker

[acolwell GONE FROM CHROMIUM, 2012/08/16 00:54:18]

Done.

+  DVLOG(1) << "OnPipelineReadyState(" << readyState << ")";
+
+  switch (readyState) {
+    case media::Pipeline::kHaveMetadata:
+      SetReadyState(WebMediaPlayer::ReadyStateHaveMetadata);
+      break;
+    case media::Pipeline::kHaveEnoughData:
+      SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);
+      break;
+  }
+
+  // Repaint to trigger UI update.
+  Repaint();
+}
+
 void WebMediaPlayerImpl::OnDemuxerOpened() {
   DCHECK_EQ(main_loop_, MessageLoop::current());
 
@@ -965,11 +966,13 @@ void WebMediaPlayerImpl::NotifyDownloading(bool is_downloading) {
 }
 
 void WebMediaPlayerImpl::StartPipeline() {
+  starting_ = true;
   pipeline_->Start(
       filter_collection_.Pass(),
       base::Bind(&WebMediaPlayerProxy::PipelineEndedCallback, proxy_.get()),
       base::Bind(&WebMediaPlayerProxy::PipelineErrorCallback, proxy_.get()),
-      base::Bind(&WebMediaPlayerProxy::PipelineInitializationCallback,
+      base::Bind(&WebMediaPlayerProxy::PipelineSeekCallback, proxy_.get()),
+      base::Bind(&WebMediaPlayerProxy::PipelineReadyStateCallback,
                  proxy_.get()));
 }
 
@@ -984,6 +987,17 @@ void WebMediaPlayerImpl::SetNetworkState(WebMediaPlayer::NetworkState state) {
 void WebMediaPlayerImpl::SetReadyState(WebMediaPlayer::ReadyState state) {
   DCHECK_EQ(main_loop_, MessageLoop::current());
   DVLOG(1) << "SetReadyState: " << state;
+
+  if (ready_state_ == WebMediaPlayer::ReadyStateHaveNothing &&
+      state == WebMediaPlayer::ReadyStateHaveMetadata) {

[scherkus (not reviewing), 2012/08/15 23:43:16]

this assumes we always set to metadata at some point in time

typical practice in HTMLMediaElement land is to use >= -- thoughts?

[acolwell GONE FROM CHROMIUM, 2012/08/16 00:54:18]

Good point. Changed to >=

+    if (!hasVideo())
+      GetClient()->disableAcceleratedCompositing();
+  } else if (state == WebMediaPlayer::ReadyStateHaveEnoughData) {
+    if (is_local_source_ &&
+        network_state_ == WebMediaPlayer::NetworkStateLoading)
+      SetNetworkState(WebMediaPlayer::NetworkStateLoaded);

[scherkus (not reviewing), 2012/08/15 23:43:16]

{} for if statements >1 line

[acolwell GONE FROM CHROMIUM, 2012/08/16 00:54:18]

Done.

+  }
+
   ready_state_ = state;
   // Always notify to ensure client has the latest value.
   GetClient()->readyStateChanged();