Add UMA metrics and finch experiment for DTLS1.2 in WebRTC.

Add UMA metrics and finch experiment for DTLS1.2 in WebRTC.

The corresponding webrtc change is https://codereview.webrtc.org/1337673002/.

BUG=523033
Committed: https://crrev.com/8ff81738a6f8b74e09966335be2849c04a8cbde1
Cr-Commit-Position: refs/heads/master@{#351936}

[guoweis_left_chromium, 2015-09-22 21:02:06]

Patchset #2 (id:20001) has been deleted

[guoweis_left_chromium, 2015-09-22 21:04:57]

Patchset #4 (id:80001) has been deleted

[guoweis_left_chromium, 2015-09-22 21:05:19]

Patchset #1 (id:1) has been deleted

[guoweis_left_chromium, 2015-09-22 21:05:24]

Patchset #1 (id:40001) has been deleted

[guoweis_left_chromium, 2015-09-22 21:05:29]

Patchset #1 (id:60001) has been deleted

[guoweis_left_chromium, 2015-09-22 21:12:52]

guoweis@chromium.org changed reviewers:
+ rsleevi@chromium.org

[guoweis_left_chromium, 2015-09-22 21:12:53]

Hi, Ryan:

Could you review this code change? I'm trying to map SSL cipher from WebRTC (as
defined kSslCipherMap from opensslstsreamadapter.cc) into the existing
SSLCipherSuite in histograms.xml
(https://code.google.com/p/chromium/codesearch#chromium/src/tools/metrics/hist...).

However, what I found out is that some of the ssl ciphers, although defined in
histograms.xml, there is no matching definition in
net/third_party/nss/ssl/sslproto.h.

Here is the list of ciphers which I can't find matching definition of.

      // SSLCIPHER_ENTRY(TLS_RSA_WITH_AES_256_GCM_SHA384),
      // SSLCIPHER_ENTRY(TLS_DHE_RSA_WITH_AES_256_GCM_SHA384),
      // SSLCIPHER_ENTRY(TLS_DH_RSA_WITH_AES_128_GCM_SHA256),
      // SSLCIPHER_ENTRY(TLS_DH_RSA_WITH_AES_256_GCM_SHA384),

      // ECDH HMAC based ciphersuites from RFC5289.
      // SSLCIPHER_ENTRY(TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384),
      // SSLCIPHER_ENTRY(TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384),

      // ECDH GCM based ciphersuites from RFC5289.
      // SSLCIPHER_ENTRY(TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384),
      // SSLCIPHER_ENTRY(TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384),

Do you know why it's the case?

Thanks,
Guowei

[guoweis_left_chromium, 2015-09-22 21:14:20]

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

[guoweis_left_chromium, 2015-09-22 21:14:21]

+ others. 

Btw, the WebRTC ssl cipher definition is from the on-going CL:
https://codereview.webrtc.org/1337673002/

[Ryan Sleevi, 2015-09-22 21:20:56]

On 2015/09/22 21:12:53, guoweis_chromium wrote:
> Hi, Ryan:
> 
> Could you review this code change? I'm trying to map SSL cipher from WebRTC
(as
> defined kSslCipherMap from opensslstsreamadapter.cc) into the existing
> SSLCipherSuite in histograms.xml
>
(https://code.google.com/p/chromium/codesearch#chromium/src/tools/metrics/hist...).
> 
> However, what I found out is that some of the ssl ciphers, although defined in
> histograms.xml, there is no matching definition in
> net/third_party/nss/ssl/sslproto.h.
> 
> Here is the list of ciphers which I can't find matching definition of.
> 
>       // SSLCIPHER_ENTRY(TLS_RSA_WITH_AES_256_GCM_SHA384),
>       // SSLCIPHER_ENTRY(TLS_DHE_RSA_WITH_AES_256_GCM_SHA384),
>       // SSLCIPHER_ENTRY(TLS_DH_RSA_WITH_AES_128_GCM_SHA256),
>       // SSLCIPHER_ENTRY(TLS_DH_RSA_WITH_AES_256_GCM_SHA384),
> 
>       // ECDH HMAC based ciphersuites from RFC5289.
>       // SSLCIPHER_ENTRY(TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384),
>       // SSLCIPHER_ENTRY(TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384),
> 
>       // ECDH GCM based ciphersuites from RFC5289.
>       // SSLCIPHER_ENTRY(TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384),
>       // SSLCIPHER_ENTRY(TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384),
> 
> Do you know why it's the case?

because we don't implement some of them?

Nothing should ever be using DH_RSA in this day and age, GCM or not.

[Ryan Sleevi, 2015-09-22 21:21:10]

https://codereview.chromium.org/1335023002/diff/100001/content/renderer/media...
File content/renderer/media/rtc_peer_connection_handler.cc (right):

https://codereview.chromium.org/1335023002/diff/100001/content/renderer/media...
content/renderer/media/rtc_peer_connection_handler.cc:35: #include
"net/third_party/nss/ssl/sslproto.h"
BUG: You should not be directly including NSS headers, especially since Chrome
now uses BoringSSL.

https://codereview.chromium.org/1335023002/diff/100001/content/renderer/media...
content/renderer/media/rtc_peer_connection_handler.cc:126: }
This is very inefficient for what it does - given that you've got a static
table, in source, you should be able to easily pre-sort this table and then use
either std::lower_bound or std::binary_search as appropriate.

Alternatively, if you simply define the WebRTC enum values to match their TLS
wire encoding, you avoid the need for any such mapping at all.

[davidben, 2015-09-22 21:29:46]

davidben@chromium.org changed reviewers:
+ davidben@chromium.org

[davidben, 2015-09-22 21:29:46]

https://codereview.chromium.org/1335023002/diff/100001/content/renderer/media...
File content/renderer/media/rtc_peer_connection_handler.cc (right):

https://codereview.chromium.org/1335023002/diff/100001/content/renderer/media...
content/renderer/media/rtc_peer_connection_handler.cc:126: }
On 2015/09/22 21:21:10, Ryan Sleevi wrote:
> This is very inefficient for what it does - given that you've got a static
> table, in source, you should be able to easily pre-sort this table and then
use
> either std::lower_bound or std::binary_search as appropriate.
> 
> Alternatively, if you simply define the WebRTC enum values to match their TLS
> wire encoding, you avoid the need for any such mapping at all.

+1 to that. TLS already provides standard well-known values for each of these.
Please don't add a new enum. New ciphers may be added in the future and I don't
want to have to chase down multiple enums just to add them. (The WebRTC enum
cites kSslCipherMap which isn't even used in BoringSSL!)

[guoweis_left_chromium, 2015-09-22 21:31:59]

On 2015/09/22 21:29:46, David Benjamin wrote:
>
https://codereview.chromium.org/1335023002/diff/100001/content/renderer/media...
> File content/renderer/media/rtc_peer_connection_handler.cc (right):
> 
>
https://codereview.chromium.org/1335023002/diff/100001/content/renderer/media...
> content/renderer/media/rtc_peer_connection_handler.cc:126: }
> On 2015/09/22 21:21:10, Ryan Sleevi wrote:
> > This is very inefficient for what it does - given that you've got a static
> > table, in source, you should be able to easily pre-sort this table and then
> use
> > either std::lower_bound or std::binary_search as appropriate.
> > 
> > Alternatively, if you simply define the WebRTC enum values to match their
TLS
> > wire encoding, you avoid the need for any such mapping at all.
> 
> +1 to that. TLS already provides standard well-known values for each of these.
> Please don't add a new enum. New ciphers may be added in the future and I
don't
> want to have to chase down multiple enums just to add them. (The WebRTC enum
> cites kSslCipherMap which isn't even used in BoringSSL!)

I'm not an expert in crypto. Could you point me to these well-known definition?
When I searched for some cipher name, there are always multiple copies around in
the code and get me confused.

[Ryan Sleevi, 2015-09-22 21:34:38]

On 2015/09/22 21:31:59, guoweis_chromium wrote:
> I'm not an expert in crypto. Could you point me to these well-known
definition?
> When I searched for some cipher name, there are always multiple copies around
in
> the code and get me confused.

http://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-param...

[davidben, 2015-09-22 21:36:47]

On 2015/09/22 21:31:59, guoweis_chromium wrote:
> On 2015/09/22 21:29:46, David Benjamin wrote:
> >
>
https://codereview.chromium.org/1335023002/diff/100001/content/renderer/media...
> > File content/renderer/media/rtc_peer_connection_handler.cc (right):
> > 
> >
>
https://codereview.chromium.org/1335023002/diff/100001/content/renderer/media...
> > content/renderer/media/rtc_peer_connection_handler.cc:126: }
> > On 2015/09/22 21:21:10, Ryan Sleevi wrote:
> > > This is very inefficient for what it does - given that you've got a static
> > > table, in source, you should be able to easily pre-sort this table and
then
> > use
> > > either std::lower_bound or std::binary_search as appropriate.
> > > 
> > > Alternatively, if you simply define the WebRTC enum values to match their
> TLS
> > > wire encoding, you avoid the need for any such mapping at all.
> > 
> > +1 to that. TLS already provides standard well-known values for each of
these.
> > Please don't add a new enum. New ciphers may be added in the future and I
> don't
> > want to have to chase down multiple enums just to add them. (The WebRTC enum
> > cites kSslCipherMap which isn't even used in BoringSSL!)
> 
> I'm not an expert in crypto. Could you point me to these well-known
definition?
> When I searched for some cipher name, there are always multiple copies around
in
> the code and get me confused.

The values are always the same. They're assigned by IANA:
http://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-param...

But you shouldn't be replicating a list of them at all. Extract the uint16_t out
of BoringSSL and then pass it straight up to WebRTC's embedder without going
through any string representation. Then histogram that value straight. At no
point should WebRTC ever have to maintain a giant mapping for all cipher suites
that exist.

https://code.google.com/p/chromium/codesearch#chromium/src/net/socket/ssl_cli...
https://boringssl.googlesource.com/boringssl/+/master/include/openssl/ssl.h#193

(That you need to cast to uint16_t is due to a dumb OpenSSL thing. They put
extra gunk in the high bits of the ID.)

[juberti2, 2015-09-24 13:29:44]

Agree we shouldn't be maintaining our own enum and converting.

We should either pass strings up as far as possible, and then call a
WebRTC-provided routine to convert string->IANA id, or push up string and id as
suggested.

I tend to prefer the first approach since I think the only thing that the id is
actually useful for is this specific case, and the first approach maintains
symmetry between SSL and SRTP.

https://codereview.chromium.org/1335023002/diff/100001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1335023002/diff/100001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:51748: +    first transport signals the
OnCompleted event.
We might not have all the ciphers determined at the time the first transport
goes to completed, i.e. if we have multiple transports.

[davidben, 2015-09-24 15:42:39]

On 2015/09/24 13:29:44, juberti2 wrote:
> Agree we shouldn't be maintaining our own enum and converting.
> 
> We should either pass strings up as far as possible, and then call a
> WebRTC-provided routine to convert string->IANA id, or push up string and id
as
> suggested.
> 
> I tend to prefer the first approach since I think the only thing that the id
is
> actually useful for is this specific case, and the first approach maintains
> symmetry between SSL and SRTP.

BoringSSL doesn't have an API to convert from IANA names to IDs, only the other
way around. Pushing up the ID seems better since any SSL implementation
necessarily has to be able to look up by ID, but you typically never have to
look up by IANA name. (BoringSSL doesn't even maintain a list of IANA names. For
historical reasons, OpenSSL used different names in cipher strings.)

[juberti2, 2015-09-24 16:15:48]

On 2015/09/24 15:42:39, David Benjamin wrote:
> On 2015/09/24 13:29:44, juberti2 wrote:
> > Agree we shouldn't be maintaining our own enum and converting.
> > 
> > We should either pass strings up as far as possible, and then call a
> > WebRTC-provided routine to convert string->IANA id, or push up string and id
> as
> > suggested.
> > 
> > I tend to prefer the first approach since I think the only thing that the id
> is
> > actually useful for is this specific case, and the first approach maintains
> > symmetry between SSL and SRTP.
> 
> BoringSSL doesn't have an API to convert from IANA names to IDs, only the
other
> way around. Pushing up the ID seems better since any SSL implementation
> necessarily has to be able to look up by ID, but you typically never have to
> look up by IANA name. (BoringSSL doesn't even maintain a list of IANA names.
For
> historical reasons, OpenSSL used different names in cipher strings.)

Sure, but this all seems like artifice given that the only reason we are
reporting ids instead of names is to meet the UMA criterion that the histogram
value be an int; certainly the string would have been more readable.

Anyway, if BoringSSL won't even support the former approach without some
surgery, I can be persuaded to go with the latter.

[Ryan Sleevi, 2015-09-24 16:20:45]

On 2015/09/24 16:15:48, juberti2 wrote:
> Sure, but this all seems like artifice given that the only reason we are
> reporting ids instead of names is to meet the UMA criterion that the histogram
> value be an int; certainly the string would have been more readable.

I'm not sure I understand who it would be more readable for.

If for code, then constants / enums are just as readable as strings.
If for debugging, well, that's a debugging aid then, and that's just talking
about API convenience.

As David mentioned, TLS APIs strongly eschew the string form (it's inefficient
to code size and to lookups, there were existing strings that conflicted with
the IANA DB during the SSL3 -> TLS1 conversion), which is why they all* (GnuTLS,
OpenSSL, BoringSSL, NSS, MatrixSSL, PolarSSL) use the ID form.

It's just convenient that UMA also needs an ID form, so we can just easily map
them.

For SRTP, it uses strings because it uses SDP. If SDP were a binary format, it's
perfectly reasonable to believe that the APIs would have been based solely
around the numeric ID.

Anyways, sounds like there's agreement that the asymmetry (string vs ID) is a
true and accurate reflection of the underlying protocols, and thus perfectly
reasonable. For the case where we want to UMA the string form (SRTP cipher),
having a conversion is fine, and that conversion should be kept as close to the
thing that does SRTP - so WebRTC. By having WebRTC convert the string to a
WebRTC-defined enum, you preserve the API boundary and strict typing. We should
then convert that WebRTC-defined enum to a Chrome-defined enum, as part of
ensuring good insulation between API changes in WebRTC (similar to how we map
IDs from Blink).

[davidben, 2015-09-24 16:22:49]

On 2015/09/24 16:15:48, juberti2 wrote:
> On 2015/09/24 15:42:39, David Benjamin wrote:
> > On 2015/09/24 13:29:44, juberti2 wrote:
> > > Agree we shouldn't be maintaining our own enum and converting.
> > > 
> > > We should either pass strings up as far as possible, and then call a
> > > WebRTC-provided routine to convert string->IANA id, or push up string and
id
> > as
> > > suggested.
> > > 
> > > I tend to prefer the first approach since I think the only thing that the
id
> > is
> > > actually useful for is this specific case, and the first approach
maintains
> > > symmetry between SSL and SRTP.
> > 
> > BoringSSL doesn't have an API to convert from IANA names to IDs, only the
> other
> > way around. Pushing up the ID seems better since any SSL implementation
> > necessarily has to be able to look up by ID, but you typically never have to
> > look up by IANA name. (BoringSSL doesn't even maintain a list of IANA names.
> For
> > historical reasons, OpenSSL used different names in cipher strings.)
> 
> Sure, but this all seems like artifice given that the only reason we are
> reporting ids instead of names is to meet the UMA criterion that the histogram
> value be an int; certainly the string would have been more readable.

It's not just UMA. A cipher suite in TLS is identified a uint16. That's simply
what they are. At no point in any protocol do you refer to them by name. In
fact, one of the proposals I've seen for TLS 1.3 (that I'm not a fan of, but
nevermind that) would involve changing a few of the names. Same with some of the
extension names which is equally a no-op as extensions are numbers.

> Anyway, if BoringSSL won't even support the former approach without some
> surgery, I can be persuaded to go with the latter.

[davidben, 2015-09-24 16:23:46]

On 2015/09/24 16:22:49, David Benjamin wrote:
> (that I'm not a fan of, but nevermind that)

(Not a fan of the particular proposal, that is. TLS 1.3 in general is fine, as
is changing those names.)

[guoweis_left_chromium, 2015-09-24 18:32:55]

Patchset #2 (id:120001) has been deleted

[juberti2, 2015-09-24 21:15:22]

On 2015/09/24 16:23:46, David Benjamin wrote:
> On 2015/09/24 16:22:49, David Benjamin wrote:
> > (that I'm not a fan of, but nevermind that)
> 
> (Not a fan of the particular proposal, that is. TLS 1.3 in general is fine, as
> is changing those names.)

If the names can change over time, then certainly my proposal is out the window.

So sure - let's go with IDs, and use them *both* for TLS and SRTP. In DTLS-SRTP,
each SRTP ciphersuite (at least the ones we care about) is given a uint16 id as
well, so we can just use those numbers rather than having to have webrtc local
enums. See https://tools.ietf.org/html/rfc5764#section-4.1.2.

[guoweis_left_chromium, 2015-09-25 21:36:29]

Patchset #2 (id:140001) has been deleted

[guoweis_left_chromium, 2015-09-25 21:38:39]

PTAL.

https://codereview.chromium.org/1335023002/diff/100001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1335023002/diff/100001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:51748: +    first transport signals the
OnCompleted event.
On 2015/09/24 13:29:43, juberti2 wrote:
> We might not have all the ciphers determined at the time the first transport
> goes to completed, i.e. if we have multiple transports.

I re-read the code. It has been changed recently and will trigger for each
transport.

[guoweis_left_chromium, 2015-09-25 21:39:24]

guoweis@chromium.org changed reviewers:
+ rkaplow@chromium.org

[guoweis_left_chromium, 2015-09-25 21:39:24]

rkaplow@chromium.org: Please review changes in histograms.xml

[rkaplow, 2015-09-25 21:44:42]

lgtm

[Ryan Sleevi, 2015-09-27 02:37:46]

I don't believe you need my LGTM, but a few suggestions.

https://codereview.chromium.org/1335023002/diff/160001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1335023002/diff/160001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:50393: +<histogram
name="WebRTC.PeerConnection.SrtpCrypto" enum="DTLS_SRTPCryptoSuite">
SrtpCryptoSuite?

https://codereview.chromium.org/1335023002/diff/160001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:50396: +    Counters on the type of SRTP
Cryptos used by WebRTC. This is collected
I'm not sure what "cryptos" is meant to convey, but it doesn't feel
grammatically correct.

Is this Crypto Suites? That seems to match your naming of the histogram.

https://codereview.chromium.org/1335023002/diff/160001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:50401: +<histogram
name="WebRTC.PeerConnection.SslCipher" enum="SSLCipherSuite">
SslCipherSuite?

https://codereview.chromium.org/1335023002/diff/160001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:50404: +    Counters on the type of SSL
Ciphers used by WebRTC. This is collected
ciphersuites

https://codereview.chromium.org/1335023002/diff/160001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:55066: +  <summary>DTLS/SRTP crypto
suites from the IANA registry</summary>
considering I've had to search for Justin's comments twice, since Google-foo
doesn't show the registry when searching for "SRTP crypto suite registry" (well,
it shows the SDP registrations but not the numeric forms Justin highlighted), it
may help to expand this comment to indicate the URL.

[pthatcher2, 2015-09-29 21:59:43]

https://codereview.chromium.org/1335023002/diff/160001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1335023002/diff/160001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:50393: +<histogram
name="WebRTC.PeerConnection.SrtpCrypto" enum="DTLS_SRTPCryptoSuite">
On 2015/09/27 02:37:46, Ryan Sleevi wrote:
> SrtpCryptoSuite?

+1

https://codereview.chromium.org/1335023002/diff/160001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:50396: +    Counters on the type of SRTP
Cryptos used by WebRTC. This is collected
On 2015/09/27 02:37:46, Ryan Sleevi wrote:
> I'm not sure what "cryptos" is meant to convey, but it doesn't feel
> grammatically correct.
> 
> Is this Crypto Suites? That seems to match your naming of the histogram. 

+1 to crypto suites (no need to capitalize)

https://codereview.chromium.org/1335023002/diff/160001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:50401: +<histogram
name="WebRTC.PeerConnection.SslCipher" enum="SSLCipherSuite">
On 2015/09/27 02:37:46, Ryan Sleevi wrote:
> SslCipherSuite?

+1

[guoweis_left_chromium, 2015-09-30 17:52:22]

guoweis@chromium.org changed reviewers:
- juberti@chromium.org

[guoweis_left_chromium, 2015-09-30 17:52:36]

guoweis@chromium.org changed reviewers:
- pthatcher@chromium.org

[guoweis_left_chromium, 2015-09-30 17:52:52]

guoweis@chromium.org changed reviewers:
- davidben@chromium.org, jbauch@webrtc.org, rsleevi@chromium.org

[guoweis_left_chromium, 2015-09-30 17:53:44]

guoweis@chromium.org changed reviewers:
+ juberti@chromium.org, pthatcher@chromium.org, rsleevi@chromium.org,
tommi@chromium.org

[guoweis_left_chromium, 2015-09-30 17:53:44]

PTAL.

[pthatcher2, 2015-09-30 17:59:25]

lgtm

[tommi (sloooow) - chröme, 2015-09-30 19:24:45]

lgtm

https://codereview.chromium.org/1335023002/diff/200001/content/renderer/media...
File content/renderer/media/webrtc/peer_connection_dependency_factory.cc
(right):

https://codereview.chromium.org/1335023002/diff/200001/content/renderer/media...
content/renderer/media/webrtc/peer_connection_dependency_factory.cc:384:
StartsWith(group_name, "Enabled", base::CompareCase::SENSITIVE)) {
nit: StartsWith() check first since you've already checked the FieldTrialList

[guoweis_left_chromium, 2015-10-01 21:57:58]

The CQ bit was checked by guoweis@chromium.org

[guoweis_left_chromium, 2015-10-01 21:57:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
rkaplow@chromium.org, rsleevi@chromium.org, pthatcher@chromium.org,
tommi@chromium.org
Link to the patchset: https://codereview.chromium.org/1335023002/#ps220001
(title: " ")

[commit-bot: I haz the power, 2015-10-01 21:59:56]

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

[guoweis_left_chromium, 2015-10-01 22:00:44]

The CQ bit was unchecked by guoweis@chromium.org

[guoweis_left_chromium, 2015-10-01 22:04:38]

The CQ bit was checked by guoweis@chromium.org

[guoweis_left_chromium, 2015-10-01 22:04:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
rkaplow@chromium.org, pthatcher@chromium.org, tommi@chromium.org,
rsleevi@chromium.org
Link to the patchset: https://codereview.chromium.org/1335023002/#ps240001
(title: " ")

[commit-bot: I haz the power, 2015-10-01 22:05:32]

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

[guoweis_left_chromium, 2015-10-01 22:19:35]

The CQ bit was checked by guoweis@chromium.org

[guoweis_left_chromium, 2015-10-01 22:19:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
rkaplow@chromium.org, pthatcher@chromium.org, tommi@chromium.org,
rsleevi@chromium.org
Link to the patchset: https://codereview.chromium.org/1335023002/#ps260001
(title: " ")

[commit-bot: I haz the power, 2015-10-01 22:19:50]

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

[commit-bot: I haz the power, 2015-10-01 22:35:19]

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

[commit-bot: I haz the power, 2015-10-01 22:35:21]

Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[guoweis2, 2015-10-02 00:31:19]

The CQ bit was checked by guoweis@google.com

[commit-bot: I haz the power, 2015-10-02 00:31:44]

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

[commit-bot: I haz the power, 2015-10-02 00:45:56]

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

[commit-bot: I haz the power, 2015-10-02 00:45:57]

Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[guoweis2, 2015-10-02 01:31:12]

The CQ bit was checked by guoweis@google.com

[commit-bot: I haz the power, 2015-10-02 01:31:31]

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

[commit-bot: I haz the power, 2015-10-02 02:10:06]

Committed patchset #7 (id:260001)

[commit-bot: I haz the power, 2015-10-02 02:10:54]

Patchset 7 (id:??) landed as
https://crrev.com/8ff81738a6f8b74e09966335be2849c04a8cbde1
Cr-Commit-Position: refs/heads/master@{#351936}