[chromecast] Pass media task runner to MediaPipelineBackendManager.

[chromecast] Pass media task runner to MediaPipelineBackendManager.

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

[tianyuwang1, 2016-04-04 20:51:21]

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

[tianyuwang1, 2016-04-04 22:58:00]

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

[commit-bot: I haz the power, 2016-04-04 22:58:32]

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

[alokp, 2016-04-04 23:08:28]

https://codereview.chromium.org/1858803002/diff/1/chromecast/browser/cast_con...
File chromecast/browser/cast_content_browser_client.cc (right):

https://codereview.chromium.org/1858803002/diff/1/chromecast/browser/cast_con...
chromecast/browser/cast_content_browser_client.cc:112:
DCHECK(cast_browser_main_parts_);
move this DCHECK to CastContentBrowserClient::media_pipeline_backend_manager()

https://codereview.chromium.org/1858803002/diff/1/chromecast/media/cma/backen...
File chromecast/media/cma/backend/media_pipeline_backend_manager.h (right):

https://codereview.chromium.org/1858803002/diff/1/chromecast/media/cma/backen...
chromecast/media/cma/backend/media_pipeline_backend_manager.h:48:
scoped_refptr<base::SingleThreadTaskRunner> media_task_runner() const {
"media_" prefix is unnecessary here.

[tianyuwang1, 2016-04-05 01:33:31]

PTAL

https://codereview.chromium.org/1858803002/diff/1/chromecast/browser/cast_con...
File chromecast/browser/cast_content_browser_client.cc (right):

https://codereview.chromium.org/1858803002/diff/1/chromecast/browser/cast_con...
chromecast/browser/cast_content_browser_client.cc:112:
DCHECK(cast_browser_main_parts_);
On 2016/04/04 23:08:28, alokp wrote:
> move this DCHECK to CastContentBrowserClient::media_pipeline_backend_manager()

Done.

https://codereview.chromium.org/1858803002/diff/1/chromecast/media/cma/backen...
File chromecast/media/cma/backend/media_pipeline_backend_manager.h (right):

https://codereview.chromium.org/1858803002/diff/1/chromecast/media/cma/backen...
chromecast/media/cma/backend/media_pipeline_backend_manager.h:48:
scoped_refptr<base::SingleThreadTaskRunner> media_task_runner() const {
On 2016/04/04 23:08:28, alokp wrote:
> "media_" prefix is unnecessary here.

Done.

[alokp, 2016-04-05 20:35:46]

lgtm % comments

https://codereview.chromium.org/1858803002/diff/20001/chromecast/media/audio/...
File chromecast/media/audio/cast_audio_manager.h (right):

https://codereview.chromium.org/1858803002/diff/20001/chromecast/media/audio/...
chromecast/media/audio/cast_audio_manager.h:23: // This class does not take
ownership of |backend_manager|
redundant comment - ownership role is obvious from pointer type.

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

https://codereview.chromium.org/1858803002/diff/20001/chromecast/media/cma/ba...
chromecast/media/cma/backend/media_pipeline_backend_manager.h:40: void
OnMediaPipelineBackendDestroyed(const MediaPipelineBackend* backend);
This should not be a public member function. You should make
MediaPipelineBackendWrapper a friend and make this private. It can be done in a
separate patch.

https://codereview.chromium.org/1858803002/diff/20001/chromecast/media/cma/ba...
chromecast/media/cma/backend/media_pipeline_backend_manager.h:48:
scoped_refptr<base::SingleThreadTaskRunner> task_runner() const {
This should return a raw pointer.

[tianyuwang1, 2016-04-05 22:24:04]

https://codereview.chromium.org/1858803002/diff/20001/chromecast/media/audio/...
File chromecast/media/audio/cast_audio_manager.h (right):

https://codereview.chromium.org/1858803002/diff/20001/chromecast/media/audio/...
chromecast/media/audio/cast_audio_manager.h:23: // This class does not take
ownership of |backend_manager|
On 2016/04/05 20:35:46, alokp wrote:
> redundant comment - ownership role is obvious from pointer type.

Done.

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

https://codereview.chromium.org/1858803002/diff/20001/chromecast/media/cma/ba...
chromecast/media/cma/backend/media_pipeline_backend_manager.h:40: void
OnMediaPipelineBackendDestroyed(const MediaPipelineBackend* backend);
On 2016/04/05 20:35:46, alokp wrote:
> This should not be a public member function. You should make
> MediaPipelineBackendWrapper a friend and make this private. It can be done in
a
> separate patch.

Done in this patch.

https://codereview.chromium.org/1858803002/diff/20001/chromecast/media/cma/ba...
chromecast/media/cma/backend/media_pipeline_backend_manager.h:48:
scoped_refptr<base::SingleThreadTaskRunner> task_runner() const {
On 2016/04/05 20:35:46, alokp wrote:
> This should return a raw pointer.

Done.

[tianyuwang1, 2016-04-05 23:48:55]

The CQ bit was checked by tianyuwang@google.com

[tianyuwang1, 2016-04-05 23:48:56]

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/1858803002/#ps40001
(title: "Minor changes according to comments.")

[commit-bot: I haz the power, 2016-04-05 23:49:31]

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

[alokp, 2016-04-06 00:07:10]

this will again break the tot builders unless you have the internal patch ready
to go in.

[tianyuwang1, 2016-04-06 00:14:50]

On 2016/04/06 00:07:10, alokp wrote:
> this will again break the tot builders unless you have the internal patch
ready
> to go in.

I think the internal patch should be ready to go.  I've resolved most of the
comments.

[tianyuwang1, 2016-04-06 00:19:42]

The CQ bit was unchecked by tianyuwang@google.com

[tianyuwang1, 2016-04-06 20:55:03]

The CQ bit was checked by tianyuwang@google.com

[commit-bot: I haz the power, 2016-04-06 20:55:28]

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

[tianyuwang1, 2016-04-06 21:00:13]

The CQ bit was unchecked by tianyuwang@google.com

[tianyuwang1, 2016-04-06 21:16:33]

The CQ bit was checked by tianyuwang@google.com

[commit-bot: I haz the power, 2016-04-06 21:17:20]

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

[commit-bot: I haz the power, 2016-04-06 21:24:30]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-06 21:26:00]

Description was changed from

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

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

to

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

BUG=597450

Committed: https://crrev.com/c7e35d80cffa26c4ddc76ba6f71fd3127dccec31
Cr-Commit-Position: refs/heads/master@{#385556}
==========

[commit-bot: I haz the power, 2016-04-06 21:26:01]

Patchset 3 (id:??) landed as
https://crrev.com/c7e35d80cffa26c4ddc76ba6f71fd3127dccec31
Cr-Commit-Position: refs/heads/master@{#385556}