Remove ActiveLoader::Cancel().

This will force clients to rely on the destructor to cancel loaders instead and prevents the situation where you hold onto an ActiveLoader that has been cancelled. BufferedResourceLoader did this inside of didFail(), which led to WillFulfillRead() returning true, which led to a hang since the loader had been cancelled.

BUG=106751
TEST=http/tests/appacache/video.html

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

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

4 files changed, 5 insertions, 12 deletions

diff --git a/webkit/media/active_loader.cc b/webkit/media/active_loader.cc
index 767ddff..cc1c5e2 100644
--- a/webkit/media/active_loader.cc
+++ b/webkit/media/active_loader.cc
@@ -15,7 +15,7 @@ ActiveLoader::ActiveLoader(scoped_ptr<WebKit::WebURLLoader> loader)
 }
 
 ActiveLoader::~ActiveLoader() {
-  Cancel();
+  loader_->cancel();
 }
 
 void ActiveLoader::SetDeferred(bool deferred) {
@@ -23,8 +23,4 @@ void ActiveLoader::SetDeferred(bool deferred) {
   loader_->setDefersLoading(deferred);
 }
 
-void ActiveLoader::Cancel() {
-  loader_->cancel();
-}
-
 }  // namespace webkit_media
diff --git a/webkit/media/active_loader.h b/webkit/media/active_loader.h
index b7300fc..1717307 100644
--- a/webkit/media/active_loader.h
+++ b/webkit/media/active_loader.h
@@ -28,9 +28,6 @@ class ActiveLoader {
   void SetDeferred(bool deferred);
   bool deferred() { return deferred_; }
 
-  // Cancels the URL loader associated with this object.
-  void Cancel();
-
  private:
   friend class BufferedDataSourceTest;
 
diff --git a/webkit/media/buffered_data_source_unittest.cc b/webkit/media/buffered_data_source_unittest.cc
index 98a34d6..f1d60e2 100644
--- a/webkit/media/buffered_data_source_unittest.cc
+++ b/webkit/media/buffered_data_source_unittest.cc
@@ -401,7 +401,6 @@ TEST_F(BufferedDataSourceTest, SetBitrate) {
   EXPECT_EQ(1234, loader_bitrate());
 
   // During teardown we'll also report our final network status.
-  EXPECT_CALL(host_, SetNetworkActivity(true));
   EXPECT_CALL(host_, SetBufferedBytes(4000000));
 
   EXPECT_TRUE(data_source_->loading());
@@ -427,7 +426,6 @@ TEST_F(BufferedDataSourceTest, SetPlaybackRate) {
   EXPECT_NE(old_loader, loader());
 
   // During teardown we'll also report our final network status.
-  EXPECT_CALL(host_, SetNetworkActivity(true));
   EXPECT_CALL(host_, SetBufferedBytes(4000000));
 
   EXPECT_TRUE(data_source_->loading());
@@ -447,8 +445,8 @@ TEST_F(BufferedDataSourceTest, Read) {
   FinishRead();
 
   // During teardown we'll also report our final network status.
+  EXPECT_CALL(host_, SetNetworkActivity(false));
   EXPECT_CALL(host_, SetBufferedBytes(kDataSize));
-  //EXPECT_CALL(host_, SetNetworkActivity(false));
 
   EXPECT_TRUE(data_source_->loading());
   Stop();
diff --git a/webkit/media/buffered_resource_loader.cc b/webkit/media/buffered_resource_loader.cc
index afac0db..4bb6d80 100644
--- a/webkit/media/buffered_resource_loader.cc
+++ b/webkit/media/buffered_resource_loader.cc
@@ -511,7 +511,9 @@ void BufferedResourceLoader::didFail(
   DCHECK(active_loader_.get());
 
   // We don't need to continue loading after failure.
-  active_loader_->Cancel();
+  //
+  // Keep it alive until we exit this method so that |error| remains valid.
+  scoped_ptr<ActiveLoader> active_loader(active_loader_.release());
   NotifyNetworkEvent();
 
   // Don't leave start callbacks hanging around.