Remove reference counting from BufferedResourceLoader.

Reference counting was originally added in r23274 when we were still using ResourceLoaderBridge for URL loading. After switching to WebURLLoader in r69429 it turns out we don't need reference counting anymore.

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

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

7 files changed, 22 insertions, 40 deletions

diff --git a/webkit/media/active_loader.cc b/webkit/media/active_loader.cc
index af5bde7..e10098f 100644
--- a/webkit/media/active_loader.cc
+++ b/webkit/media/active_loader.cc
@@ -9,17 +9,13 @@
 
 namespace webkit_media {
 
-ActiveLoader::ActiveLoader(
-    const scoped_refptr<BufferedResourceLoader>& parent,
-    WebKit::WebURLLoader* loader)
-    : parent_(parent),
-      loader_(loader),
+ActiveLoader::ActiveLoader(WebKit::WebURLLoader* loader)
+    : loader_(loader),
       deferred_(false) {
 }
 
 ActiveLoader::~ActiveLoader() {
-  if (parent_)
-    Cancel();
+  Cancel();
 }
 
 void ActiveLoader::SetDeferred(bool deferred) {
@@ -28,10 +24,7 @@ void ActiveLoader::SetDeferred(bool deferred) {
 }
 
 void ActiveLoader::Cancel() {
-  // We only need to maintain a reference to our parent while the loader is
-  // still active. Failing to do so can result in circular refcounts.
   loader_->cancel();
-  parent_ = NULL;
 }
 
 }  // namespace webkit_media
diff --git a/webkit/media/active_loader.h b/webkit/media/active_loader.h
index 18c220c..8a63ed0 100644
--- a/webkit/media/active_loader.h
+++ b/webkit/media/active_loader.h
@@ -6,7 +6,6 @@
 #define WEBKIT_MEDIA_ACTIVE_LOADER_H_
 
 #include "base/basictypes.h"
-#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 
 namespace WebKit {
@@ -15,20 +14,16 @@ class WebURLLoader;
 
 namespace webkit_media {
 
-class BufferedResourceLoader;
-
-// Wraps an active WebURLLoader with some additional state and maintains a
-// reference to its parent.
+// Wraps an active WebURLLoader with some additional state.
 //
 // Handles deferring and deletion of loaders.
 class ActiveLoader {
  public:
-  // Creates an ActiveLoader with a reference to its parent. The initial state
-  // assumes that the loader has started loading and has not been deferred.
+  // Creates an ActiveLoader with the given loader. It is assumed that the
+  // initial state of |loader| is loading and not deferred.
   //
   // ActiveLoader takes ownership of |loader|.
-  ActiveLoader(const scoped_refptr<BufferedResourceLoader>& parent,
-               WebKit::WebURLLoader* loader);
+  explicit ActiveLoader(WebKit::WebURLLoader* loader);
   ~ActiveLoader();
 
   // Starts or stops deferring the resource load.
@@ -41,7 +36,6 @@ class ActiveLoader {
  private:
   friend class BufferedDataSourceTest;
 
-  scoped_refptr<BufferedResourceLoader> parent_;
   scoped_ptr<WebKit::WebURLLoader> loader_;
   bool deferred_;
 
diff --git a/webkit/media/buffered_data_source.cc b/webkit/media/buffered_data_source.cc
index 8a5feba..794c40e 100644
--- a/webkit/media/buffered_data_source.cc
+++ b/webkit/media/buffered_data_source.cc
@@ -222,7 +222,7 @@ void BufferedDataSource::InitializeTask() {
   if (url_.SchemeIs(kHttpScheme) || url_.SchemeIs(kHttpsScheme)) {
     // Do an unbounded range request starting at the beginning.  If the server
     // responds with 200 instead of 206 we'll fall back into a streaming mode.
-    loader_ = CreateResourceLoader(0, kPositionNotSpecified);
+    loader_.reset(CreateResourceLoader(0, kPositionNotSpecified));
     loader_->Start(
         base::Bind(&BufferedDataSource::HttpInitialStartCallback, this),
         base::Bind(&BufferedDataSource::NetworkEventCallback, this),
@@ -231,8 +231,8 @@ void BufferedDataSource::InitializeTask() {
     // For all other protocols, assume they support range request. We fetch
     // the full range of the resource to obtain the instance size because
     // we won't be served HTTP headers.
-    loader_ = CreateResourceLoader(kPositionNotSpecified,
-                                   kPositionNotSpecified);
+    loader_.reset(CreateResourceLoader(kPositionNotSpecified,
+                                       kPositionNotSpecified));
     loader_->Start(
         base::Bind(&BufferedDataSource::NonHttpInitialStartCallback, this),
         base::Bind(&BufferedDataSource::NetworkEventCallback, this),
@@ -303,7 +303,7 @@ void BufferedDataSource::RestartLoadingTask() {
       return;
   }
 
-  loader_ = CreateResourceLoader(read_position_, kPositionNotSpecified);
+  loader_.reset(CreateResourceLoader(read_position_, kPositionNotSpecified));
   loader_->Start(
       base::Bind(&BufferedDataSource::PartialReadStartCallback, this),
       base::Bind(&BufferedDataSource::NetworkEventCallback, this),
@@ -363,7 +363,7 @@ BufferedDataSource::ChooseDeferStrategy() {
 // prior to make this method call.
 void BufferedDataSource::ReadInternal() {
   DCHECK(MessageLoop::current() == render_loop_);
-  DCHECK(loader_);
+  DCHECK(loader_.get());
 
   // First we prepare the intermediate read buffer for BufferedResourceLoader
   // to write to.
@@ -441,8 +441,8 @@ void BufferedDataSource::HttpInitialStartCallback(int error) {
     // Assuming that the Range header was causing the problem. Retry without
     // the Range header.
     using_range_request_ = false;
-    loader_ = CreateResourceLoader(kPositionNotSpecified,
-                                   kPositionNotSpecified);
+    loader_.reset(CreateResourceLoader(kPositionNotSpecified,
+                                       kPositionNotSpecified));
     loader_->Start(
         base::Bind(&BufferedDataSource::HttpInitialStartCallback, this),
         base::Bind(&BufferedDataSource::NetworkEventCallback, this),
diff --git a/webkit/media/buffered_data_source.h b/webkit/media/buffered_data_source.h
index 1332489..11210b9 100644
--- a/webkit/media/buffered_data_source.h
+++ b/webkit/media/buffered_data_source.h
@@ -155,7 +155,7 @@ class BufferedDataSource : public WebDataSource {
   WebKit::WebFrame* frame_;
 
   // A resource loader for the media resource.
-  scoped_refptr<BufferedResourceLoader> loader_;
+  scoped_ptr<BufferedResourceLoader> loader_;
 
   // True if |loader| is downloading data.
   bool is_downloading_data_;
diff --git a/webkit/media/buffered_resource_loader.cc b/webkit/media/buffered_resource_loader.cc
index ea273fe..b1a4003 100644
--- a/webkit/media/buffered_resource_loader.cc
+++ b/webkit/media/buffered_resource_loader.cc
@@ -182,7 +182,7 @@ void BufferedResourceLoader::Start(
 
   // Start the resource loading.
   loader->loadAsynchronously(request, this);
-  active_loader_.reset(new ActiveLoader(this, loader));
+  active_loader_.reset(new ActiveLoader(loader));
 }
 
 void BufferedResourceLoader::Stop() {
diff --git a/webkit/media/buffered_resource_loader.h b/webkit/media/buffered_resource_loader.h
index b498fd7..28815c3 100644
--- a/webkit/media/buffered_resource_loader.h
+++ b/webkit/media/buffered_resource_loader.h
@@ -37,9 +37,7 @@ const char kDataScheme[] = "data";
 // WebURLLoader and does the actual resource loading. This object does
 // buffering internally, it defers the resource loading if buffer is full
 // and un-defers the resource loading if it is under buffered.
-class BufferedResourceLoader
-    : public base::RefCountedThreadSafe<BufferedResourceLoader>,
-      public WebKit::WebURLLoaderClient {
+class BufferedResourceLoader : public WebKit::WebURLLoaderClient {
  public:
   // kNeverDefer - Aggresively buffer; never defer loading while paused.
   // kReadThenDefer - Request only enough data to fulfill read requests.
@@ -65,6 +63,7 @@ class BufferedResourceLoader
                          int bitrate,
                          float playback_rate,
                          media::MediaLog* media_log);
+  virtual ~BufferedResourceLoader();
 
   // Start the resource loading with the specified URL and range.
   // This method operates in asynchronous mode. Once there's a response from the
@@ -175,10 +174,6 @@ class BufferedResourceLoader
   // accordingly.
   void SetBitrate(int bitrate);
 
- protected:
-  friend class base::RefCountedThreadSafe<BufferedResourceLoader>;
-  virtual ~BufferedResourceLoader();
-
  private:
   friend class BufferedDataSourceTest;
   friend class BufferedResourceLoaderTest;
diff --git a/webkit/media/buffered_resource_loader_unittest.cc b/webkit/media/buffered_resource_loader_unittest.cc
index fafefb8..9416f0f 100644
--- a/webkit/media/buffered_resource_loader_unittest.cc
+++ b/webkit/media/buffered_resource_loader_unittest.cc
@@ -82,10 +82,10 @@ class BufferedResourceLoaderTest : public testing::Test {
     last_position_ = last_position;
 
     url_loader_ = new NiceMock<MockWebURLLoader>();
-    loader_ = new BufferedResourceLoader(
+    loader_.reset(new BufferedResourceLoader(
         gurl_, first_position_, last_position_,
         BufferedResourceLoader::kThresholdDefer, 0, 0,
-        new media::MediaLog());
+        new media::MediaLog()));
     loader_->SetURLLoaderForTest(url_loader_);
   }
 
@@ -191,7 +191,7 @@ class BufferedResourceLoaderTest : public testing::Test {
     InSequence s;
     EXPECT_CALL(*url_loader_, cancel());
     loader_->Stop();
-    loader_ = NULL;
+    loader_.reset();
   }
 
   // Helper method to write to |loader_| from |data_|.
@@ -291,7 +291,7 @@ class BufferedResourceLoaderTest : public testing::Test {
   int64 first_position_;
   int64 last_position_;
 
-  scoped_refptr<BufferedResourceLoader> loader_;
+  scoped_ptr<BufferedResourceLoader> loader_;
   NiceMock<MockWebURLLoader>* url_loader_;
 
   MockWebFrameClient client_;