Revert 201791 "Fix various MediaSource related crashes on Android."

> Fix various MediaSource related crashes on Android.
> 
> This change fixes several issues that were causing crashes.
> - MediaPlayerHostMsg_DemuxerReady_Params was not initializing the xxx_codec
>   member variables so audio-only or video-only content would randomly be
>   flagged as A/V content with a random codec ID.
> - ChunkDemuxer::Stop() was not being called which could cause the delegate
>   to be accessed after it was destroyed.
> - MediaSourceDelegate was signalling that it had metadata before the 
>   ChunkDemuxer was even initialized. This caused the HTMLMediaElement to
>   signal that the load completed way too early and caused play() to be
>   called when the delegate wasn't ready.
> 
> BUG=233420
> TEST=webkitmediasource-play.html LayoutTest reliably runs w/o crashing now.
> 
> Review URL: https://chromiumcodereview.appspot.com/15754004

TBR=acolwell@chromium.org

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

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

6 files changed, 15 insertions, 80 deletions

diff --git a/media/base/android/demuxer_stream_player_params.cc b/media/base/android/demuxer_stream_player_params.cc
index 3ed1a8c..ab9fcaf 100644
--- a/media/base/android/demuxer_stream_player_params.cc
+++ b/media/base/android/demuxer_stream_player_params.cc
@@ -8,11 +8,9 @@ namespace media {
 
 MediaPlayerHostMsg_DemuxerReady_Params::
     MediaPlayerHostMsg_DemuxerReady_Params()
-    : audio_codec(kUnknownAudioCodec),
-      audio_channels(0),
+    : audio_channels(0),
       audio_sampling_rate(0),
       is_audio_encrypted(false),
-      video_codec(kUnknownVideoCodec),
       is_video_encrypted(false),
       duration_ms(0) {}
 
diff --git a/media/base/android/media_source_player.cc b/media/base/android/media_source_player.cc
index f6cbd48..b29d182 100644
--- a/media/base/android/media_source_player.cc
+++ b/media/base/android/media_source_player.cc
@@ -208,6 +208,7 @@ MediaSourcePlayer::MediaSourcePlayer(
       waiting_for_audio_data_(false),
       waiting_for_video_data_(false),
       weak_this_(this) {
+  OnMediaMetadataChanged(duration_, width_, height_, false);
 }
 
 MediaSourcePlayer::~MediaSourcePlayer() {
@@ -220,11 +221,9 @@ void MediaSourcePlayer::SetVideoSurface(jobject surface) {
     return;
   }
 
-  if (HasVideo()) {
-    video_decoder_job_.reset(new VideoDecoderJob(
-        base::MessageLoopProxy::current(), video_codec_,
-        gfx::Size(width_, height_), surface));
-  }
+  video_decoder_job_.reset(new VideoDecoderJob(
+      base::MessageLoopProxy::current(), video_codec_,
+      gfx::Size(width_, height_), surface));
 
   if (pending_play_)
     StartInternal();
diff --git a/webkit/media/android/media_source_delegate.cc b/webkit/media/android/media_source_delegate.cc
index 66e7563..bf15894 100644
--- a/webkit/media/android/media_source_delegate.cc
+++ b/webkit/media/android/media_source_delegate.cc
@@ -88,26 +88,7 @@ MediaSourceDelegate::MediaSourceDelegate(
   }
 }
 
-MediaSourceDelegate::~MediaSourceDelegate() {
-  DVLOG(1) << "MediaSourceDelegate::~MediaSourceDelegate() : " << player_id_;
-  DCHECK(!chunk_demuxer_);
-}
-
-void MediaSourceDelegate::Destroy() {
-  DVLOG(1) << "MediaSourceDelegate::Destroy() : " << player_id_;
-  if (!chunk_demuxer_) {
-    delete this;
-    return;
-  }
-
-  update_network_state_cb_.Reset();
-  media_source_.reset();
-  client_ = NULL;
-  proxy_ = NULL;
-
-  chunk_demuxer_->Stop(
-      BIND_TO_RENDER_LOOP(&MediaSourceDelegate::OnDemuxerStopDone));
-}
+MediaSourceDelegate::~MediaSourceDelegate() {}
 
 void MediaSourceDelegate::Initialize(
     WebKit::WebMediaSource* media_source,
@@ -218,8 +199,6 @@ WebMediaPlayer::MediaKeyException MediaSourceDelegate::CancelKeyRequest(
 }
 
 void MediaSourceDelegate::Seek(base::TimeDelta time) {
-  DVLOG(1) << "MediaSourceDelegate::Seek(" << time.InSecondsF() << ") : "
-           << player_id_;
   seeking_ = true;
   DCHECK(chunk_demuxer_);
   if (!chunk_demuxer_)
@@ -248,8 +227,6 @@ void MediaSourceDelegate::SetDuration(base::TimeDelta duration) {
 
 void MediaSourceDelegate::OnReadFromDemuxer(media::DemuxerStream::Type type,
                                             bool seek_done) {
-  DVLOG(1) << "MediaSourceDelegate::OnReadFromDemuxer(" << type
-           << ", " << seek_done << ") : " << player_id_;
   if (seeking_ && !seek_done)
       return;  // Drop the request during seeking.
   seeking_ = false;
@@ -278,7 +255,6 @@ void MediaSourceDelegate::OnBufferReady(
     size_t index,
     DemuxerStream::Status status,
     const scoped_refptr<media::DecoderBuffer>& buffer) {
-  DVLOG(1) << "MediaSourceDelegate::OnBufferReady() : " << player_id_;
   DCHECK(status == DemuxerStream::kAborted ||
          index < params->access_units.size());
   bool is_audio = stream->type() == DemuxerStream::AUDIO;
@@ -366,8 +342,6 @@ void MediaSourceDelegate::OnBufferReady(
 
 void MediaSourceDelegate::OnDemuxerError(
     media::PipelineStatus status) {
-  DVLOG(1) << "MediaSourceDelegate::OnDemuxerError(" << status << ") : "
-           << player_id_;
   if (status != media::PIPELINE_OK) {
     DCHECK(status == media::DEMUXER_ERROR_COULD_NOT_OPEN ||
            status == media::DEMUXER_ERROR_COULD_NOT_PARSE ||
@@ -380,8 +354,6 @@ void MediaSourceDelegate::OnDemuxerError(
 
 void MediaSourceDelegate::OnDemuxerInitDone(
     media::PipelineStatus status) {
-  DVLOG(1) << "MediaSourceDelegate::OnDemuxerInitDone(" << status << ") : "
-           << player_id_;
   if (status != media::PIPELINE_OK) {
     OnDemuxerError(status);
     return;
@@ -389,12 +361,6 @@ void MediaSourceDelegate::OnDemuxerInitDone(
   NotifyDemuxerReady("");
 }
 
-void MediaSourceDelegate::OnDemuxerStopDone() {
-  DVLOG(1) << "MediaSourceDelegate::OnDemuxerStopDone() : " << player_id_;
-  chunk_demuxer_.reset();
-  delete this;
-}
-
 void MediaSourceDelegate::NotifyDemuxerReady(const std::string& key_system) {
   MediaPlayerHostMsg_DemuxerReady_Params params;
   DemuxerStream* audio_stream = chunk_demuxer_->GetStream(DemuxerStream::AUDIO);
@@ -433,9 +399,6 @@ void MediaSourceDelegate::NotifyDemuxerReady(const std::string& key_system) {
 }
 
 void MediaSourceDelegate::OnDemuxerOpened() {
-  if (!media_source_)
-    return;
-
   media_source_->open(new WebMediaSourceClientImpl(
       chunk_demuxer_.get(), base::Bind(&LogMediaSourceError, media_log_)));
 }
@@ -444,9 +407,6 @@ void MediaSourceDelegate::OnKeyError(const std::string& key_system,
                                      const std::string& session_id,
                                      media::Decryptor::KeyError error_code,
                                      int system_code) {
-  if (!client_)
-    return;
-
   client_->keyError(
       WebString::fromUTF8(key_system),
       WebString::fromUTF8(session_id),
@@ -462,9 +422,6 @@ void MediaSourceDelegate::OnKeyMessage(const std::string& key_system,
   DLOG_IF(WARNING, !default_url.empty() && !default_url_gurl.is_valid())
       << "Invalid URL in default_url: " << default_url;
 
-  if (!client_)
-    return;
-
   client_->keyMessage(WebString::fromUTF8(key_system),
                       WebString::fromUTF8(session_id),
                       reinterpret_cast<const uint8*>(message.data()),
@@ -474,11 +431,7 @@ void MediaSourceDelegate::OnKeyMessage(const std::string& key_system,
 
 void MediaSourceDelegate::OnKeyAdded(const std::string& key_system,
                                      const std::string& session_id) {
-  if (!client_)
-    return;
-
   NotifyDemuxerReady(key_system);
-
   client_->keyAdded(WebString::fromUTF8(key_system),
                     WebString::fromUTF8(session_id));
 }
@@ -489,7 +442,7 @@ void MediaSourceDelegate::OnNeedKey(const std::string& key_system,
                                     scoped_ptr<uint8[]> init_data,
                                     int init_data_size) {
   // Do not fire NeedKey event if encrypted media is not enabled.
-  if (!client_ || !decryptor_)
+  if (!decryptor_)
     return;
 
   CHECK(init_data_size >= 0);
diff --git a/webkit/media/android/media_source_delegate.h b/webkit/media/android/media_source_delegate.h
index 18b93e5..f2cf340 100644
--- a/webkit/media/android/media_source_delegate.h
+++ b/webkit/media/android/media_source_delegate.h
@@ -40,20 +40,13 @@ class MediaSourceDelegate : public media::DemuxerHost {
   typedef base::Callback<void(WebKit::WebMediaPlayer::NetworkState)>
       UpdateNetworkStateCB;
 
-  // Helper class used by scoped_ptr to destroy an instance of
-  // MediaSourceDelegate.
-  class Destroyer {
-   public:
-    inline void operator()(void* media_source_delegate) const {
-      static_cast<MediaSourceDelegate*>(media_source_delegate)->Destroy();
-    }
-  };
-
   MediaSourceDelegate(WebKit::WebFrame* frame,
                       WebKit::WebMediaPlayerClient* client,
                       WebMediaPlayerProxyAndroid* proxy,
                       int player_id,
                       media::MediaLog* media_log);
+  virtual ~MediaSourceDelegate();
+
   // Initialize the MediaSourceDelegate. |media_source| will be owned by
   // this object after this call.
   void Initialize(WebKit::WebMediaSource* media_source,
@@ -87,14 +80,6 @@ class MediaSourceDelegate : public media::DemuxerHost {
   void OnReadFromDemuxer(media::DemuxerStream::Type type, bool seek_done);
 
  private:
-  friend MediaSourceDelegate::Destroyer;
-
-  // This is private to enforce use of the Destroyer.
-  virtual ~MediaSourceDelegate();
-
-  // Called by the Destroyer to destroy an instance of this object.
-  void Destroy();
-
   // Methods inherited from DemuxerHost.
   virtual void SetTotalBytes(int64 total_bytes) OVERRIDE;
   virtual void AddBufferedByteRange(int64 start, int64 end) OVERRIDE;
@@ -105,7 +90,6 @@ class MediaSourceDelegate : public media::DemuxerHost {
 
   // Callbacks for ChunkDemuxer & Decryptor.
   void OnDemuxerInitDone(media::PipelineStatus status);
-  void OnDemuxerStopDone();
   void OnDemuxerOpened();
   void OnKeyAdded(const std::string& key_system, const std::string& session_id);
   void OnKeyError(const std::string& key_system,
@@ -143,7 +127,7 @@ class MediaSourceDelegate : public media::DemuxerHost {
 
   base::WeakPtrFactory<MediaSourceDelegate> weak_this_;
 
-  WebKit::WebMediaPlayerClient* client_;
+  WebKit::WebMediaPlayerClient* const client_;
   WebMediaPlayerProxyAndroid* proxy_;
   int player_id_;
 
@@ -179,3 +163,4 @@ class MediaSourceDelegate : public media::DemuxerHost {
 
 }  // namespace webkit_media
 #endif  // WEBKIT_MEDIA_ANDROID_MEDIA_SOURCE_DELEGATE_H_
+
diff --git a/webkit/media/android/webmediaplayer_android.cc b/webkit/media/android/webmediaplayer_android.cc
index acfeb5d..973c5f2 100644
--- a/webkit/media/android/webmediaplayer_android.cc
+++ b/webkit/media/android/webmediaplayer_android.cc
@@ -21,6 +21,7 @@
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebMediaPlayerClient.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebMediaSource.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebView.h"
+#include "webkit/media/android/media_source_delegate.h"
 #include "webkit/media/android/webmediaplayer_manager_android.h"
 #include "webkit/media/android/webmediaplayer_proxy_android.h"
 #include "webkit/media/webmediaplayer_util.h"
@@ -551,7 +552,6 @@ void WebMediaPlayerAndroid::Detach() {
     stream_id_ = 0;
   }
 
-  media_source_delegate_.reset();
   current_frame_ = NULL;
   manager_ = NULL;
   proxy_ = NULL;
diff --git a/webkit/media/android/webmediaplayer_android.h b/webkit/media/android/webmediaplayer_android.h
index f344086..c492910 100644
--- a/webkit/media/android/webmediaplayer_android.h
+++ b/webkit/media/android/webmediaplayer_android.h
@@ -19,7 +19,6 @@
 #include "third_party/WebKit/Source/Platform/chromium/public/WebURL.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebMediaPlayer.h"
 #include "ui/gfx/rect_f.h"
-#include "webkit/media/android/media_source_delegate.h"
 #include "webkit/media/android/stream_texture_factory_android.h"
 
 namespace media {
@@ -35,6 +34,8 @@ class WebLayerImpl;
 }
 
 namespace webkit_media {
+
+class MediaSourceDelegate;
 class WebMediaPlayerManagerAndroid;
 class WebMediaPlayerProxyAndroid;
 
@@ -303,8 +304,7 @@ class WebMediaPlayerAndroid
   gfx::RectF last_computed_rect_;
 #endif
 
-  scoped_ptr<MediaSourceDelegate,
-             MediaSourceDelegate::Destroyer> media_source_delegate_;
+  scoped_ptr<MediaSourceDelegate> media_source_delegate_;
 
   // Proxy object that delegates method calls on Render Thread.
   // This object is created on the Render Thread and is only called in the