Remove reference counting from media::Demuxer and friends.

media/filters/decrypting_demuxer_stream.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/filters/decrypting_demuxer_stream.h"
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/message_loop_proxy.h"
 #include "media/base/audio_decoder_config.h"
 #include "media/base/video_decoder_config.h"
 #include "media/base/bind_to_loop.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/decryptor.h"
 #include "media/base/demuxer_stream.h"
 #include "media/base/pipeline.h"
 
 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()) ||
           (stream->type() == DemuxerStream::VIDEO &&
            stream->video_decoder_config().IsValidConfig() &&
            stream->video_decoder_config().is_encrypted()));
 }
 
 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),
       key_added_while_decrypt_pending_(false) {
 }
 
 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) {
   DVLOG(3) << "Read()";
   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK_EQ(state_, kIdle) << state_;
   DCHECK(!read_cb.is_null());
   CHECK(read_cb_.is_null()) << "Overlapping reads are not supported.";
 
   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());
 
   reset_cb_ = BindToCurrentLoop(closure);
 
   decryptor_->CancelDecrypt(GetDecryptorStreamType());
 
   // Reset() cannot complete if the read callback is still pending.
   // Defer the resetting process in this case. The |reset_cb_| will be fired
@@ skipping 30 matching lines @@
   DCHECK(state_ != kUninitialized && state_ != kDecryptorRequested) << state_;
   return stream_type_;
 }
 
 void DecryptingDemuxerStream::EnableBitstreamConverter() {
   demuxer_stream_->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());
   DCHECK_EQ(state_, kDecryptorRequested) << state_;
   DCHECK(!init_cb_.is_null());
   DCHECK(!set_decryptor_ready_cb_.is_null());
 
   set_decryptor_ready_cb_.Reset();
   decryptor_ = decryptor;
 
   decryptor_->RegisterNewKeyCB(
       GetDecryptorStreamType(),
       BIND_TO_LOOP(&DecryptingDemuxerStream::OnKeyAdded));
 
   state_ = kIdle;
   base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK);
 }
 
 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());
   DCHECK_EQ(buffer != NULL, status == kOk) << status;
 
   if (!reset_cb_.is_null()) {
     base::ResetAndReturn(&read_cb_).Run(kAborted, NULL);
     DoReset();
     return;
   }
@@ skipping 28 matching lines @@
   state_ = kPendingDecrypt;
   DecryptPendingBuffer();
 }
 
 void DecryptingDemuxerStream::DecryptPendingBuffer() {
   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK_EQ(state_, kPendingDecrypt) << state_;
   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);
   DCHECK(!read_cb_.is_null());
   DCHECK(pending_buffer_to_decrypt_);
 
   bool need_to_try_again_if_nokey = key_added_while_decrypt_pending_;
   key_added_while_decrypt_pending_ = false;
 
   if (!reset_cb_.is_null()) {
@@ skipping 50 matching lines @@
   DCHECK(read_cb_.is_null());
   state_ = kIdle;
   base::ResetAndReturn(&reset_cb_).Run();
 }
 
 Decryptor::StreamType DecryptingDemuxerStream::GetDecryptorStreamType() const {
   DCHECK(stream_type_ == AUDIO || stream_type_ == VIDEO);
   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));
 
   switch (stream_type_) {
     case AUDIO: {
       const AudioDecoderConfig& input_audio_config =
           stream->audio_decoder_config();
       audio_config_.reset(new AudioDecoderConfig());
       audio_config_->Initialize(input_audio_config.codec(),
@@ skipping 24 matching lines @@
       break;
     }
 
     default:
       NOTREACHED();
       return;
   }
 }
 
 }  // namespace media