[chromecast] Pass media task runner to MediaPipelineBackendManager.

[chromecast] Pass media task runner to MediaPipelineBackendManager.

This required making MediaPipelineBackendManager non-singleton and
passed around wherever required.

BUG=597450
Committed: https://crrev.com/7f6a0145ae3deb8920771b2a2347af5f5c91aedd
Cr-Commit-Position: refs/heads/master@{#384643}

[tianyuwang1, 2016-03-31 00:57:26]

tianyuwang@google.com changed reviewers:
+ alokp@chromium.org, halliwell@chromium.org

[tianyuwang1, 2016-03-31 01:09:28]

tianyuwang@google.com changed reviewers:
+ kmackay@chromium.org

[tianyuwang1, 2016-03-31 01:09:46]

The CQ bit was checked by tianyuwang@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-03-31 01:10:15]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1845783002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1845783002/20001

[commit-bot: I haz the power, 2016-03-31 01:29:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-31 01:29:48]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[tianyuwang1, 2016-03-31 01:41:59]

The CQ bit was checked by tianyuwang@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-03-31 01:42:15]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1845783002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1845783002/40001

[commit-bot: I haz the power, 2016-03-31 02:04:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-31 02:04:29]

Dry run: Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tianyuwang1, 2016-03-31 02:26:33]

The CQ bit was checked by tianyuwang@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-03-31 02:26:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1845783002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1845783002/60001

[commit-bot: I haz the power, 2016-03-31 02:50:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-31 02:50:23]

Dry run: Try jobs failed on following builders:
  cast_shell_android on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[alokp, 2016-03-31 04:37:24]

looks mostly good - just a few nits

https://codereview.chromium.org/1845783002/diff/60001/chromecast/browser/cast...
File chromecast/browser/cast_browser_main_parts.cc (right):

https://codereview.chromium.org/1845783002/diff/60001/chromecast/browser/cast...
chromecast/browser/cast_browser_main_parts.cc:498:
media_pipeline_backend_manager_.reset(nullptr);
1. No need for nullptr.
2. This needs to happen before we finalize the media shlib.
3. We should probably move media shlib initialize/finalize from
MediaResourceTracker to MediaPipelineBackendManager. This can be done in a
separate patch.

https://codereview.chromium.org/1845783002/diff/60001/chromecast/browser/cast...
File chromecast/browser/cast_browser_main_parts.h (right):

https://codereview.chromium.org/1845783002/diff/60001/chromecast/browser/cast...
chromecast/browser/cast_browser_main_parts.h:47:
media::MediaPipelineBackendManager* media_pipeline_backend_manager();
We do not use MediaPipelineBackend on Android. This needs to be moved into
!defined(OS_ANDROID) block.

https://codereview.chromium.org/1845783002/diff/60001/chromecast/media/audio/...
File chromecast/media/audio/cast_audio_manager.cc (right):

https://codereview.chromium.org/1845783002/diff/60001/chromecast/media/audio/...
chromecast/media/audio/cast_audio_manager.cc:69:
DCHECK(media::MediaMessageLoop::GetTaskRunner()->BelongsToCurrentThread());
Remove this check from here.

https://codereview.chromium.org/1845783002/diff/60001/chromecast/media/cma/ba...
File chromecast/media/cma/backend/media_pipeline_backend_manager.cc (right):

https://codereview.chromium.org/1845783002/diff/60001/chromecast/media/cma/ba...
chromecast/media/cma/backend/media_pipeline_backend_manager.cc:20: :
media_task_runner_(media_task_runner) {
std::move

https://codereview.chromium.org/1845783002/diff/60001/chromecast/media/cma/ba...
File chromecast/media/cma/backend/media_pipeline_backend_manager.h (right):

https://codereview.chromium.org/1845783002/diff/60001/chromecast/media/cma/ba...
chromecast/media/cma/backend/media_pipeline_backend_manager.h:34:
MediaPipelineBackend* CreateMediaPipelineBackend(
scoped_ptr?

https://codereview.chromium.org/1845783002/diff/60001/chromecast/media/cma/ba...
chromecast/media/cma/backend/media_pipeline_backend_manager.h:38:
MediaPipelineBackend* CreateMediaPipelineBackend(
ditto

[tianyuwang1, 2016-03-31 18:14:35]

The CQ bit was checked by tianyuwang@google.com to run a CQ dry run

[tianyuwang1, 2016-03-31 18:14:54]

Thanks for the review.  PTAL.

https://codereview.chromium.org/1845783002/diff/60001/chromecast/browser/cast...
File chromecast/browser/cast_browser_main_parts.cc (right):

https://codereview.chromium.org/1845783002/diff/60001/chromecast/browser/cast...
chromecast/browser/cast_browser_main_parts.cc:498:
media_pipeline_backend_manager_.reset(nullptr);
On 2016/03/31 04:37:24, alokp wrote:
> 1. No need for nullptr.
> 2. This needs to happen before we finalize the media shlib.
> 3. We should probably move media shlib initialize/finalize from
> MediaResourceTracker to MediaPipelineBackendManager. This can be done in a
> separate patch.

Done.

https://codereview.chromium.org/1845783002/diff/60001/chromecast/browser/cast...
File chromecast/browser/cast_browser_main_parts.h (right):

https://codereview.chromium.org/1845783002/diff/60001/chromecast/browser/cast...
chromecast/browser/cast_browser_main_parts.h:47:
media::MediaPipelineBackendManager* media_pipeline_backend_manager();
On 2016/03/31 04:37:24, alokp wrote:
> We do not use MediaPipelineBackend on Android. This needs to be moved into
> !defined(OS_ANDROID) block.

Done.

https://codereview.chromium.org/1845783002/diff/60001/chromecast/media/audio/...
File chromecast/media/audio/cast_audio_manager.cc (right):

https://codereview.chromium.org/1845783002/diff/60001/chromecast/media/audio/...
chromecast/media/audio/cast_audio_manager.cc:69:
DCHECK(media::MediaMessageLoop::GetTaskRunner()->BelongsToCurrentThread());
On 2016/03/31 04:37:24, alokp wrote:
> Remove this check from here.

Done.

https://codereview.chromium.org/1845783002/diff/60001/chromecast/media/cma/ba...
File chromecast/media/cma/backend/media_pipeline_backend_manager.cc (right):

https://codereview.chromium.org/1845783002/diff/60001/chromecast/media/cma/ba...
chromecast/media/cma/backend/media_pipeline_backend_manager.cc:20: :
media_task_runner_(media_task_runner) {
On 2016/03/31 04:37:24, alokp wrote:
> std::move

Done.

https://codereview.chromium.org/1845783002/diff/60001/chromecast/media/cma/ba...
File chromecast/media/cma/backend/media_pipeline_backend_manager.h (right):

https://codereview.chromium.org/1845783002/diff/60001/chromecast/media/cma/ba...
chromecast/media/cma/backend/media_pipeline_backend_manager.h:34:
MediaPipelineBackend* CreateMediaPipelineBackend(
On 2016/03/31 04:37:24, alokp wrote:
> scoped_ptr?

Done.

https://codereview.chromium.org/1845783002/diff/60001/chromecast/media/cma/ba...
chromecast/media/cma/backend/media_pipeline_backend_manager.h:38:
MediaPipelineBackend* CreateMediaPipelineBackend(
On 2016/03/31 04:37:24, alokp wrote:
> ditto

Done.

[commit-bot: I haz the power, 2016-03-31 18:15:30]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1845783002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1845783002/80001

[commit-bot: I haz the power, 2016-03-31 19:12:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-31 19:12:43]

Dry run: This issue passed the CQ dry run.

[alokp, 2016-04-01 03:05:17]

https://codereview.chromium.org/1845783002/diff/80001/chromecast/browser/cast...
File chromecast/browser/cast_content_browser_client.cc (right):

https://codereview.chromium.org/1845783002/diff/80001/chromecast/browser/cast...
chromecast/browser/cast_content_browser_client.cc:74:
browser_client->GetCmaMediaPipelineClient()));
This change is not needed and will break mojo build.

https://codereview.chromium.org/1845783002/diff/80001/chromecast/browser/cast...
File chromecast/browser/cast_content_browser_client.h (right):

https://codereview.chromium.org/1845783002/diff/80001/chromecast/browser/cast...
chromecast/browser/cast_content_browser_client.h:78:
media::MediaPipelineBackendManager* media_pipeline_backend_manager();
where is this being used?

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/audio/...
File chromecast/media/audio/cast_audio_manager.cc (right):

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/audio/...
chromecast/media/audio/cast_audio_manager.cc:11: #include
"chromecast/media/base/media_message_loop.h"
remove the include for media_message_loop.h

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/audio/...
chromecast/media/audio/cast_audio_manager.cc:13: #include
"chromecast/public/cast_media_shlib.h"
remove

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/audio/...
chromecast/media/audio/cast_audio_manager.cc:14: #include
"chromecast/public/media/media_pipeline_backend.h"
remove

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/cma/ba...
File chromecast/media/cma/backend/media_pipeline_backend_manager.cc (right):

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/cma/ba...
chromecast/media/cma/backend/media_pipeline_backend_manager.cc:9: #include
"base/bind.h"
remove

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/cma/ba...
chromecast/media/cma/backend/media_pipeline_backend_manager.cc:10: #include
"base/location.h"
remove

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/cma/ba...
File chromecast/media/cma/backend/media_pipeline_backend_wrapper.h (right):

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/cma/ba...
chromecast/media/cma/backend/media_pipeline_backend_wrapper.h:29: // This class
takes the ownership of |backend|.  It doesn't take ownership
This comment is redundant. It is obvious from the type of pointers being passed.

[tianyuwang1, 2016-04-01 17:23:03]

The CQ bit was checked by tianyuwang@google.com to run a CQ dry run

[tianyuwang1, 2016-04-01 17:23:20]

PTAL

https://codereview.chromium.org/1845783002/diff/80001/chromecast/browser/cast...
File chromecast/browser/cast_content_browser_client.cc (right):

https://codereview.chromium.org/1845783002/diff/80001/chromecast/browser/cast...
chromecast/browser/cast_content_browser_client.cc:74:
browser_client->GetCmaMediaPipelineClient()));
On 2016/04/01 03:05:16, alokp wrote:
> This change is not needed and will break mojo build.

Reverted this change.

https://codereview.chromium.org/1845783002/diff/80001/chromecast/browser/cast...
File chromecast/browser/cast_content_browser_client.h (right):

https://codereview.chromium.org/1845783002/diff/80001/chromecast/browser/cast...
chromecast/browser/cast_content_browser_client.h:78:
media::MediaPipelineBackendManager* media_pipeline_backend_manager();
On 2016/04/01 03:05:16, alokp wrote:
> where is this being used?

This is being used in cast_content_browser_client_internal.cc

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/audio/...
File chromecast/media/audio/cast_audio_manager.cc (right):

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/audio/...
chromecast/media/audio/cast_audio_manager.cc:11: #include
"chromecast/media/base/media_message_loop.h"
On 2016/04/01 03:05:16, alokp wrote:
> remove the include for media_message_loop.h

Done.

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/audio/...
chromecast/media/audio/cast_audio_manager.cc:13: #include
"chromecast/public/cast_media_shlib.h"
On 2016/04/01 03:05:16, alokp wrote:
> remove

Done.

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/audio/...
chromecast/media/audio/cast_audio_manager.cc:14: #include
"chromecast/public/media/media_pipeline_backend.h"
On 2016/04/01 03:05:16, alokp wrote:
> remove

Done.

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/cma/ba...
File chromecast/media/cma/backend/media_pipeline_backend_manager.cc (right):

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/cma/ba...
chromecast/media/cma/backend/media_pipeline_backend_manager.cc:9: #include
"base/bind.h"
On 2016/04/01 03:05:16, alokp wrote:
> remove

Done.

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/cma/ba...
chromecast/media/cma/backend/media_pipeline_backend_manager.cc:10: #include
"base/location.h"
On 2016/04/01 03:05:16, alokp wrote:
> remove

Done.

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/cma/ba...
File chromecast/media/cma/backend/media_pipeline_backend_wrapper.h (right):

https://codereview.chromium.org/1845783002/diff/80001/chromecast/media/cma/ba...
chromecast/media/cma/backend/media_pipeline_backend_wrapper.h:29: // This class
takes the ownership of |backend|.  It doesn't take ownership
On 2016/04/01 03:05:16, alokp wrote:
> This comment is redundant. It is obvious from the type of pointers being
passed.

Done.

[commit-bot: I haz the power, 2016-04-01 17:23:35]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1845783002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1845783002/100001

[tianyuwang1, 2016-04-01 17:25:14]

The CQ bit was checked by tianyuwang@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-04-01 17:25:57]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1845783002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1845783002/120001

[alokp, 2016-04-01 17:53:37]

lgtm % nits

https://codereview.chromium.org/1845783002/diff/120001/chromecast/browser/cas...
File chromecast/browser/cast_content_browser_client.cc (right):

https://codereview.chromium.org/1845783002/diff/120001/chromecast/browser/cas...
chromecast/browser/cast_content_browser_client.cc:113: return
cast_browser_main_parts_->media_pipeline_backend_manager()
Use CastContentBrowserClient::media_pipeline_backend_manager()

https://codereview.chromium.org/1845783002/diff/120001/chromecast/media/audio...
File chromecast/media/audio/cast_audio_manager_factory.h (right):

https://codereview.chromium.org/1845783002/diff/120001/chromecast/media/audio...
chromecast/media/audio/cast_audio_manager_factory.h:18: // This class does not
take ownership of |backend_manager|.
redundant comment

[tianyuwang1, 2016-04-01 17:58:21]

https://codereview.chromium.org/1845783002/diff/120001/chromecast/browser/cas...
File chromecast/browser/cast_content_browser_client.cc (right):

https://codereview.chromium.org/1845783002/diff/120001/chromecast/browser/cas...
chromecast/browser/cast_content_browser_client.cc:113: return
cast_browser_main_parts_->media_pipeline_backend_manager()
On 2016/04/01 17:53:37, alokp wrote:
> Use CastContentBrowserClient::media_pipeline_backend_manager()

Done.

https://codereview.chromium.org/1845783002/diff/120001/chromecast/media/audio...
File chromecast/media/audio/cast_audio_manager_factory.h (right):

https://codereview.chromium.org/1845783002/diff/120001/chromecast/media/audio...
chromecast/media/audio/cast_audio_manager_factory.h:18: // This class does not
take ownership of |backend_manager|.
On 2016/04/01 17:53:37, alokp wrote:
> redundant comment

Done.

[tianyuwang1, 2016-04-01 17:58:27]

The CQ bit was checked by tianyuwang@google.com

[tianyuwang1, 2016-04-01 17:58:28]

The patchset sent to the CQ was uploaded after l-g-t-m from alokp@chromium.org
Link to the patchset: https://codereview.chromium.org/1845783002/#ps140001
(title: "minor fixes")

[commit-bot: I haz the power, 2016-04-01 17:58:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1845783002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1845783002/140001

[commit-bot: I haz the power, 2016-04-01 18:48:14]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-04-01 18:50:38]

Description was changed from

==========
[chromecast] Pass media task runner to MediaPipelineBackendManager.

This required making MediaPipelineBackendManager non-singleton and
passed around wherever required.

BUG=597450
==========

to

==========
[chromecast] Pass media task runner to MediaPipelineBackendManager.

This required making MediaPipelineBackendManager non-singleton and
passed around wherever required.

BUG=597450

Committed: https://crrev.com/7f6a0145ae3deb8920771b2a2347af5f5c91aedd
Cr-Commit-Position: refs/heads/master@{#384643}
==========

[commit-bot: I haz the power, 2016-04-01 18:50:39]

Patchset 8 (id:??) landed as
https://crrev.com/7f6a0145ae3deb8920771b2a2347af5f5c91aedd
Cr-Commit-Position: refs/heads/master@{#384643}

[tianyuwang1, 2016-04-02 00:29:30]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/1855633002/ by tianyuwang@google.com.

The reason for reverting is: Broke chromecast.  Will reland it after that's
fixed..