Fix a teardown hang caused by an Abort() call while there is a pending read.

webkit/glue/media/buffered_data_source_unittest.cc

Index: webkit/glue/media/buffered_data_source_unittest.cc
diff --git a/webkit/glue/media/buffered_data_source_unittest.cc b/webkit/glue/media/buffered_data_source_unittest.cc
index 6e9a87d15dded92b600c33d6e3399db78ebbe665..f3708038a05314ae5d2d54e20bf88e861d430b92 100644
--- a/webkit/glue/media/buffered_data_source_unittest.cc
+++ b/webkit/glue/media/buffered_data_source_unittest.cc
@@ -492,4 +492,70 @@ TEST_F(BufferedDataSourceTest, FileHasLoadedState) {
   StopDataSource();
 }
 
+// This test makes sure that Stop() does not require a task to run on
+// |message_loop_| before it calls its callback. This prevents accidental
+// introduction of a pipeline teardown deadlock. The pipeline owner blocks
+// the render message loop while waiting for Stop() to complete. Since this
+// object runs on the render message loop, Stop() will not complete if it
+// requires a task to run on the the message loop that is being blocked.
+TEST_F(BufferedDataSourceTest, StopDoesNotUseMessageLoopForCallback) {
+  InitializeDataSource(kFileUrl, net::OK, true, 1024, LOADED);
+
+  // Create a callback that lets us verify that it was called before
+  // Stop() returns. This is to make sure that the callback does not
+  // require |message_loop_| to execute tasks before being called.
+  media::MockCallback* stop_callback = media::NewExpectedCallback();
+  bool stop_done_called = false;
+  ON_CALL(*stop_callback, RunWithParams(_))

[scherkus (not reviewing), 2011/01/26 03:04:10]

nice usage :)

to confirm this keeps the expectation but simply assigns an action?

[acolwell GONE FROM CHROMIUM, 2011/01/26 17:41:05]

Thanks.
Yes. The expectation stays in place. It just changes what happens when the
method is called.

+      .WillByDefault(Assign(&stop_done_called, true));
+
+  // Stop() the data source like normal.
+  data_source_->Stop(stop_callback);
+
+  // Verify that the callback was called inside the Stop() call.
+  EXPECT_TRUE(stop_done_called);
+
+  message_loop_->RunAllPending();
+}
+
+TEST_F(BufferedDataSourceTest, AbortDuringPendingRead) {
+  InitializeDataSource(kFileUrl, net::OK, true, 1024, LOADED);
+
+  // Setup a way to verify that Read() is not called on the loader.
+  // We are doing this to make sure that the ReadTask() is still on
+  // the message loop queue when Abort() is called.
+  bool read_called = false;
+  ON_CALL(*loader_, Read(_, _, _ , _))
+      .WillByDefault(DoAll(Assign(&read_called, true),
+                           DeleteArg<3>()));
+
+  // Initiate a Read() on the data source, but don't allow the
+  // message loop to run.
+  data_source_->Read(
+      0, 10, buffer_,
+      NewCallback(static_cast<BufferedDataSourceTest*>(this),
+                  &BufferedDataSourceTest::ReadCallback));
+
+  // Call Abort() with the read pending.
+  EXPECT_CALL(*this, ReadCallback(-1));
+  EXPECT_CALL(*loader_, Stop());
+  data_source_->Abort();
+
+  // Verify that Read()'s after the Abort() issue callback with an error.
+  EXPECT_CALL(*this, ReadCallback(-1));
+  data_source_->Read(
+      0, 10, buffer_,
+      NewCallback(static_cast<BufferedDataSourceTest*>(this),
+                  &BufferedDataSourceTest::ReadCallback));
+
+  // Stop() the data source like normal.
+  data_source_->Stop(media::NewExpectedCallback());
+
+  // Allow cleanup task to run.
+  message_loop_->RunAllPending();
+
+  // Verify that Read() was not called on the loader.
+  EXPECT_FALSE(read_called);
+}
+
 }  // namespace webkit_glue