Change ownership of PipelineImpl

Change ownership of PipelineImpl

WMPI owns an instance of PipelineImpl and passes its address to
a PipelineController. WMPI calls method on both |pipeline_| and
|pipeline_controller_|.

This CL moves the ownership of PipelineImpl to the PipelineController,
and exposes the rest of the Pipeline interface through the
PipelineController. This will simplify reasoning about Pipeline
callbacks and states changes by forcing all calls from WMPI to go
through |pipeline_controller_|.

bug=

Review-Url: https://codereview.chromium.org/2689863002
Cr-Commit-Position: refs/heads/master@{#452764}
Committed: https://chromium.googlesource.com/chromium/src/+/350936ff217789a657ef44417c374c6a0423f06f

[tguilbert, 2017-02-11 01:52:28]

Description was changed from

==========
works


temp

BUG=
==========

to

==========
//TODO(tguilbert)

BUG=
==========

[tguilbert, 2017-02-11 01:53:04]

The CQ bit was checked by tguilbert@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-11 01:53:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-11 02:21:10]

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

[commit-bot: I haz the power, 2017-02-11 02:21:12]

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

[tguilbert, 2017-02-14 21:05:38]

The CQ bit was checked by tguilbert@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-14 21:06:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-14 22:42:17]

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

[commit-bot: I haz the power, 2017-02-14 22:42:18]

Dry run: This issue passed the CQ dry run.

[tguilbert, 2017-02-23 22:17:19]

Description was changed from

==========
//TODO(tguilbert)

BUG=
==========

to

==========
Change ownership of PipelineImpl

WMPI owns an instance of PipelineImpl and passes its address to
a PipelineController. WMPI calls method on both |pipeline_| and
|pipeline_controller_|.

This CL moves the ownership of PipelineImpl to the PipelineController,
and exposes the rest of the Pipeline interface through the
PipelineController. This will simplify reasoning about Pipeline
callbacks and states changes by forcing all calls from WMPI to go
through |pipeline_controller_|.

bug=
==========

[tguilbert, 2017-02-23 22:17:29]

The CQ bit was checked by tguilbert@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 22:18:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tguilbert, 2017-02-23 22:21:36]

The CQ bit was checked by tguilbert@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 22:22:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tguilbert, 2017-02-23 22:22:24]

tguilbert@chromium.org changed reviewers:
+ sandersd@chromium.org

[tguilbert, 2017-02-23 22:22:25]

PTAL, thank you!

[sandersd (OOO until July 31), 2017-02-23 23:11:47]

lgtm % nits.

https://codereview.chromium.org/2689863002/diff/60001/media/filters/pipeline_...
File media/filters/pipeline_controller.cc (right):

https://codereview.chromium.org/2689863002/diff/60001/media/filters/pipeline_...
media/filters/pipeline_controller.cc:250: pipeline_->Stop();
Should we perhaps be resetting |state_| here?

https://codereview.chromium.org/2689863002/diff/60001/media/filters/pipeline_...
File media/filters/pipeline_controller.h (right):

https://codereview.chromium.org/2689863002/diff/60001/media/filters/pipeline_...
media/filters/pipeline_controller.h:26: //   - Expose an operation that cycles
via suspend+resume.
Nit: Reword these as:
  - Expose an operation that replaces the renderer.
  - Expose an operation that replaces the demuxer.

I'm not sure if replacing the demuxer implies replacing the renderer as well.
Clarify that if you can.

[commit-bot: I haz the power, 2017-02-24 00:21:26]

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

[commit-bot: I haz the power, 2017-02-24 00:21:27]

Dry run: This issue passed the CQ dry run.

[tguilbert, 2017-02-24 02:13:59]

https://codereview.chromium.org/2689863002/diff/60001/media/filters/pipeline_...
File media/filters/pipeline_controller.cc (right):

https://codereview.chromium.org/2689863002/diff/60001/media/filters/pipeline_...
media/filters/pipeline_controller.cc:250: pipeline_->Stop();
On 2017/02/23 23:11:46, sandersd wrote:
> Should we perhaps be resetting |state_| here?

I added a comment, a TODO, and opened crbug.com/695734 to track the work
officially.

https://codereview.chromium.org/2689863002/diff/60001/media/filters/pipeline_...
File media/filters/pipeline_controller.h (right):

https://codereview.chromium.org/2689863002/diff/60001/media/filters/pipeline_...
media/filters/pipeline_controller.h:26: //   - Expose an operation that cycles
via suspend+resume.
On 2017/02/23 23:11:46, sandersd wrote:
> Nit: Reword these as:
>   - Expose an operation that replaces the renderer.
>   - Expose an operation that replaces the demuxer.
> 
> I'm not sure if replacing the demuxer implies replacing the renderer as well.
> Clarify that if you can.

Done.

[tguilbert, 2017-02-24 02:14:02]

The CQ bit was checked by tguilbert@chromium.org

[tguilbert, 2017-02-24 02:14:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
sandersd@chromium.org
Link to the patchset: https://codereview.chromium.org/2689863002/#ps80001
(title: "Addressed comment")

[commit-bot: I haz the power, 2017-02-24 02:14:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-24 02:18:04]

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

[commit-bot: I haz the power, 2017-02-24 02:18:05]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tguilbert, 2017-02-24 03:22:14]

The CQ bit was checked by tguilbert@chromium.org

[tguilbert, 2017-02-24 03:22:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
sandersd@chromium.org
Link to the patchset: https://codereview.chromium.org/2689863002/#ps100001
(title: "Rebase")

[commit-bot: I haz the power, 2017-02-24 03:22:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-24 03:34:46]

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

[commit-bot: I haz the power, 2017-02-24 03:34:47]

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

[tguilbert, 2017-02-24 04:09:58]

The CQ bit was checked by tguilbert@chromium.org

[tguilbert, 2017-02-24 04:09:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
sandersd@chromium.org
Link to the patchset: https://codereview.chromium.org/2689863002/#ps120001
(title: "Proper rebase")

[commit-bot: I haz the power, 2017-02-24 04:10:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-24 05:39:31]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1487909398210770,
"parent_rev": "5378c93cfa61880bb755fff32f231fcf06cc8dd2", "commit_rev":
"350936ff217789a657ef44417c374c6a0423f06f"}

[commit-bot: I haz the power, 2017-02-24 05:40:06]

Description was changed from

==========
Change ownership of PipelineImpl

WMPI owns an instance of PipelineImpl and passes its address to
a PipelineController. WMPI calls method on both |pipeline_| and
|pipeline_controller_|.

This CL moves the ownership of PipelineImpl to the PipelineController,
and exposes the rest of the Pipeline interface through the
PipelineController. This will simplify reasoning about Pipeline
callbacks and states changes by forcing all calls from WMPI to go
through |pipeline_controller_|.

bug=
==========

to

==========
Change ownership of PipelineImpl

WMPI owns an instance of PipelineImpl and passes its address to
a PipelineController. WMPI calls method on both |pipeline_| and
|pipeline_controller_|.

This CL moves the ownership of PipelineImpl to the PipelineController,
and exposes the rest of the Pipeline interface through the
PipelineController. This will simplify reasoning about Pipeline
callbacks and states changes by forcing all calls from WMPI to go
through |pipeline_controller_|.

bug=

Review-Url: https://codereview.chromium.org/2689863002
Cr-Commit-Position: refs/heads/master@{#452764}
Committed:
https://chromium.googlesource.com/chromium/src/+/350936ff217789a657ef44417c37...
==========

[commit-bot: I haz the power, 2017-02-24 05:40:07]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/350936ff217789a657ef44417c37...