Remove reference counting from media::Demuxer and friends.

media/filters/decrypting_demuxer_stream.cc

Index: media/filters/decrypting_demuxer_stream.cc
diff --git a/media/filters/decrypting_demuxer_stream.cc b/media/filters/decrypting_demuxer_stream.cc
index 694116811aeaae9f3a79b756bb8c4034601746db..1bb61f62b0a82fca4d37184a6843bfe30aa591a2 100644
--- a/media/filters/decrypting_demuxer_stream.cc
+++ b/media/filters/decrypting_demuxer_stream.cc
@@ -20,10 +20,9 @@
 namespace media {
 
 #define BIND_TO_LOOP(function) \
-    media::BindToLoop(message_loop_, base::Bind(function, this))
+    media::BindToLoop(message_loop_, base::Bind(function, weak_this_))
 
-static bool IsStreamValidAndEncrypted(
-    const scoped_refptr<DemuxerStream>& stream) {
+static bool IsStreamValidAndEncrypted(DemuxerStream* stream) {
   return ((stream->type() == DemuxerStream::AUDIO &&
            stream->audio_decoder_config().IsValidConfig() &&
            stream->audio_decoder_config().is_encrypted()) ||
@@ -36,7 +35,9 @@ DecryptingDemuxerStream::DecryptingDemuxerStream(
     const scoped_refptr<base::MessageLoopProxy>& message_loop,
     const SetDecryptorReadyCB& set_decryptor_ready_cb)
     : message_loop_(message_loop),
+      weak_factory_(this),
       state_(kUninitialized),
+      demuxer_stream_(NULL),
       stream_type_(UNKNOWN),
       set_decryptor_ready_cb_(set_decryptor_ready_cb),
       decryptor_(NULL),
@@ -44,14 +45,23 @@ DecryptingDemuxerStream::DecryptingDemuxerStream(
 }
 
 void DecryptingDemuxerStream::Initialize(
-    const scoped_refptr<DemuxerStream>& stream,
+    DemuxerStream* stream,
     const PipelineStatusCB& status_cb) {
-  if (!message_loop_->BelongsToCurrentThread()) {

[acolwell GONE FROM CHROMIUM, 2013/04/17 20:24:53]

Would you mind moving these transformations to a separate CL. It would be nice
to verify that this doesn't break anything independent of the ref_counting
changes.

[scherkus (not reviewing), 2013/04/19 01:07:22]

Done and landed http://crrev.com/194721

-    message_loop_->PostTask(FROM_HERE, base::Bind(
-        &DecryptingDemuxerStream::DoInitialize, this, stream, status_cb));
-    return;
-  }
-  DoInitialize(stream, status_cb);
+  DVLOG(2) << "Initialize()";
+  DCHECK(message_loop_->BelongsToCurrentThread());
+  DCHECK_EQ(state_, kUninitialized) << state_;
+
+  DCHECK(!demuxer_stream_);
+  weak_this_ = weak_factory_.GetWeakPtr();
+  demuxer_stream_ = stream;
+  stream_type_ = stream->type();
+  init_cb_ = status_cb;
+
+  SetDecoderConfig(stream);
+
+  state_ = kDecryptorRequested;
+  set_decryptor_ready_cb_.Run(
+      BIND_TO_LOOP(&DecryptingDemuxerStream::SetDecryptor));
 }
 
 void DecryptingDemuxerStream::Read(const ReadCB& read_cb) {
@@ -64,17 +74,12 @@ void DecryptingDemuxerStream::Read(const ReadCB& read_cb) {
   read_cb_ = read_cb;
   state_ = kPendingDemuxerRead;
   demuxer_stream_->Read(
-      base::Bind(&DecryptingDemuxerStream::DecryptBuffer, this));
+      base::Bind(&DecryptingDemuxerStream::DecryptBuffer, weak_this_));
 }
 
 void DecryptingDemuxerStream::Reset(const base::Closure& closure) {
-  if (!message_loop_->BelongsToCurrentThread()) {
-    message_loop_->PostTask(FROM_HERE, base::Bind(
-        &DecryptingDemuxerStream::Reset, this, closure));
-    return;
-  }
-
   DVLOG(2) << "Reset() - state: " << state_;
+  DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK(state_ != kUninitialized && state_ != kDecryptorRequested) << state_;
   DCHECK(init_cb_.is_null());  // No Reset() during pending initialization.
   DCHECK(reset_cb_.is_null());
@@ -125,25 +130,6 @@ void DecryptingDemuxerStream::EnableBitstreamConverter() {
 
 DecryptingDemuxerStream::~DecryptingDemuxerStream() {}
 
-void DecryptingDemuxerStream::DoInitialize(
-    const scoped_refptr<DemuxerStream>& stream,
-    const PipelineStatusCB& status_cb) {
-  DVLOG(2) << "DoInitialize()";
-  DCHECK(message_loop_->BelongsToCurrentThread());
-  DCHECK_EQ(state_, kUninitialized) << state_;
-
-  DCHECK(!demuxer_stream_);
-  demuxer_stream_ = stream;
-  stream_type_ = stream->type();
-  init_cb_ = status_cb;
-
-  SetDecoderConfig(stream);
-
-  state_ = kDecryptorRequested;
-  set_decryptor_ready_cb_.Run(
-      BIND_TO_LOOP(&DecryptingDemuxerStream::SetDecryptor));
-}
-
 void DecryptingDemuxerStream::SetDecryptor(Decryptor* decryptor) {
   DVLOG(2) << "SetDecryptor()";
   DCHECK(message_loop_->BelongsToCurrentThread());
@@ -165,19 +151,7 @@ void DecryptingDemuxerStream::SetDecryptor(Decryptor* decryptor) {
 void DecryptingDemuxerStream::DecryptBuffer(
     DemuxerStream::Status status,
     const scoped_refptr<DecoderBuffer>& buffer) {
-  // In theory, we don't need to force post the task here, because we do a
-  // force task post in DeliverBuffer(). Therefore, even if
-  // demuxer_stream_->Read() execute the read callback on the same execution
-  // stack we are still fine. But it looks like a force post task makes the
-  // logic more understandable and manageable, so why not?
-  message_loop_->PostTask(FROM_HERE, base::Bind(
-      &DecryptingDemuxerStream::DoDecryptBuffer, this, status, buffer));
-}
-
-void DecryptingDemuxerStream::DoDecryptBuffer(
-    DemuxerStream::Status status,
-    const scoped_refptr<DecoderBuffer>& buffer) {
-  DVLOG(3) << "DoDecryptBuffer()";
+  DVLOG(3) << "DecryptBuffer()";
   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK_EQ(state_, kPendingDemuxerRead) << state_;
   DCHECK(!read_cb_.is_null());
@@ -226,25 +200,13 @@ void DecryptingDemuxerStream::DecryptPendingBuffer() {
   decryptor_->Decrypt(
       GetDecryptorStreamType(),
       pending_buffer_to_decrypt_,
-      base::Bind(&DecryptingDemuxerStream::DeliverBuffer, this));
+      BIND_TO_LOOP(&DecryptingDemuxerStream::DeliverBuffer));
 }
 
 void DecryptingDemuxerStream::DeliverBuffer(
     Decryptor::Status status,
     const scoped_refptr<DecoderBuffer>& decrypted_buffer) {
-  // We need to force task post here because the DecryptCB can be executed
-  // synchronously in Reset(). Instead of using more complicated logic in
-  // those function to fix it, why not force task post here to make everything
-  // simple and clear?
-  message_loop_->PostTask(FROM_HERE, base::Bind(
-      &DecryptingDemuxerStream::DoDeliverBuffer, this,
-      status, decrypted_buffer));
-}
-
-void DecryptingDemuxerStream::DoDeliverBuffer(
-    Decryptor::Status status,
-    const scoped_refptr<DecoderBuffer>& decrypted_buffer) {
-  DVLOG(3) << "DoDeliverBuffer() - status: " << status;
+  DVLOG(3) << "DeliverBuffer() - status: " << status;
   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK_EQ(state_, kPendingDecrypt) << state_;
   DCHECK_NE(status, Decryptor::kNeedMoreData);
@@ -315,8 +277,7 @@ Decryptor::StreamType DecryptingDemuxerStream::GetDecryptorStreamType() const {
   return stream_type_ == AUDIO ? Decryptor::kAudio : Decryptor::kVideo;
 }
 
-void DecryptingDemuxerStream::SetDecoderConfig(
-    const scoped_refptr<DemuxerStream>& stream) {
+void DecryptingDemuxerStream::SetDecoderConfig(DemuxerStream* stream) {

[acolwell GONE FROM CHROMIUM, 2013/04/17 20:24:53]

nit: All callers essentially pass demuxer_stream_ so we could just drop the
stream parameter altogether here.

   // The decoder selector or upstream demuxer make sure the stream is valid and
   // potentially encrypted.
   DCHECK(IsStreamValidAndEncrypted(stream));