Use AutoUnlock class and Lock::AssertAcquired() methods to verify expected behavior wrt locks in base classes.

Review URL: http://codereview.chromium.org/48168

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

2 files changed, 25 insertions, 27 deletions

diff --git a/media/filters/decoder_base.h b/media/filters/decoder_base.h
index de57d6e..8a2ef60 100644
--- a/media/filters/decoder_base.h
+++ b/media/filters/decoder_base.h
@@ -135,8 +135,6 @@ class DecoderBase : public Decoder {
  private:
   // The GCL compiler does not like .cc files that directly access members of
   // a base class.  This inline method helps.
-  // TODO(ralphl): Does it really help?  Remove this comment after try server
-  // reports success.
   FilterHost* host() const { return Decoder::host_; }
 
   // Schedules a task that will execute the ProcessTask method.
@@ -173,51 +171,49 @@ class DecoderBase : public Decoder {
   // if reads have happened, else false.  This method must be called with
   // |lock_| acquired.  If the method submits any reads, then it will Release()
   // the |lock_| when calling the demuxer and then re-Acquire() the |lock_|.
-  // TODO(ralphl): Update lock to have AssertAcquired and call it here
-  // TODO(ralphl): Fix AutoUnlock and use it here instead of Release/Acquire
   bool SubmitReads() {
+    lock_.AssertAcquired();
     bool did_read = false;
     if (IsRunning() &&
         pending_reads_ + input_queue_.size() < output_queue_.size()) {
       did_read = true;
       size_t read = output_queue_.size() - pending_reads_ - input_queue_.size();
       pending_reads_ += read;
-      // Release |lock_| before calling the demuxer.
-      lock_.Release();
-      while (read) {
-        demuxer_stream_->Read(new AssignableBuffer<DecoderBase, Buffer>(this));
-        --read;
+      {
+        AutoUnlock unlock(lock_);
+        while (read) {
+          demuxer_stream_->
+              Read(new AssignableBuffer<DecoderBase, Buffer>(this));
+          --read;
+        }
       }
-      lock_.Acquire();
     }
     return did_read;
   }
 
   // If the |input_queue_| has any buffers, this method will call the derived
   // class's OnDecode() method.
-  // TODO(ralphl): Update lock to have AssertAcquired and call it here
-  // TODO(ralphl): Fix AutoUnlock and use it here instead of Release/Acquire
   bool ProcessInput() {
+    lock_.AssertAcquired();
     bool did_decode = false;
     while (IsRunning() && !input_queue_.empty()) {
       did_decode = true;
       Buffer* input = input_queue_.front();
       input_queue_.pop_front();
       // Release |lock_| before calling the derived class to do the decode.
-      lock_.Release();
-      OnDecode(input);
-      input->Release();
-      lock_.Acquire();
+      {
+        AutoUnlock unlock(lock_);
+        OnDecode(input);
+        input->Release();
+      }
     }
     return did_decode;
   }
 
-
   // Removes any buffers from the |result_queue_| and assigns them to a pending
   // read Assignable buffer in the |output_queue_|.
-  // TODO(ralphl): Update lock to have AssertAcquired and call it here
-  // TODO(ralphl): Fix AutoUnlock and use it here instead of Release/Acquire
   bool ProcessOutput() {
+    lock_.AssertAcquired();
     bool called_renderer = false;
     while (IsRunning() && !output_queue_.empty() && !result_queue_.empty()) {
       called_renderer = true;
@@ -226,18 +222,20 @@ class DecoderBase : public Decoder {
       Assignable<Output>* assignable_output = output_queue_.front();
       output_queue_.pop_front();
       // Release |lock_| before calling the renderer.
-      lock_.Release();
-      assignable_output->SetBuffer(output);
-      output->Release();
-      assignable_output->OnAssignment();
-      assignable_output->Release();
-      lock_.Acquire();
+      {
+        AutoUnlock unlock(lock_);
+        assignable_output->SetBuffer(output);
+        output->Release();
+        assignable_output->OnAssignment();
+        assignable_output->Release();
+      }
     }
     return called_renderer;
   }
 
   // Throw away all buffers in all queues.
   void DiscardQueues() {
+    lock_.AssertAcquired();
     while (!input_queue_.empty()) {
       input_queue_.front()->Release();
       input_queue_.pop_front();
diff --git a/media/filters/video_renderer_base.cc b/media/filters/video_renderer_base.cc
index 9533bf3..57caf99 100644
--- a/media/filters/video_renderer_base.cc
+++ b/media/filters/video_renderer_base.cc
@@ -108,9 +108,9 @@ void VideoRendererBase::SubmitReads() {
   }
 }
 
-// Assumes |lock_| has been acquired!
 bool VideoRendererBase::UpdateQueue(base::TimeDelta time,
                                     VideoFrame* new_frame) {
+  lock_.AssertAcquired();
   bool updated_front = false;
 
   // If a new frame is passed in then put it at the back of the queue.  If the
@@ -143,8 +143,8 @@ bool VideoRendererBase::UpdateQueue(base::TimeDelta time,
   return updated_front;
 }
 
-// Assumes |lock_| has been acquired!
 void VideoRendererBase::DiscardAllFrames() {
+  lock_.AssertAcquired();
   while (!queue_.empty()) {
     queue_.front()->Release();
     queue_.pop_front();