An adhoc fix for memory leak when playing a badly muxed video file

Since the current data souce implementation for media resource loading
assumes forward reading, when playing some badly muxed video files that
has out-of-order read patterns a memory leak is observed. Here's the reason:
1. The video file has out-of-order read pattern
2. A lot of BufferedResourceLoader is created and destroyed to accomodate
   the seek requests that cannot be served by the buffer.
3. BufferedResourceLoader is not destroyed immediately when a new request
   is made, it is destroyed when the request has completed (triggered by
   cancelling the request).
4. Since request completion comes at a much lower rate than creation of
   BufferedResourceLoader, a lot of BufferedResourceLoader are remain
   alive and keep increasing, a leak is resulted.

Adhoc solution:
   We can destroy all buffers in BufferedResourceLoader when it is called
   to stop. This can greatly reduce the leak amount. Although a lot of
   BufferedResourceLoader are still alive, they will be destroyed eventually
   (since a request completion is received due to cancelling of the request).

A better solution would be to destroy the BufferedResourceLoader right away
after it is stopped and fix the leak in ResourceDispatcher so we don't need
to lengthen the lifetime of BufferedResourceLoader.

CL for fixing ResourceDispatcher:
http://codereview.chromium.org/115396

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

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

2 files changed, 15 insertions, 9 deletions

diff --git a/chrome/renderer/media/buffered_data_source.cc b/chrome/renderer/media/buffered_data_source.cc
index 67d3b9d..23eb7e7 100644
--- a/chrome/renderer/media/buffered_data_source.cc
+++ b/chrome/renderer/media/buffered_data_source.cc
@@ -5,6 +5,7 @@
 #include "base/compiler_specific.h"
 #include "base/message_loop.h"
 #include "base/process_util.h"
+#include "base/stl_util-inl.h"
 #include "base/string_util.h"
 #include "chrome/common/extensions/url_pattern.h"
 #include "chrome/renderer/media/buffered_data_source.h"
@@ -56,13 +57,10 @@ BufferedResourceLoader::BufferedResourceLoader(int32 routing_id,
       first_byte_position_(first_byte_position),
       last_byte_position_(last_byte_position),
       render_loop_(RenderThread::current()->message_loop()) {
-  AddRef();
 }
 
 BufferedResourceLoader::~BufferedResourceLoader() {
-  for (size_t i = 0; i < buffers_.size(); ++i)
-    delete buffers_[i];
-  buffers_.clear();
+  STLDeleteElements<BufferQueue>(&buffers_);
 }
 
 int BufferedResourceLoader::Start(net::CompletionCallback* start_callback) {
@@ -142,10 +140,15 @@ int BufferedResourceLoader::Start(net::CompletionCallback* start_callback) {
 }
 
 void BufferedResourceLoader::Stop() {
+  BufferQueue delete_queue;
   {
     AutoLock auto_lock(lock_);
     stopped_ = true;
+    // Use of |buffers_| is protected by the lock, we can destroy it safely.
+    delete_queue.swap(buffers_);
+    buffers_.clear();
   }
+  STLDeleteElements<BufferQueue>(&delete_queue);
 
   // Wakes up the waiting thread so they can catch the stop signal.
   render_loop_->PostTask(FROM_HERE,
@@ -375,18 +378,17 @@ void BufferedResourceLoader::OnCompletedRequest(
 
   if (async_start_)
     InvokeAndResetStartCallback(status.os_error());
-  Release();
 }
 
 /////////////////////////////////////////////////////////////////////////////
 // BufferedResourceLoader, private
 void BufferedResourceLoader::AppendToBuffer(const uint8* data, size_t size) {
-  scoped_ptr<Buffer> buffer(new Buffer(size));
-  memcpy(buffer->data.get(), data, size);
   {
     AutoLock auto_lock(lock_);
     if (!stopped_) {
-      buffers_.push_back(buffer.release());
+      Buffer* buffer = new Buffer(size);
+      memcpy(buffer->data.get(), data, size);
+      buffers_.push_back(buffer);
       buffered_bytes_ += size;
     }
   }
@@ -430,6 +432,7 @@ void BufferedResourceLoader::OnDestroy() {
   DCHECK(MessageLoop::current() == render_loop_);
   if (bridge_.get()) {
     bridge_->Cancel();
+    bridge_.reset();
   }
 }
 
diff --git a/chrome/renderer/media/buffered_data_source.h b/chrome/renderer/media/buffered_data_source.h
index cc1a0bb..9072275 100644
--- a/chrome/renderer/media/buffered_data_source.h
+++ b/chrome/renderer/media/buffered_data_source.h
@@ -100,6 +100,8 @@ class BufferedResourceLoader :
  private:
   // Append buffer to the queue of buffers.
   void AppendToBuffer(const uint8* buffer, size_t size);
+
+  // Destroy buffers in |buffers_|.
   void SignalComplete();
   bool ShouldEnableDefer();
   bool ShouldDisableDefer();
@@ -126,7 +128,8 @@ class BufferedResourceLoader :
   int64 offset_;
   int64 content_length_;
 
-  std::deque<Buffer*> buffers_;
+  typedef std::deque<Buffer*> BufferQueue;
+  BufferQueue buffers_;
   size_t buffered_bytes_;
   size_t buffer_limit_;
   base::WaitableEvent buffer_event_;