Adding Abort() to DataSource to wake up hanging Read()

This change adds an Abort() method to BufferedDataSource, which will
wake up a blocking read if one exists. When WebMediaPlayerImpl being
destroyed, it now tells BufferedDataSource to abort before it tells
the pipeline to stop, so the pipeline will not hang while waiting
for a never-ending Read() to return.

BUG=54465
TEST=test_shell_tests

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

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

5 files changed, 112 insertions, 11 deletions

diff --git a/webkit/glue/media/buffered_data_source.cc b/webkit/glue/media/buffered_data_source.cc
index 3456e3a..a4e3901 100644
--- a/webkit/glue/media/buffered_data_source.cc
+++ b/webkit/glue/media/buffered_data_source.cc
@@ -15,6 +15,7 @@
 #include "net/http/http_response_headers.h"
 #include "webkit/glue/media/buffered_data_source.h"
 #include "webkit/glue/webkit_glue.h"
+#include "webkit/glue/webmediaplayer_impl.h"
 
 namespace {
 
@@ -542,7 +543,8 @@ bool BufferedDataSource::IsMediaFormatSupported(
 // BufferedDataSource, protected
 BufferedDataSource::BufferedDataSource(
     MessageLoop* render_loop,
-    webkit_glue::MediaResourceLoaderBridgeFactory* bridge_factory)
+    webkit_glue::MediaResourceLoaderBridgeFactory* bridge_factory,
+    webkit_glue::WebMediaPlayerImpl::Proxy* proxy)
     : total_bytes_(kPositionNotSpecified),
       loaded_(false),
       streaming_(false),
@@ -562,6 +564,8 @@ BufferedDataSource::BufferedDataSource(
       stopped_on_render_loop_(false),
       media_is_paused_(true),
       using_range_request_(true) {
+  if (proxy)
+    proxy->SetDataSource(this);
 }
 
 BufferedDataSource::~BufferedDataSource() {
@@ -623,6 +627,7 @@ void BufferedDataSource::Stop(media::FilterCallback* callback) {
     callback->Run();
     delete callback;
   }
+
   render_loop_->PostTask(FROM_HERE,
       NewRunnableMethod(this, &BufferedDataSource::CleanupTask));
 }
@@ -655,6 +660,20 @@ bool BufferedDataSource::IsStreaming() {
   return streaming_;
 }
 
+void BufferedDataSource::Abort() {
+  DCHECK(MessageLoop::current() == render_loop_);
+
+  // If we are told to abort, immediately return from any pending read
+  // with an error.
+  if (read_callback_.get()) {
+    {
+      AutoLock auto_lock(lock_);
+      DoneRead_Locked(net::ERR_FAILED);
+    }
+    CleanupTask();
+  }
+}
+
 /////////////////////////////////////////////////////////////////////////////
 // BufferedDataSource, render thread tasks
 void BufferedDataSource::InitializeTask() {
@@ -720,7 +739,10 @@ void BufferedDataSource::ReadTask(
 
 void BufferedDataSource::CleanupTask() {
   DCHECK(MessageLoop::current() == render_loop_);
-  DCHECK(!stopped_on_render_loop_);
+
+  // If we have already stopped, do nothing.
+  if (stopped_on_render_loop_)
+    return;
 
   // Stop the watch dog.
   watch_dog_timer_.Stop();
diff --git a/webkit/glue/media/buffered_data_source.h b/webkit/glue/media/buffered_data_source.h
index e7b2e47..6b740c9 100644
--- a/webkit/glue/media/buffered_data_source.h
+++ b/webkit/glue/media/buffered_data_source.h
@@ -21,6 +21,8 @@
 #include "net/base/completion_callback.h"
 #include "net/base/file_stream.h"
 #include "webkit/glue/media/media_resource_loader_bridge_factory.h"
+#include "webkit/glue/media/web_data_source.h"
+#include "webkit/glue/webmediaplayer_impl.h"
 
 namespace webkit_glue {
 /////////////////////////////////////////////////////////////////////////////
@@ -212,18 +214,20 @@ class BufferedResourceLoader :
   DISALLOW_COPY_AND_ASSIGN(BufferedResourceLoader);
 };
 
-class BufferedDataSource : public media::DataSource {
+class BufferedDataSource : public WebDataSource {
  public:
   // Methods called from pipeline thread
   // Static methods for creating this class.
   static media::FilterFactory* CreateFactory(
       MessageLoop* message_loop,
-      webkit_glue::MediaResourceLoaderBridgeFactory* bridge_factory) {
-    return new media::FilterFactoryImpl2<
+      webkit_glue::MediaResourceLoaderBridgeFactory* bridge_factory,
+      webkit_glue::WebMediaPlayerImpl::Proxy* proxy) {
+    return new media::FilterFactoryImpl3<
         BufferedDataSource,
         MessageLoop*,
-        webkit_glue::MediaResourceLoaderBridgeFactory*>(
-        message_loop, bridge_factory);
+        webkit_glue::MediaResourceLoaderBridgeFactory*,
+        webkit_glue::WebMediaPlayerImpl::Proxy*>(
+        message_loop, bridge_factory, proxy);
   }
 
   // media::FilterFactoryImpl2 implementation.
@@ -248,10 +252,14 @@ class BufferedDataSource : public media::DataSource {
     return media_format_;
   }
 
+  // webkit_glue::WebDataSource implementation.
+  virtual void Abort();
+
  protected:
   BufferedDataSource(
       MessageLoop* render_loop,
-      webkit_glue::MediaResourceLoaderBridgeFactory* bridge_factory);
+      webkit_glue::MediaResourceLoaderBridgeFactory* bridge_factory,
+      webkit_glue::WebMediaPlayerImpl::Proxy* proxy);
   virtual ~BufferedDataSource();
 
   // A factory method to create a BufferedResourceLoader based on the read
@@ -266,10 +274,11 @@ class BufferedDataSource : public media::DataSource {
   virtual base::TimeDelta GetTimeoutMilliseconds();
 
  private:
-  friend class media::FilterFactoryImpl2<
+  friend class media::FilterFactoryImpl3<
       BufferedDataSource,
       MessageLoop*,
-      webkit_glue::MediaResourceLoaderBridgeFactory*>;
+      webkit_glue::MediaResourceLoaderBridgeFactory*,
+      webkit_glue::WebMediaPlayerImpl::Proxy*>;
 
   // Posted to perform initialization on render thread and start resource
   // loading.
diff --git a/webkit/glue/media/buffered_data_source_unittest.cc b/webkit/glue/media/buffered_data_source_unittest.cc
index f032595..b78f759 100644
--- a/webkit/glue/media/buffered_data_source_unittest.cc
+++ b/webkit/glue/media/buffered_data_source_unittest.cc
@@ -561,7 +561,7 @@ class MockBufferedDataSource : public BufferedDataSource {
  protected:
   MockBufferedDataSource(
       MessageLoop* message_loop, MediaResourceLoaderBridgeFactory* factory)
-      : BufferedDataSource(message_loop, factory) {
+      : BufferedDataSource(message_loop, factory, NULL) {
   }
 
  private:
@@ -757,6 +757,23 @@ class BufferedDataSourceTest : public testing::Test {
               memcmp(buffer_, data_ + static_cast<int>(position), read_size));
   }
 
+  void ReadDataSourceHang(int64 position, int size) {
+    EXPECT_TRUE(loader_);
+
+    // Expect a call to read, but the call never returns.
+    EXPECT_CALL(*loader_, Read(position, size, NotNull(), NotNull()));
+    data_source_->Read(
+        position, size, buffer_,
+        NewCallback(this, &BufferedDataSourceTest::ReadCallback));
+    message_loop_->RunAllPending();
+
+    // Now expect the read to return after aborting the data source.
+    EXPECT_CALL(*this, ReadCallback(_));
+    EXPECT_CALL(*loader_, Stop());
+    data_source_->Abort();
+    message_loop_->RunAllPending();
+  }
+
   void ReadDataSourceMiss(int64 position, int size) {
     EXPECT_TRUE(loader_);
 
@@ -946,6 +963,11 @@ TEST_F(BufferedDataSourceTest, ReadCacheMiss) {
   StopDataSource();
 }
 
+TEST_F(BufferedDataSourceTest, ReadHang) {
+  InitializeDataSource(kHttpUrl, net::OK, true, 25, LOADING);
+  ReadDataSourceHang(10, 10);
+}
+
 TEST_F(BufferedDataSourceTest, ReadFailed) {
   InitializeDataSource(kHttpUrl, net::OK, true, 1024, LOADING);
   ReadDataSourceHit(10, 10, 10);
diff --git a/webkit/glue/media/web_data_source.cc b/webkit/glue/media/web_data_source.cc
new file mode 100644
index 0000000..a46d594
--- /dev/null
+++ b/webkit/glue/media/web_data_source.cc
@@ -0,0 +1,17 @@
+// Copyright (c) 2010 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/base/filters.h"
+#include "webkit/glue/media/web_data_source.h"
+
+namespace webkit_glue {
+
+WebDataSource::WebDataSource()
+    : media::DataSource() {
+}
+
+WebDataSource::~WebDataSource() {
+}
+
+}  // namespace webkit_glue
diff --git a/webkit/glue/media/web_data_source.h b/webkit/glue/media/web_data_source.h
new file mode 100644
index 0000000..2bbfd1c
--- /dev/null
+++ b/webkit/glue/media/web_data_source.h
@@ -0,0 +1,31 @@
+// Copyright (c) 2010 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.
+
+#ifndef WEBKIT_GLUE_MEDIA_WEB_DATA_SOURCE_H_
+#define WEBKIT_GLUE_MEDIA_WEB_DATA_SOURCE_H_
+
+#include "media/base/filters.h"
+
+namespace webkit_glue {
+
+// An interface that allows WebMediaPlayerImpl::Proxy to communicate with the
+// DataSource in the pipeline.
+class WebDataSource : public media::DataSource {
+ public:
+  WebDataSource();
+  virtual ~WebDataSource();
+
+  // This method is used to unblock any read calls that would cause the
+  // media pipeline to stall.
+  //
+  // Method called on the render thread.
+  virtual void Abort() = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(WebDataSource);
+};
+
+}  // namespace webkit_glue
+
+#endif  // WEBKIT_GLUE_MEDIA_WEB_DATA_SOURCE_H_