Convert internal representation of Srtp cryptos from string to int.

Convert internal representation of Srtp cryptos from string to int.

Note that the coversion from int to string happens in 3 places
1) SDP layer from int to external names. mediasession.cc GetSupportedSuiteNames.
2) for SSL_CTX_set_tlsext_use_srtp(), converting from int to internal names.
3) stats collection also needs external names.

External names are like AES_CM_128_HMAC_SHA1_80, specified in sslstreamadapter.cc.
Internal names are like SRTP_AES128_CM_SHA1_80, specified in opensslstreamadapter.cc.

The conversion from string to int happens in one place only, SDP layer, SrtpFilter::ApplyParams().

BUG=webrtc:5043
Committed: https://crrev.com/2764e1027a08a5543e04b854a27a520801faf6eb
Cr-Commit-Position: refs/heads/master@{#10701}

[guoweis_webrtc, 2015-10-29 18:20:42]

Description was changed from

==========
Convert internal representation of Srtp cryptos from string to int.

Note that the coversion from int to string happens in 2 places
1) SDP layer from int to external names.
2) for SSL_CTX_set_tlsext_use_srtp(), converting from int to internal names.

External names are like AES_CM_128_HMAC_SHA1_80, specified in
sslstreamadapter.cc.
Internal names are like SRTP_AES128_CM_SHA1_80, specified in
opensslstreamadapter.cc.

The conversion from string to int happens in one place only, SDP layer.

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

to

==========
Convert internal representation of Srtp cryptos from string to int.

Note that the coversion from int to string happens in 3 places
1) SDP layer from int to external names. mediasession.cc GetSupportedSuiteNames.
2) for SSL_CTX_set_tlsext_use_srtp(), converting from int to internal names.
3) stats collection.

External names are like AES_CM_128_HMAC_SHA1_80, specified in
sslstreamadapter.cc.
Internal names are like SRTP_AES128_CM_SHA1_80, specified in
opensslstreamadapter.cc.

The conversion from string to int happens in one place only, SDP layer,
SrtpFilter::ApplyParams().

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

[guoweis_webrtc, 2015-10-29 18:21:13]

Description was changed from

==========
Convert internal representation of Srtp cryptos from string to int.

Note that the coversion from int to string happens in 3 places
1) SDP layer from int to external names. mediasession.cc GetSupportedSuiteNames.
2) for SSL_CTX_set_tlsext_use_srtp(), converting from int to internal names.
3) stats collection.

External names are like AES_CM_128_HMAC_SHA1_80, specified in
sslstreamadapter.cc.
Internal names are like SRTP_AES128_CM_SHA1_80, specified in
opensslstreamadapter.cc.

The conversion from string to int happens in one place only, SDP layer,
SrtpFilter::ApplyParams().

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

to

==========
Convert internal representation of Srtp cryptos from string to int.

Note that the coversion from int to string happens in 3 places
1) SDP layer from int to external names. mediasession.cc GetSupportedSuiteNames.
2) for SSL_CTX_set_tlsext_use_srtp(), converting from int to internal names.
3) stats collection also needs external names.

External names are like AES_CM_128_HMAC_SHA1_80, specified in
sslstreamadapter.cc.
Internal names are like SRTP_AES128_CM_SHA1_80, specified in
opensslstreamadapter.cc.

The conversion from string to int happens in one place only, SDP layer,
SrtpFilter::ApplyParams().

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

[guoweis_webrtc, 2015-10-29 18:31:02]

guoweis@webrtc.org changed reviewers:
+ juberti@webrtc.org, pthatcher@webrtc.org

[guoweis_webrtc, 2015-10-29 18:31:02]

PTAL.

[guoweis_webrtc, 2015-11-01 21:33:17]

Description was changed from

==========
Convert internal representation of Srtp cryptos from string to int.

Note that the coversion from int to string happens in 3 places
1) SDP layer from int to external names. mediasession.cc GetSupportedSuiteNames.
2) for SSL_CTX_set_tlsext_use_srtp(), converting from int to internal names.
3) stats collection also needs external names.

External names are like AES_CM_128_HMAC_SHA1_80, specified in
sslstreamadapter.cc.
Internal names are like SRTP_AES128_CM_SHA1_80, specified in
opensslstreamadapter.cc.

The conversion from string to int happens in one place only, SDP layer,
SrtpFilter::ApplyParams().

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

to

==========
Convert internal representation of Srtp cryptos from string to int.

Note that the coversion from int to string happens in 3 places
1) SDP layer from int to external names. mediasession.cc GetSupportedSuiteNames.
2) for SSL_CTX_set_tlsext_use_srtp(), converting from int to internal names.
3) stats collection also needs external names.

External names are like AES_CM_128_HMAC_SHA1_80, specified in
sslstreamadapter.cc.
Internal names are like SRTP_AES128_CM_SHA1_80, specified in
opensslstreamadapter.cc.

The conversion from string to int happens in one place only, SDP layer,
SrtpFilter::ApplyParams().

BUG=webrtc:5043
==========

[pthatcher1, 2015-11-11 19:59:41]

Looks much better, but I think we can improve some more places to be more
consistent with the names.

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/peerconne...
File talk/app/webrtc/peerconnection_unittest.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnection_unittest.cc:114: static const int
kDefaultSrtpCipher = rtc::SRTP_AES128_CM_SHA1_32;
Should we rename this to kDefaultSrtpCryptoSuite?

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/statscoll...
File talk/app/webrtc/statscollector.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/statscoll...
talk/app/webrtc/statscollector.cc:733: int srtp_cipher =
channel_iter.srtp_cipher;
Should we rename srtp_chiper to srtp_crypto_suite?  

I mean the local variable, not the member variable.

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/statscoll...
talk/app/webrtc/statscollector.cc:738: int ssl_cipher = channel_iter.ssl_cipher;
And should we rename ssl_cipher to ssl_cipher_suite?

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/statscoll...
File talk/app/webrtc/statscollector_unittest.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/statscoll...
talk/app/webrtc/statscollector_unittest.cc:750: std::string dtls_cipher =
ExtractStatsValue(
Similarly here and below with x_cipher to x_cipher_suite?

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/webrtcses...
File talk/app/webrtc/webrtcsession.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/webrtcses...
talk/app/webrtc/webrtcsession.cc:2168: int ssl_cipher =
stats.channel_stats[0].ssl_cipher;
And here to?

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/channe...
File talk/session/media/channel.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/channe...
talk/session/media/channel.cc:810: std::vector<int> ciphers;
Similarly here, with crypto_suites?

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/channe...
talk/session/media/channel.cc:837: int selected_cipher;
And here with selected_crypto_suite?

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/channel.h
File talk/session/media/channel.h (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/channe...
talk/session/media/channel.h:286: virtual void
GetSrtpCryptoSuites(std::vector<int>* ciphers) const = 0;
Should we use an enum instead of an int?

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/channe...
talk/session/media/channel.h:409: virtual void
GetSrtpCryptoSuites(std::vector<int>* ciphers) const;
Shouldn't the variable name be crypto_suites?  Here and above and below.

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/medias...
File talk/session/media/mediasession.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/medias...
talk/session/media/mediasession.cc:165: std::vector<int> cryptos;
I think crypto_suites or suites would be better.

Here and below.

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/medias...
File talk/session/media/mediasession.h (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/medias...
talk/session/media/mediasession.h:557: void
GetDefaultSrtpCryptoSuites(std::vector<int>* crypto_suites);
And should these be changed from crypto_suites to crypto_suite_names or cs_names
or just names?

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/srtpfi...
File talk/session/media/srtpfilter.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/srtpfi...
talk/session/media/srtpfilter.cc:432:
rtc::SrtpCryptoSuiteNameToId(send_params.cipher_suite), send_key,
I think SrtpCryptoSuiteFromName would be a better method name.

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/base/opensslstream...
File webrtc/base/opensslstreamadapter.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/base/opensslstream...
webrtc/base/opensslstreamadapter.cc:353: std::string
OpenSSLStreamAdapter::SslCipherSuiteToName(int cipher) {
cipher_suite or suite?

Same here and below

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/base/opensslstream...
webrtc/base/opensslstreamadapter.cc:448: bool
OpenSSLStreamAdapter::GetDtlsSrtpCryptoSuite(int* cipher) {
crypto_suite

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/base/opensslstream...
File webrtc/base/opensslstreamadapter.h (right):

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/base/opensslstream...
webrtc/base/opensslstreamadapter.h:91: static std::string
SslCipherSuiteToName(int cipher);
cipher_suite?

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/base/opensslstream...
webrtc/base/opensslstreamadapter.h:105: bool GetDtlsSrtpCryptoSuite(int* cipher)
override;
crypto_suites or suites?

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/base/sslstreamadap...
File webrtc/base/sslstreamadapter.h (right):

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/base/sslstreamadap...
webrtc/base/sslstreamadapter.h:181: virtual bool GetDtlsSrtpCryptoSuite(int*
cipher);
More here.

I'm going to stop commenting on these.  Please find the rest and remain
consistent.

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/p2p/base/dtlstrans...
File webrtc/p2p/base/dtlstransportchannel_unittest.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/p2p/base/dtlstrans...
webrtc/p2p/base/dtlstransportchannel_unittest.cc:475: client2_.CheckSrtp(0);
Can we make a constant for "rtc::SRTP_CRYPTO_SUITE_UNSET = 0" or something
similar?

[guoweis_webrtc, 2015-11-17 01:21:16]

PTAL. I ended up renaming these vars.

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/peerconne...
File talk/app/webrtc/peerconnection_unittest.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnection_unittest.cc:114: static const int
kDefaultSrtpCipher = rtc::SRTP_AES128_CM_SHA1_32;
On 2015/11/11 19:59:40, pthatcher1 wrote:
> Should we rename this to kDefaultSrtpCryptoSuite?

Done.

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/statscoll...
File talk/app/webrtc/statscollector.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/statscoll...
talk/app/webrtc/statscollector.cc:733: int srtp_cipher =
channel_iter.srtp_cipher;
On 2015/11/11 19:59:40, pthatcher1 wrote:
> Should we rename srtp_chiper to srtp_crypto_suite?  
> 
> I mean the local variable, not the member variable.

Done.

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/statscoll...
talk/app/webrtc/statscollector.cc:738: int ssl_cipher = channel_iter.ssl_cipher;
On 2015/11/11 19:59:40, pthatcher1 wrote:
> And should we rename ssl_cipher to ssl_cipher_suite?

Done.

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/statscoll...
File talk/app/webrtc/statscollector_unittest.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/statscoll...
talk/app/webrtc/statscollector_unittest.cc:750: std::string dtls_cipher =
ExtractStatsValue(
On 2015/11/11 19:59:40, pthatcher1 wrote:
> Similarly here and below with x_cipher to x_cipher_suite?

Done.

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/webrtcses...
File talk/app/webrtc/webrtcsession.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/app/webrtc/webrtcses...
talk/app/webrtc/webrtcsession.cc:2168: int ssl_cipher =
stats.channel_stats[0].ssl_cipher;
On 2015/11/11 19:59:40, pthatcher1 wrote:
> And here to?

Done.

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/channe...
File talk/session/media/channel.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/channe...
talk/session/media/channel.cc:810: std::vector<int> ciphers;
On 2015/11/11 19:59:40, pthatcher1 wrote:
> Similarly here, with crypto_suites?

Done.

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/channe...
talk/session/media/channel.cc:837: int selected_cipher;
On 2015/11/11 19:59:40, pthatcher1 wrote:
> And here with selected_crypto_suite?

Done.

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/channel.h
File talk/session/media/channel.h (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/channe...
talk/session/media/channel.h:286: virtual void
GetSrtpCryptoSuites(std::vector<int>* ciphers) const = 0;
On 2015/11/11 19:59:40, pthatcher1 wrote:
> Should we use an enum instead of an int?

just like SSL, the integer is well defined and we shouldn't reinvent them, which
creates need for code change when new suite pops up.

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/channe...
talk/session/media/channel.h:409: virtual void
GetSrtpCryptoSuites(std::vector<int>* ciphers) const;
On 2015/11/11 19:59:40, pthatcher1 wrote:
> Shouldn't the variable name be crypto_suites?  Here and above and below.

Done.

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/medias...
File talk/session/media/mediasession.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/medias...
talk/session/media/mediasession.cc:165: std::vector<int> cryptos;
On 2015/11/11 19:59:40, pthatcher1 wrote:
> I think crypto_suites or suites would be better.
> 
> Here and below.

Done.

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/medias...
File talk/session/media/mediasession.h (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/medias...
talk/session/media/mediasession.h:557: void
GetDefaultSrtpCryptoSuites(std::vector<int>* crypto_suites);
On 2015/11/11 19:59:40, pthatcher1 wrote:
> And should these be changed from crypto_suites to crypto_suite_names or
cs_names
> or just names?

Done.

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/srtpfi...
File talk/session/media/srtpfilter.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/talk/session/media/srtpfi...
talk/session/media/srtpfilter.cc:432:
rtc::SrtpCryptoSuiteNameToId(send_params.cipher_suite), send_key,
On 2015/11/11 19:59:40, pthatcher1 wrote:
> I think SrtpCryptoSuiteFromName would be a better method name.

Done.

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/base/opensslstream...
File webrtc/base/opensslstreamadapter.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/base/opensslstream...
webrtc/base/opensslstreamadapter.cc:353: std::string
OpenSSLStreamAdapter::SslCipherSuiteToName(int cipher) {
On 2015/11/11 19:59:40, pthatcher1 wrote:
> cipher_suite or suite?
> 
> Same here and below

Done.

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/base/opensslstream...
webrtc/base/opensslstreamadapter.cc:448: bool
OpenSSLStreamAdapter::GetDtlsSrtpCryptoSuite(int* cipher) {
On 2015/11/11 19:59:40, pthatcher1 wrote:
> crypto_suite

Done.

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/base/opensslstream...
File webrtc/base/opensslstreamadapter.h (right):

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/base/opensslstream...
webrtc/base/opensslstreamadapter.h:91: static std::string
SslCipherSuiteToName(int cipher);
On 2015/11/11 19:59:41, pthatcher1 wrote:
> cipher_suite?

Done.

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/base/opensslstream...
webrtc/base/opensslstreamadapter.h:105: bool GetDtlsSrtpCryptoSuite(int* cipher)
override;
On 2015/11/11 19:59:40, pthatcher1 wrote:
> crypto_suites or suites?

Done.

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/base/sslstreamadap...
File webrtc/base/sslstreamadapter.h (right):

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/base/sslstreamadap...
webrtc/base/sslstreamadapter.h:181: virtual bool GetDtlsSrtpCryptoSuite(int*
cipher);
On 2015/11/11 19:59:41, pthatcher1 wrote:
> More here.
> 
> I'm going to stop commenting on these.  Please find the rest and remain
> consistent.

Done.

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/p2p/base/dtlstrans...
File webrtc/p2p/base/dtlstransportchannel_unittest.cc (right):

https://codereview.webrtc.org/1416673006/diff/40001/webrtc/p2p/base/dtlstrans...
webrtc/p2p/base/dtlstransportchannel_unittest.cc:475: client2_.CheckSrtp(0);
On 2015/11/11 19:59:41, pthatcher1 wrote:
> Can we make a constant for "rtc::SRTP_CRYPTO_SUITE_UNSET = 0" or something
> similar?

Done.

[pthatcher1, 2015-11-17 21:02:05]

lgtm

[guoweis_webrtc, 2015-11-17 21:14:25]

The CQ bit was checked by guoweis@webrtc.org

[commit-bot: I haz the power, 2015-11-17 21:14:35]

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

[juberti, 2015-11-17 21:19:07]

juberti@google.com changed reviewers:
+ juberti@google.com

[juberti, 2015-11-17 21:19:08]

lgtm with a few comments

https://codereview.webrtc.org/1416673006/diff/100001/talk/session/media/media...
File talk/session/media/mediasession.cc (right):

https://codereview.webrtc.org/1416673006/diff/100001/talk/session/media/media...
talk/session/media/mediasession.cc:196: void GetDefaultSrtpCryptoSuiteNames(
Seems like it would be simplest to have a wrapper function, and the
GetXXXXSrtpCryptoSuiteNames function would just call
wrapper(GetXXXSrtpCryptoSuites).

https://codereview.webrtc.org/1416673006/diff/100001/webrtc/base/sslstreamada...
File webrtc/base/sslstreamadapter.cc (right):

https://codereview.webrtc.org/1416673006/diff/100001/webrtc/base/sslstreamada...
webrtc/base/sslstreamadapter.cc:33: std::string SrtpCryptoSuiteToName(int
srtp_crypto) {
naming: crypto_suite, here and in the function below?

https://codereview.webrtc.org/1416673006/diff/100001/webrtc/base/sslstreamada...
File webrtc/base/sslstreamadapter.h (right):

https://codereview.webrtc.org/1416673006/diff/100001/webrtc/base/sslstreamada...
webrtc/base/sslstreamadapter.h:23: const int SRTP_CRYPTO_SUITE_UNSET = 0;
Would SRTP_INVALID_CRYPTO_SUITE make more sense?

https://codereview.webrtc.org/1416673006/diff/100001/webrtc/p2p/base/faketran...
File webrtc/p2p/base/faketransportcontroller.h (right):

https://codereview.webrtc.org/1416673006/diff/100001/webrtc/p2p/base/faketran...
webrtc/p2p/base/faketransportcontroller.h:278: if (chosen_srtp_cipher_) {
should this have an explicit "!= SRTP_INVALID_CRYPTO_SUITE" check?

https://codereview.webrtc.org/1416673006/diff/100001/webrtc/p2p/base/faketran...
webrtc/p2p/base/faketransportcontroller.h:325: int chosen_srtp_cipher_ = 0;
should this be set to SRTP_INVALID_CRYPTO_SUITE?

[guoweis_webrtc, 2015-11-17 21:22:02]

The CQ bit was unchecked by guoweis@webrtc.org

[guoweis_webrtc, 2015-11-18 18:41:20]

The CQ bit was checked by guoweis@webrtc.org

[guoweis_webrtc, 2015-11-18 18:41:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
pthatcher@webrtc.org, juberti@google.com
Link to the patchset: https://codereview.webrtc.org/1416673006/#ps120001 (title:
"address comments.")

[commit-bot: I haz the power, 2015-11-18 18:41:27]

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

[commit-bot: I haz the power, 2015-11-18 18:43:31]

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

[commit-bot: I haz the power, 2015-11-18 18:43:31]

Try jobs failed on following builders:
  linux_msan on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_msan/builds/6027)
  linux_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_rel/builds/10858)
  mac_compile_x64_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_x64_dbg/bui...)

[guoweis_webrtc, 2015-11-18 18:45:20]

The CQ bit was checked by guoweis@webrtc.org

[guoweis_webrtc, 2015-11-18 18:45:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
pthatcher@webrtc.org, juberti@google.com
Link to the patchset: https://codereview.webrtc.org/1416673006/#ps140001 (title:
"missed a name change.")

[commit-bot: I haz the power, 2015-11-18 18:45:30]

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

[commit-bot: I haz the power, 2015-11-18 18:54:40]

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

[commit-bot: I haz the power, 2015-11-18 18:54:41]

Try jobs failed on following builders:
  win_x64_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_rel/builds/10735)

[guoweis_webrtc, 2015-11-18 19:16:58]

The CQ bit was checked by guoweis@webrtc.org

[guoweis_webrtc, 2015-11-18 19:16:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
pthatcher@webrtc.org, juberti@google.com
Link to the patchset: https://codereview.webrtc.org/1416673006/#ps160001 (title:
"fix a bug")

[commit-bot: I haz the power, 2015-11-18 19:17:11]

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

[guoweis_webrtc, 2015-11-18 19:17:18]

Landing this now.

https://codereview.webrtc.org/1416673006/diff/100001/talk/session/media/media...
File talk/session/media/mediasession.cc (right):

https://codereview.webrtc.org/1416673006/diff/100001/talk/session/media/media...
talk/session/media/mediasession.cc:196: void GetDefaultSrtpCryptoSuiteNames(
On 2015/11/17 21:19:08, juberti wrote:
> Seems like it would be simplest to have a wrapper function, and the
> GetXXXXSrtpCryptoSuiteNames function would just call
> wrapper(GetXXXSrtpCryptoSuites).

Do you mean the following:

std::vector<int> GetSupportedDataCryptoSuites();
std::vector<string> ConvertSrtpSuitesToNames(const std::vector<int>& );

https://codereview.webrtc.org/1416673006/diff/100001/webrtc/base/sslstreamada...
File webrtc/base/sslstreamadapter.cc (right):

https://codereview.webrtc.org/1416673006/diff/100001/webrtc/base/sslstreamada...
webrtc/base/sslstreamadapter.cc:33: std::string SrtpCryptoSuiteToName(int
srtp_crypto) {
On 2015/11/17 21:19:08, juberti wrote:
> naming: crypto_suite, here and in the function below?

Done.

https://codereview.webrtc.org/1416673006/diff/100001/webrtc/base/sslstreamada...
File webrtc/base/sslstreamadapter.h (right):

https://codereview.webrtc.org/1416673006/diff/100001/webrtc/base/sslstreamada...
webrtc/base/sslstreamadapter.h:23: const int SRTP_CRYPTO_SUITE_UNSET = 0;
On 2015/11/17 21:19:08, juberti wrote:
> Would SRTP_INVALID_CRYPTO_SUITE make more sense?

Done.

https://codereview.webrtc.org/1416673006/diff/100001/webrtc/p2p/base/faketran...
File webrtc/p2p/base/faketransportcontroller.h (right):

https://codereview.webrtc.org/1416673006/diff/100001/webrtc/p2p/base/faketran...
webrtc/p2p/base/faketransportcontroller.h:278: if (chosen_srtp_cipher_) {
On 2015/11/17 21:19:08, juberti wrote:
> should this have an explicit "!= SRTP_INVALID_CRYPTO_SUITE" check?

Done.

https://codereview.webrtc.org/1416673006/diff/100001/webrtc/p2p/base/faketran...
webrtc/p2p/base/faketransportcontroller.h:325: int chosen_srtp_cipher_ = 0;
On 2015/11/17 21:19:08, juberti wrote:
> should this be set to SRTP_INVALID_CRYPTO_SUITE?

Done.

[juberti, 2015-11-18 20:19:15]

https://codereview.webrtc.org/1416673006/diff/100001/talk/session/media/media...
File talk/session/media/mediasession.cc (right):

https://codereview.webrtc.org/1416673006/diff/100001/talk/session/media/media...
talk/session/media/mediasession.cc:196: void GetDefaultSrtpCryptoSuiteNames(
On 2015/11/18 19:17:17, guoweis wrote:
> On 2015/11/17 21:19:08, juberti wrote:
> > Seems like it would be simplest to have a wrapper function, and the
> > GetXXXXSrtpCryptoSuiteNames function would just call
> > wrapper(GetXXXSrtpCryptoSuites).
> 
> Do you mean the following:
> 
> std::vector<int> GetSupportedDataCryptoSuites();
> std::vector<string> ConvertSrtpSuitesToNames(const std::vector<int>& );

more like GetSupportedCryptoSuiteNames(void (*func)(std::vector<int>*),
std::vector<std::string>* names)

this function would be a helper, and then the actual
GetSupportedXXXXCryptoSuiteNames functions would just be a thin wrapper around
this function.

[guoweis_webrtc, 2015-11-18 21:05:47]

The CQ bit was unchecked by guoweis@webrtc.org

[guoweis_webrtc, 2015-11-18 23:49:37]

The CQ bit was checked by guoweis@webrtc.org

[guoweis_webrtc, 2015-11-18 23:49:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
pthatcher@webrtc.org, juberti@google.com
Link to the patchset: https://codereview.webrtc.org/1416673006/#ps180001 (title:
"change srtp crypto name conversion")

[commit-bot: I haz the power, 2015-11-18 23:49:50]

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

[juberti, 2015-11-19 01:26:48]

lgtm

[commit-bot: I haz the power, 2015-11-19 02:02:36]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-11-19 02:02:51]

Patchset 10 (id:??) landed as
https://crrev.com/2764e1027a08a5543e04b854a27a520801faf6eb
Cr-Commit-Position: refs/heads/master@{#10701}

[guoweis_webrtc, 2015-11-19 03:03:14]

A revert of this CL (patchset #10 id:180001) has been created in
https://codereview.webrtc.org/1455233005/ by guoweis@webrtc.org.

The reason for reverting is: Broke chromium fyi build. .