Simplify handling of options in WebRtcVoiceMediaEngine.

Simplify handling of options in WebRtcVoiceMediaEngine.
Also removes unnecessary typedef ChannelList.

BUG=webrtc:4690
Committed: https://crrev.com/63b345441a995665c1cdd0329b65f895675874ff
Cr-Commit-Position: refs/heads/master@{#10107}

[the sun, 2015-09-23 13:02:33]

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

[the sun, 2015-09-23 13:19:56]

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

[pbos-webrtc, 2015-09-23 13:41:34]

https://codereview.webrtc.org/1364753002/diff/1/talk/media/webrtc/webrtcvoice...
File talk/media/webrtc/webrtcvoiceengine.cc (right):

https://codereview.webrtc.org/1364753002/diff/1/talk/media/webrtc/webrtcvoice...
talk/media/webrtc/webrtcvoiceengine.cc:1222: auto i =
std::find(channels_.begin(), channels_.end(), channel);
s/i/it/

https://codereview.webrtc.org/1364753002/diff/1/talk/media/webrtc/webrtcvoice...
talk/media/webrtc/webrtcvoiceengine.cc:2053:
engine()->ApplyOptions(engine()->GetOptions());
Why is this not a no-op?

[the sun, 2015-09-23 15:11:28]

https://codereview.webrtc.org/1364753002/diff/1/talk/media/webrtc/webrtcvoice...
File talk/media/webrtc/webrtcvoiceengine.cc (right):

https://codereview.webrtc.org/1364753002/diff/1/talk/media/webrtc/webrtcvoice...
talk/media/webrtc/webrtcvoiceengine.cc:1222: auto i =
std::find(channels_.begin(), channels_.end(), channel);
On 2015/09/23 13:41:34, pbos-webrtc wrote:
> s/i/it/

Done.

https://codereview.webrtc.org/1364753002/diff/1/talk/media/webrtc/webrtcvoice...
talk/media/webrtc/webrtcvoiceengine.cc:2053:
engine()->ApplyOptions(engine()->GetOptions());
On 2015/09/23 13:41:34, pbos-webrtc wrote:
> Why is this not a no-op?

Because the engine options_ can be different from the channel options_. Yes, it
is confusing. This CL makes it a little less confusing.

[pbos-webrtc, 2015-09-23 15:13:14]

https://codereview.webrtc.org/1364753002/diff/1/talk/media/webrtc/webrtcvoice...
File talk/media/webrtc/webrtcvoiceengine.cc (right):

https://codereview.webrtc.org/1364753002/diff/1/talk/media/webrtc/webrtcvoice...
talk/media/webrtc/webrtcvoiceengine.cc:2053:
engine()->ApplyOptions(engine()->GetOptions());
On 2015/09/23 15:11:28, the sun wrote:
> On 2015/09/23 13:41:34, pbos-webrtc wrote:
> > Why is this not a no-op?
> 
> Because the engine options_ can be different from the channel options_. Yes,
it
> is confusing. This CL makes it a little less confusing.

But you're applying engine's GetOptions to engine? Or are there separate
engine()s? Put a comment here?

[the sun, 2015-09-23 15:29:22]

On 2015/09/23 15:13:14, pbos-webrtc wrote:
>
https://codereview.webrtc.org/1364753002/diff/1/talk/media/webrtc/webrtcvoice...
> File talk/media/webrtc/webrtcvoiceengine.cc (right):
> 
>
https://codereview.webrtc.org/1364753002/diff/1/talk/media/webrtc/webrtcvoice...
> talk/media/webrtc/webrtcvoiceengine.cc:2053:
> engine()->ApplyOptions(engine()->GetOptions());
> On 2015/09/23 15:11:28, the sun wrote:
> > On 2015/09/23 13:41:34, pbos-webrtc wrote:
> > > Why is this not a no-op?
> > 
> > Because the engine options_ can be different from the channel options_. Yes,
> it
> > is confusing. This CL makes it a little less confusing.
> 
> But you're applying engine's GetOptions to engine? Or are there separate
> engine()s? Put a comment here?

Good pint. Point.

[the sun, 2015-09-23 15:33:36]

Andrew, can you please check the sanity of this change? Is the logic for
resetting the options to the engine defaults really required?

[Andrew MacDonald, 2015-09-23 17:50:41]

andrew@webrtc.org changed reviewers:
+ pthatcher@webrtc.org

[Andrew MacDonald, 2015-09-23 17:51:12]

On 2015/09/23 15:33:36, the sun wrote:
> Andrew, can you please check the sanity of this change? Is the logic for
> resetting the options to the engine defaults really required?

I _think_ this was needed to support the Talk plugin's configuration file. It
shouldn't be needed any longer.

Peter T. added this code, he might recall better.

[pthatcher1, 2015-09-23 19:54:06]

On 2015/09/23 17:51:12, Andrew MacDonald wrote:
> On 2015/09/23 15:33:36, the sun wrote:
> > Andrew, can you please check the sanity of this change? Is the logic for
> > resetting the options to the engine defaults really required?
> 
> I _think_ this was needed to support the Talk plugin's configuration file. It
> shouldn't be needed any longer.
> 
> Peter T. added this code, he might recall better.

This looks like a good simplification.  I probably should have done it this way
back when I wrote the code.  Not sure why I made it so confusing.  Sorry about
that!

[the sun, 2015-09-24 07:57:37]

On 2015/09/23 19:54:06, pthatcher1 wrote:
> On 2015/09/23 17:51:12, Andrew MacDonald wrote:
> > On 2015/09/23 15:33:36, the sun wrote:
> > > Andrew, can you please check the sanity of this change? Is the logic for
> > > resetting the options to the engine defaults really required?
> > 
> > I _think_ this was needed to support the Talk plugin's configuration file.
It
> > shouldn't be needed any longer.
> > 
> > Peter T. added this code, he might recall better.
> 
> This looks like a good simplification.  I probably should have done it this
way
> back when I wrote the code.  Not sure why I made it so confusing.  Sorry about
> that!

Question is: do we really need to care about resetting the state once we stop
sending?

New options will be latched in when either a new channel is created, or sending
is started again.

[Andrew MacDonald, 2015-09-26 00:47:11]

On 2015/09/24 07:57:37, the sun wrote:
> Question is: do we really need to care about resetting the state once we stop
> sending?
> 
> New options will be latched in when either a new channel is created, or
sending
> is started again.

I think what you have is good. It's reasonable for the client to provide the
settings whenever they open a new channel.

As I mentioned, I think this was just to manage the case of the Talk plugin
having a default settings file.

[the sun, 2015-09-28 10:36:11]

So, provided you are ok with this, would you mind stamping it?

[Andrew MacDonald, 2015-09-28 19:34:45]

On 2015/09/28 10:36:11, the sun wrote:
> So, provided you are ok with this, would you mind stamping it?

Ah, lgtm.

But I'm not actually a talk/* OWNER. pthatcher will have to approve.

[pthatcher1, 2015-09-28 19:46:40]

lgtm

[the sun, 2015-09-29 08:13:20]

solenberg@webrtc.org changed reviewers:
- pbos@webrtc.org

[the sun, 2015-09-29 11:36:46]

The CQ bit was checked by solenberg@webrtc.org

[the sun, 2015-09-29 11:36:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
pthatcher@webrtc.org, andrew@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1364753002/#ps100001 (title:
"rebase")

[commit-bot: I haz the power, 2015-09-29 11:37:01]

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

[commit-bot: I haz the power, 2015-09-29 13:06:36]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-09-29 13:06:43]

Patchset 6 (id:??) landed as
https://crrev.com/63b345441a995665c1cdd0329b65f895675874ff
Cr-Commit-Position: refs/heads/master@{#10107}