NetConnectivity - Collect stats for TCP/UDP network

NetConnectivity - Collect stats for TCP/UDP network
connectivity when there is no proxy server. This will
allow us to see if there is a difference in connectivity
characteristics if proxy server is not involved.

"NetConnectivity.<protocol>.NoProxy.Success.<port>.<load_size>.RTT"
collects the RTT for RTT when there is no proxy.

"NetConnectivity.<protocol>.NoProxy.Status.<port>.<load_size>"
collects the status of connectivity when there is no proxy.

Exisiting histograms are not impacted.

R=jar,eroman
TEST=browser unit tests.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=136241

[ramant (doing other things), 2012-04-28 01:54:51]

Hi Jim and Eric,
   We perform TCP/UDP connectivity tests after UMA upload has succeeded. UMA is
not tied to a profile and thus we use system_request_context to fetch the URLs. 

  We wanted to use profile specific ProxyService, but because Network
connectivity tests are performed after UMA upload has succeeded (and UMA is not
associated with a profile) and because getting the DefaultProfile is deprecated,
we thought we would use SystemProxyService to find if there is a proxy or not.

  Network connectivity tests send data to the server using TCPClientSocket and
UDPClientSocket. During discussions with willchan, rsleevi and jar, we thought
the following new histograms may give some hint on if there is a co-relation
between proxy and connectivity (and RTT).

  This CL will not be testing if there is a transparent proxy or not.

  Added the following new histograms to collect status and RTT for TCP/UDP
connectivity tests if there is no proxy server.

   "NetConnectivity.<protocol>.NoProxy.Success.<port>.<load_size>.RTT"
   "NetConnectivity.<protocol>.NoProxy.Status.<port>.<load_size>"

  Many thanks to Eric for taking a look at the Proxy related changes earlier.

  Would appreciate your comments.

thanks,
raman

[Ryan Sleevi, 2012-04-28 02:06:32]

Hi Raman,

Just some drive-by comments. One other thing I would suggest would be changing
the bool from has_no_proxy to has_proxy. This makes it easier to read in the
case of double-negatives (has_no_proxy == false means has_proxy). Just a nit :)

http://codereview.chromium.org/10206035/diff/11001/chrome/browser/net/network...
File chrome/browser/net/network_stats.cc (right):

http://codereview.chromium.org/10206035/diff/11001/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:469: rtt_histogram_name->clear();
random drive by: Since much of our error handling works via early-return style,
it seems like it might make more sense to check for failure in the if and then
let the else handle success. Otherwise, you have to continue reading and count
the columns to figure out
1) If there's error handling
2) What it does

eg:

if (result != net::OK) {
  rtt_histogram_name->clear();
} else {
  if (!has_no_proxy_server) {
    rtt_no_proxy_server_hisogram_name->clear();
  } else {
    ...
  }
}

I don't believe this will significantly impact branch mispredicts during runtime
(depending on how the compiler codegens), so if this is very perf-sensitive, the
readability might not be worth it.

http://codereview.chromium.org/10206035/diff/11001/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:676: kPorts[histogram_port_]);
Would it make more sense to check for HTTPS here?

The reason I mention it is that HTTPS will give you an indicator as to whether
or not the proxy server supports CONNECT, which is effectively how outbound
connections can be mediated by the proxy.

http://codereview.chromium.org/10206035/diff/11001/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:704: UDPStatsClient* small_udp_stats = new
UDPStatsClient();
Might be worth mentioning either here or in network_stats.h that the
(UDP/TCP)StatsClient will delete themselves when Finish() is called.

I had to check to find the "delete this" or if they were refcounted.

[ramant (doing other things), 2012-04-30 17:17:00]

Made all the changes you had suggested Ryan.

thanks very much for your comments/suggestions,
raman

http://codereview.chromium.org/10206035/diff/11001/chrome/browser/net/network...
File chrome/browser/net/network_stats.cc (right):

http://codereview.chromium.org/10206035/diff/11001/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:469: rtt_histogram_name->clear();
On 2012/04/28 02:06:32, Ryan Sleevi wrote:
> random drive by: Since much of our error handling works via early-return
style,
> it seems like it might make more sense to check for failure in the if and then
> let the else handle success. Otherwise, you have to continue reading and count
> the columns to figure out
> 1) If there's error handling
> 2) What it does
> 
> eg:
> 
> if (result != net::OK) {
>   rtt_histogram_name->clear();
> } else {
>   if (!has_no_proxy_server) {
>     rtt_no_proxy_server_hisogram_name->clear();
>   } else {
>     ...
>   }
> }
> 
> I don't believe this will significantly impact branch mispredicts during
runtime
> (depending on how the compiler codegens), so if this is very perf-sensitive,
the
> readability might not be worth it.

Done.

http://codereview.chromium.org/10206035/diff/11001/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:676: kPorts[histogram_port_]);
On 2012/04/28 02:06:32, Ryan Sleevi wrote:
> Would it make more sense to check for HTTPS here?
> 
> The reason I mention it is that HTTPS will give you an indicator as to whether
> or not the proxy server supports CONNECT, which is effectively how outbound
> connections can be mediated by the proxy.

Done.

http://codereview.chromium.org/10206035/diff/11001/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:704: UDPStatsClient* small_udp_stats = new
UDPStatsClient();
On 2012/04/28 02:06:32, Ryan Sleevi wrote:
> Might be worth mentioning either here or in network_stats.h that the
> (UDP/TCP)StatsClient will delete themselves when Finish() is called.
> 
> I had to check to find the "delete this" or if they were refcounted.

Done.

[ramant (doing other things), 2012-05-02 22:08:33]

Please hold of reviewing this change. Will update this CL with the following
changes from multiple packets CL.

http://src.chromium.org/viewvc/chrome?view=rev&revision=134976

thanks,
raman

[ramant (doing other things), 2012-05-03 15:13:19]

Hi Jim
  Merged packet loss changes into this CL. Eric said he would review the Proxy
related changes. Would appreciate if you could take a look at these changes.

thanks,
raman

[jar (doing other things), 2012-05-03 16:41:34]

Didn't you land the change that randomizes the tests, and does the packet-loss
stats gathering?  This proxy test seems pretty separable.

Comments below.  I wasn't clear what you were after (or perhaps I misread the
code... which I admittedly didn't yet read that carfully).

http://codereview.chromium.org/10206035/diff/31003/chrome/browser/net/network...
File chrome/browser/net/network_stats.cc (right):

http://codereview.chromium.org/10206035/diff/31003/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:522: std::string*
packet_loss_no_proxy_histogram_name) {
If you put ".NoProxy" at the end of the name, construction of these other names
would be much less complex (you might not need to even calculate them here).

http://codereview.chromium.org/10206035/diff/31003/chrome/browser/net/network...
File chrome/browser/net/network_stats.h (right):

http://codereview.chromium.org/10206035/diff/31003/chrome/browser/net/network...
chrome/browser/net/network_stats.h:244: //
"NetConnectivity.<protocol>.NoProxy.Status.<port>.<load_size>" as no proxy
I expect that you'll be most interesting in comparing otherwise identical stats,
with and without the proxy.  As a result, it would be "nice" if they
alphabetized next to each other.  To make that happen, I'd suggest putting
"NoProxy" at the absolute end of the new name.
NetConnectivity.<protocol>.Status.<port>.load_size>.NoProxy

http://codereview.chromium.org/10206035/diff/31003/chrome/browser/net/network...
chrome/browser/net/network_stats.h:374: class ProxyClient {
I'm surprised to see this much work.  Is the intent to run the tests "through"
the proxy?

I was expecting to see proxy detection, and then a see the histograms that are
currently emitted split into with/without a proxy (tangentially) present
(without changing the "test").

[ramant (doing other things), 2012-05-04 03:53:11]

Hi Jim,
  ProxyDetector determines if there is a proxy server or not (it relies on
ProxyService to find if there is a proxy server or not). The resolution could be
asynchronous. After the resolution is done, we start the network connectivity
tests.

  Made all the other changes you had suggested.

thanks,
raman

http://codereview.chromium.org/10206035/diff/31003/chrome/browser/net/network...
File chrome/browser/net/network_stats.cc (right):

http://codereview.chromium.org/10206035/diff/31003/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:522: std::string*
packet_loss_no_proxy_histogram_name) {
On 2012/05/03 16:41:34, jar wrote:
> If you put ".NoProxy" at the end of the name, construction of these other
names
> would be much less complex (you might not need to even calculate them here).

Done.

http://codereview.chromium.org/10206035/diff/31003/chrome/browser/net/network...
File chrome/browser/net/network_stats.h (right):

http://codereview.chromium.org/10206035/diff/31003/chrome/browser/net/network...
chrome/browser/net/network_stats.h:244: //
"NetConnectivity.<protocol>.NoProxy.Status.<port>.<load_size>" as no proxy
On 2012/05/03 16:41:34, jar wrote:
> I expect that you'll be most interesting in comparing otherwise identical
stats,
> with and without the proxy.  As a result, it would be "nice" if they
> alphabetized next to each other.  To make that happen, I'd suggest putting
> "NoProxy" at the absolute end of the new name.
> NetConnectivity.<protocol>.Status.<port>.load_size>.NoProxy

Done.

http://codereview.chromium.org/10206035/diff/31003/chrome/browser/net/network...
chrome/browser/net/network_stats.h:374: class ProxyClient {
On 2012/05/03 16:41:34, jar wrote:
> I'm surprised to see this much work.  Is the intent to run the tests "through"
> the proxy?

No. This code to detect the proxy runs before we run the test.

Renamed ProxyClient to ProxyDetector. Should we rename to a different name?
thanks.

> 
> I was expecting to see proxy detection, and then a see the histograms that are
> currently emitted split into with/without a proxy (tangentially) present
> (without changing the "test").

[ramant (doing other things), 2012-05-04 21:31:51]

Hi Jim,
  Made the changes we have discussed this morning (use the for loop to collect
stats for with proxy and without proxy).

[eroman, 2012-05-04 23:08:48]

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
File chrome/browser/net/network_stats.cc (right):

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:745: ProxyDetector::~ProxyDetector() {
When is ProxyDetector deleted? Note that if it is deleted while the
ResolveProxy() request is still in progress, that will lead to a use-after-free
when the callback is invoked.

I suggest adding one of two safety nets. Either:
  (a) assert to make sure there is no in-progress ProxyResolve request
  (b) Explicitly cancel the ProxyResolve request during deletion, if one is
outstanding. (ResolveProxy can take a request handle, and you can use this to
cancel it).

I recommend approach (a), and just making sure the lifetime is well understood.

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:750: "https://%s:%d",
network_stats_server_.c_str(), kPorts[histogram_port_]);
Assembling URLs in this fashion is generally not safe, since if the hostname is
an IPv6 literal concatenating ':' has different meaning.

There are several possible solutions:
  (a) Use HostPortPair::ToString() to stringize the host + port. In fact, I
suggest making the input to ProxyDetector be a HostPairPair regardless.

  (b) Use GURL::ReplaceComponents() to smuggle in a host and port into a URL.

The simplest strategy is probably (a), so I recommend that.

Lastly, is it intentional that the scheme be "https://" rather than "http://"?
Either is fine, just want to make sure this has been thought through (since PAC
scripts will sometimes give different results for HTTPS vs HTTP).

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:767: bool has_proxy_server =
(proxy_info_.proxy_server().is_valid() &&
One extra check to add is into that list is:
   result == net::OK

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:769: StartNetworkStatsTest(
[optional] I suggest de-coupling this from ProxyDetector. Instead, have
ProxyDetector take a Callback<boo> parameter. The caller can partially bind a
Callback to StartNetworkStatsTest with all the other parmaters already fixed
(hence don't need to mirror them in ProxyDetector).

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
File chrome/browser/net/network_stats.h (right):

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
chrome/browser/net/network_stats.h:367: NetworkStats::HistogramPortSelector
histogram_port,
nit: align

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
chrome/browser/net/network_stats.h:369: virtual ~ProxyDetector();
nit: why virtual?

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
chrome/browser/net/network_stats.h:376: void OnResolveProxyComplete(int result);
This can be private, since it is only an implementation detail to bind a
completion callback.

[ramant (doing other things), 2012-05-07 23:35:46]

Hi Eric,
  Made all the changes you had suggested. Would appreciate if you could take
another look at this CL.

thanks much,
raman

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
File chrome/browser/net/network_stats.cc (right):

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:745: ProxyDetector::~ProxyDetector() {
On 2012/05/04 23:08:48, eroman wrote:
> When is ProxyDetector deleted? Note that if it is deleted while the
> ResolveProxy() request is still in progress, that will lead to a
use-after-free
> when the callback is invoked.
> 
> I suggest adding one of two safety nets. Either:
>   (a) assert to make sure there is no in-progress ProxyResolve request
>   (b) Explicitly cancel the ProxyResolve request during deletion, if one is
> outstanding. (ResolveProxy can take a request handle, and you can use this to
> cancel it).
> 
> I recommend approach (a), and just making sure the lifetime is well
understood.

Implemented option a). Thanks for the above comment.

Done.

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:750: "https://%s:%d",
network_stats_server_.c_str(), kPorts[histogram_port_]);
On 2012/05/04 23:08:48, eroman wrote:
> Assembling URLs in this fashion is generally not safe, since if the hostname
is
> an IPv6 literal concatenating ':' has different meaning.
> 
> There are several possible solutions:
>   (a) Use HostPortPair::ToString() to stringize the host + port. In fact, I
> suggest making the input to ProxyDetector be a HostPairPair regardless.
> 
>   (b) Use GURL::ReplaceComponents() to smuggle in a host and port into a URL.
> 
> The simplest strategy is probably (a), so I recommend that.
> 
> Lastly, is it intentional that the scheme be "https://" rather than "http://"?
> Either is fine, just want to make sure this has been thought through (since
PAC
> scripts will sometimes give different results for HTTPS vs HTTP).

Implemented option a). Thanks much for the suggestion.

Used https per following comment from rsleevi:
	
Ryan Sleevi 2012/04/28 02:06:32
  Would it make more sense to check for HTTPS here?

  The reason I mention it is that HTTPS will give you an indicator as to whether
or not the proxy server supports CONNECT, which is effectively how outbound
connections can be mediated by the proxy.

Should we use http?

Done.

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:767: bool has_proxy_server =
(proxy_info_.proxy_server().is_valid() &&
On 2012/05/04 23:08:48, eroman wrote:
> One extra check to add is into that list is:
>    result == net::OK

Done.

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:769: StartNetworkStatsTest(
On 2012/05/04 23:08:48, eroman wrote:
> [optional] I suggest de-coupling this from ProxyDetector. Instead, have
> ProxyDetector take a Callback<boo> parameter. The caller can partially bind a
> Callback to StartNetworkStatsTest with all the other parmaters already fixed
> (hence don't need to mirror them in ProxyDetector).

Done.

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
File chrome/browser/net/network_stats.h (right):

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
chrome/browser/net/network_stats.h:367: NetworkStats::HistogramPortSelector
histogram_port,
On 2012/05/04 23:08:48, eroman wrote:
> nit: align

Done.

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
chrome/browser/net/network_stats.h:369: virtual ~ProxyDetector();
On 2012/05/04 23:08:48, eroman wrote:
> nit: why virtual?

Done.

http://codereview.chromium.org/10206035/diff/53002/chrome/browser/net/network...
chrome/browser/net/network_stats.h:376: void OnResolveProxyComplete(int result);
On 2012/05/04 23:08:48, eroman wrote:
> This can be private, since it is only an implementation detail to bind a
> completion callback.

Done.

[eroman, 2012-05-08 01:02:27]

the proxy detector changes look good after following comments.

(I didn't review the rest, looked like mostly just plumbing the bool throughout)

http://codereview.chromium.org/10206035/diff/51005/chrome/browser/net/network...
File chrome/browser/net/network_stats.cc (right):

http://codereview.chromium.org/10206035/diff/51005/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:751: void ProxyDetector::ResolveProxy() {
[optional] nit: I suggest calling this StartResolveProxy(), to emphasize that it
has deferred completion.

http://codereview.chromium.org/10206035/diff/51005/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:765: if (rv == net::OK)
this should actually be:
  rv != net::ERR_IO_PENDING

Basically we want to catch all *synchronous* completions, including errors.

http://codereview.chromium.org/10206035/diff/51005/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:770: DCHECK(result == net::OK);
Delete this. (Errors are possible).

http://codereview.chromium.org/10206035/diff/51005/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:773: bool has_proxy_server =
(proxy_info_.proxy_server().is_valid() &&
Please also add result == net::OK at the start.

http://codereview.chromium.org/10206035/diff/51005/chrome/browser/net/network...
File chrome/browser/net/network_stats.h (right):

http://codereview.chromium.org/10206035/diff/51005/chrome/browser/net/network...
chrome/browser/net/network_stats.h:365: typedef base::Callback<void(bool)>
ProxyDetectorCallback;
[optional] nit: since this is already enclosed in the "ProxyDetector" class, the
"ProxyDetector" prefix is a bit redundant. I suggest shortening it to something
like ResultCallback or OnResolvedCallback

[ramant (doing other things), 2012-05-08 05:28:18]

Thanks much Eric. Made all the changes.

-raman

http://codereview.chromium.org/10206035/diff/51005/chrome/browser/net/network...
File chrome/browser/net/network_stats.cc (right):

http://codereview.chromium.org/10206035/diff/51005/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:751: void ProxyDetector::ResolveProxy() {
On 2012/05/08 01:02:27, eroman wrote:
> [optional] nit: I suggest calling this StartResolveProxy(), to emphasize that
it
> has deferred completion.

Done.

http://codereview.chromium.org/10206035/diff/51005/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:765: if (rv == net::OK)
On 2012/05/08 01:02:27, eroman wrote:
> this should actually be:
>   rv != net::ERR_IO_PENDING
> 
> Basically we want to catch all *synchronous* completions, including errors.

Done.

http://codereview.chromium.org/10206035/diff/51005/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:770: DCHECK(result == net::OK);
On 2012/05/08 01:02:27, eroman wrote:
> Delete this. (Errors are possible).

Done.

http://codereview.chromium.org/10206035/diff/51005/chrome/browser/net/network...
chrome/browser/net/network_stats.cc:773: bool has_proxy_server =
(proxy_info_.proxy_server().is_valid() &&
On 2012/05/08 01:02:27, eroman wrote:
> Please also add result == net::OK at the start.

Done.

http://codereview.chromium.org/10206035/diff/51005/chrome/browser/net/network...
File chrome/browser/net/network_stats.h (right):

http://codereview.chromium.org/10206035/diff/51005/chrome/browser/net/network...
chrome/browser/net/network_stats.h:365: typedef base::Callback<void(bool)>
ProxyDetectorCallback;
On 2012/05/08 01:02:27, eroman wrote:
> [optional] nit: since this is already enclosed in the "ProxyDetector" class,
the
> "ProxyDetector" prefix is a bit redundant. I suggest shortening it to
something
> like ResultCallback or OnResolvedCallback

Done.

[eroman, 2012-05-08 06:12:49]

lgtm

[jar (doing other things), 2012-05-10 00:02:01]

lgtm

https://chromiumcodereview.appspot.com/10206035/diff/46011/chrome/browser/net...
File chrome/browser/net/network_stats.cc (right):

https://chromiumcodereview.appspot.com/10206035/diff/46011/chrome/browser/net...
chrome/browser/net/network_stats.cc:583: size_t histogram_count =
has_proxy_server_ ? 1 : 2;
nit: add comment:
// If we are running without a proxy, we'll generate 2 distinct histograms in
each case, one will have the ".NoProxy" suffix.

https://chromiumcodereview.appspot.com/10206035/diff/46011/chrome/browser/net...
chrome/browser/net/network_stats.cc:595: continue;
nit: Add comment: Packet loss histograms don't measure times or status.

https://chromiumcodereview.appspot.com/10206035/diff/46011/chrome/browser/net...
chrome/browser/net/network_stats.cc:772: callback_.Run(has_proxy_server);
As per your suggestion: Perchance this should be run via a post task.  It is
always hard to tell what state is held etc., and it is not perf critical (other
than the fact that you don't want to slow anything else down).

https://chromiumcodereview.appspot.com/10206035/diff/46011/chrome/browser/net...
chrome/browser/net/network_stats.cc:774: // TODO(rtenneti): Will we leak if
ProxtResolve is cancelled (or proxy
nit: typo: ProxtResolve --> ProxyResolve

[ramant (doing other things), 2012-05-10 02:37:34]

Hi Jim,
  Made all the changes we had discussed.

Thanks very much jar, eroman and sleevi for the code review,
raman

https://chromiumcodereview.appspot.com/10206035/diff/46011/chrome/browser/net...
File chrome/browser/net/network_stats.cc (right):

https://chromiumcodereview.appspot.com/10206035/diff/46011/chrome/browser/net...
chrome/browser/net/network_stats.cc:583: size_t histogram_count =
has_proxy_server_ ? 1 : 2;
On 2012/05/10 00:02:02, jar wrote:
> nit: add comment:
> // If we are running without a proxy, we'll generate 2 distinct histograms in
> each case, one will have the ".NoProxy" suffix.

Done.

https://chromiumcodereview.appspot.com/10206035/diff/46011/chrome/browser/net...
chrome/browser/net/network_stats.cc:595: continue;
On 2012/05/10 00:02:02, jar wrote:
> nit: Add comment: Packet loss histograms don't measure times or status.

Done.

https://chromiumcodereview.appspot.com/10206035/diff/46011/chrome/browser/net...
chrome/browser/net/network_stats.cc:772: callback_.Run(has_proxy_server);
On 2012/05/10 00:02:02, jar wrote:
> As per your suggestion: Perchance this should be run via a post task.  It is
> always hard to tell what state is held etc., and it is not perf critical
(other
> than the fact that you don't want to slow anything else down).

Done.

https://chromiumcodereview.appspot.com/10206035/diff/46011/chrome/browser/net...
chrome/browser/net/network_stats.cc:774: // TODO(rtenneti): Will we leak if
ProxtResolve is cancelled (or proxy
On 2012/05/10 00:02:02, jar wrote:
> nit: typo: ProxtResolve --> ProxyResolve

Done.

[commit-bot: I haz the power, 2012-05-10 02:37:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtenneti@chromium.org/10206035/41012

[commit-bot: I haz the power, 2012-05-10 02:59:13]

Try job failure for 10206035-41012 (retry) on linux_chromeos for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2012-05-10 03:10:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtenneti@chromium.org/10206035/41012

[commit-bot: I haz the power, 2012-05-10 04:26:36]

Change committed as 136241