[media] When testing, call RendererWrapper::Stop() synchronously.

Before this change, the testing path invoked RenderWrapper::Stop() by
posting a task, which would execute after PipelineImpl::Stop().

The production path also posts a task, but to a different thread, and
PipelineImpl::Stop() waits for that task to execute before returning.

This change makes the testing path more similar to the production path,
by ensuring that RendererWrapper::Stop() executes before
PipelineImpl::Stop() returns.

The PipelineTeardownTest is also updated so that expectations on the
Demuxer that Stop() the Pipeline are posted rather than directly
called. This better matches the production behavior where they would
be on separate threads and therefore cannot be re-entrant.

Bug: 788508
Change-Id: I894ee6c2febb3b739aaa809c6aa0d9388b76b64a
Reviewed-on: https://chromium-review.googlesource.com/791860
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519862}

media/base/pipeline_impl.cc

@@ -1042,11 +1042,7 @@ void PipelineImpl::Stop() {
 
   if (media_task_runner_->BelongsToCurrentThread()) {
     // This path is executed by unittests that share media and main threads.
-    base::Closure stop_cb = base::Bind(&base::DoNothing);
-    media_task_runner_->PostTask(
-        FROM_HERE,
-        base::Bind(&RendererWrapper::Stop,
-                   base::Unretained(renderer_wrapper_.get()), stop_cb));
+    renderer_wrapper_->Stop(base::Bind(&base::DoNothing));
   } else {
     // This path is executed by production code where the two task runners -
     // main and media - live on different threads.

media/base/pipeline_impl_unittest.cc

@@ -56,6 +56,11 @@ ACTION_P(Stop, pipeline) {
   pipeline->Stop();
 }
 
+ACTION_P(PostStop, pipeline) {
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::Bind(&Pipeline::Stop, base::Unretained(pipeline)));
+}
+
 ACTION_P2(SetError, renderer_client, status) {
   (*renderer_client)->OnError(status);
 }
@@ -929,7 +934,7 @@ class PipelineTeardownTest : public PipelineImplTest {
       if (stop_or_error == kStop) {
         EXPECT_CALL(*demuxer_, Initialize(_, _, _))
             .WillOnce(
-                DoAll(Stop(pipeline_.get()), PostCallback<1>(PIPELINE_OK)));
+                DoAll(PostStop(pipeline_.get()), PostCallback<1>(PIPELINE_OK)));
         // Note: OnStart callback is not called after pipeline is stopped.
       } else {
         EXPECT_CALL(*demuxer_, Initialize(_, _, _))
@@ -953,7 +958,7 @@ class PipelineTeardownTest : public PipelineImplTest {
       if (stop_or_error == kStop) {
         EXPECT_CALL(*renderer_, Initialize(_, _, _))
             .WillOnce(
-                DoAll(Stop(pipeline_.get()), PostCallback<2>(PIPELINE_OK)));
+                DoAll(PostStop(pipeline_.get()), PostCallback<2>(PIPELINE_OK)));
         // Note: OnStart is not callback after pipeline is stopped.
       } else {
         EXPECT_CALL(*renderer_, Initialize(_, _, _))
@@ -1026,7 +1031,7 @@ class PipelineTeardownTest : public PipelineImplTest {
       if (stop_or_error == kStop) {
         EXPECT_CALL(*demuxer_, Seek(_, _))
             .WillOnce(
-                DoAll(Stop(pipeline_.get()), RunCallback<1>(PIPELINE_OK)));
+                DoAll(PostStop(pipeline_.get()), RunCallback<1>(PIPELINE_OK)));
         // Note: OnSeek callback is not called after pipeline is stopped.
       } else {
         EXPECT_CALL(*demuxer_, Seek(_, _))
@@ -1067,7 +1072,7 @@ class PipelineTeardownTest : public PipelineImplTest {
       if (stop_or_error == kStop) {
         EXPECT_CALL(*demuxer_, Seek(_, _))
             .WillOnce(
-                DoAll(Stop(pipeline_.get()), RunCallback<1>(PIPELINE_OK)));
+                DoAll(PostStop(pipeline_.get()), RunCallback<1>(PIPELINE_OK)));
         // Note: OnResume callback is not called after pipeline is stopped.
       } else {
         EXPECT_CALL(*demuxer_, Seek(_, _))