Allow WebRtcMediaEngine to be created from any thread.

Allow WebRtcMediaEngine to be created from any thread.

This eliminates a thread hop in PeerConnectionFactory initialization,
and will allow some code to be simplified.

BUG=None

Review-Url: https://codereview.webrtc.org/2934103002
Cr-Commit-Position: refs/heads/master@{#18613}
Committed: https://chromium.googlesource.com/external/webrtc/+/eb02c03a53734b6faf01d7f98a6174918169b850

[Taylor Brandstetter, 2017-06-13 19:14:50]

The CQ bit was checked by deadbeef@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-13 19:14:58]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-06-13 19:39:51]

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

[commit-bot: I haz the power, 2017-06-13 19:39:52]

Dry run: This issue passed the CQ dry run.

[Taylor Brandstetter, 2017-06-13 21:35:50]

deadbeef@webrtc.org changed reviewers:
+ solenberg@webrtc.org

[Taylor Brandstetter, 2017-06-13 21:35:51]

https://codereview.webrtc.org/2934103002/diff/1/webrtc/media/engine/webrtcvoi...
File webrtc/media/engine/webrtcvoiceengine.cc (right):

https://codereview.webrtc.org/2934103002/diff/1/webrtc/media/engine/webrtcvoi...
webrtc/media/engine/webrtcvoiceengine.cc:336: if (voe()->engine()) {
Note that this wasn't necessary before because the "for testing" constructor
simply didn't construct an AudioState.

[Taylor Brandstetter, 2017-06-14 17:34:57]

deadbeef@webrtc.org changed reviewers:
+ kwiberg@webrtc.org

[Taylor Brandstetter, 2017-06-14 17:59:15]

PTAL. This change would help simplify https://codereview.webrtc.org/2854123003/

[the sun, 2017-06-15 07:38:19]

LGTM

Note though, that generally, allowing half-initialized objects is a bad idea. :)
A more robust solution would be to split the object up so that RAII could be
used 100%. Though in this case I think the pragmatic solution here is fine.

Also, perhaps calls on the media engines should happen on a TaskQueue instead of
maintaining the signal/worker divide...

https://codereview.webrtc.org/2934103002/diff/1/webrtc/media/engine/webrtcvoi...
File webrtc/media/engine/webrtcvoiceengine.cc (right):

https://codereview.webrtc.org/2934103002/diff/1/webrtc/media/engine/webrtcvoi...
webrtc/media/engine/webrtcvoiceengine.cc:238: RTC_DCHECK(decoder_factory);
Might as well add the missing DCHECK for encoder_factory.

[Taylor Brandstetter, 2017-06-15 15:05:59]

On 2017/06/15 07:38:19, the sun wrote:
> LGTM
> 
> Note though, that generally, allowing half-initialized objects is a bad idea.
:)

Agreed. But for now, WebRtcMediaEngine and WebRtcVideoEngine already do have an
Init, so until we change the interface of WebRtcMediaEngine it makes sense to
make use of this pattern.

> Also, perhaps calls on the media engines should happen on a TaskQueue instead
of
> maintaining the signal/worker divide...

If we want things to work efficiently (without Thread::Invokes all over the
place) I think we'll definitely be doing this eventually.

https://codereview.webrtc.org/2934103002/diff/1/webrtc/media/engine/webrtcvoi...
File webrtc/media/engine/webrtcvoiceengine.cc (right):

https://codereview.webrtc.org/2934103002/diff/1/webrtc/media/engine/webrtcvoi...
webrtc/media/engine/webrtcvoiceengine.cc:238: RTC_DCHECK(decoder_factory);
On 2017/06/15 07:38:19, the sun wrote:
> Might as well add the missing DCHECK for encoder_factory.

Done.

[Taylor Brandstetter, 2017-06-15 15:06:06]

The CQ bit was checked by deadbeef@webrtc.org

[Taylor Brandstetter, 2017-06-15 15:06:07]

The patchset sent to the CQ was uploaded after l-g-t-m from solenberg@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2934103002/#ps20001 (title:
"Adding DCHECK for encoder_factory")

[commit-bot: I haz the power, 2017-06-15 15:06:12]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-06-15 15:29:26]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1497539166432420,
"parent_rev": "d03a578333029c3d8811c0abb9b21cd12a145891", "commit_rev":
"eb02c03a53734b6faf01d7f98a6174918169b850"}

[commit-bot: I haz the power, 2017-06-15 15:29:30]

Description was changed from

==========
Allow WebRtcMediaEngine to be created from any thread.

This eliminates a thread hop in PeerConnectionFactory initialization,
and will allow some code to be simplified.

BUG=None
==========

to

==========
Allow WebRtcMediaEngine to be created from any thread.

This eliminates a thread hop in PeerConnectionFactory initialization,
and will allow some code to be simplified.

BUG=None

Review-Url: https://codereview.webrtc.org/2934103002
Cr-Commit-Position: refs/heads/master@{#18613}
Committed:
https://chromium.googlesource.com/external/webrtc/+/eb02c03a53734b6faf01d7f98...
==========

[commit-bot: I haz the power, 2017-06-15 15:29:31]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/external/webrtc/+/eb02c03a53734b6faf01d7f98...