Test divisor for zero before generating statistic

Test divisor for zero before generating statistic

r=rch
BUG=377194
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272791

[jar (doing other things), 2014-05-24 20:52:53]

I just mapped the overflow into the highest bucket size... which is about as
good as infinite (since it goes into that last bucket).

[Ryan Hamilton, 2014-05-24 21:34:20]

lgtm

I wonder if we should simply skip the histograms if there is a min_rtt of 0?

https://codereview.chromium.org/299423002/diff/1/net/quic/quic_client_session.cc
File net/quic/quic_client_session.cc (right):

https://codereview.chromium.org/299423002/diff/1/net/quic/quic_client_session...
net/quic/quic_client_session.cc:247: uint64 reordering = (stats.min_rtt_us == 0)
? GG_UINT64_C(100)
nit: no need for GG_UINT64_C() around the 100 here. I needed it below to ensure
that the multiplication was 64 bit, so it didn't overflow.

[jar (doing other things), 2014-05-24 21:40:49]

On 2014/05/24 21:34:20, Ryan Hamilton wrote:
> lgtm
> 
> I wonder if we should simply skip the histograms if there is a min_rtt of 0?
> 

It is always better to avoid "discarding" samples, and unwittingly adding bias.

We already have the second histogram for "large" rtt values.


>
https://codereview.chromium.org/299423002/diff/1/net/quic/quic_client_session.cc
> File net/quic/quic_client_session.cc (right):
> 
>
https://codereview.chromium.org/299423002/diff/1/net/quic/quic_client_session...
> net/quic/quic_client_session.cc:247: uint64 reordering = (stats.min_rtt_us ==
0)
> ? GG_UINT64_C(100)
> nit: no need for GG_UINT64_C() around the 100 here. I needed it below to
ensure
> that the multiplication was 64 bit, so it didn't overflow.

I felt really confident it would be ok.... but the ternary operator might be
happier with identically sized options on some branches (on some? compilers?)...
so I was being cautious to increase the chance that it wouldn't block a landing.

[commit-bot: I haz the power, 2014-05-24 23:19:57]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jar@chromium.org/299423002/1

[commit-bot: I haz the power, 2014-05-25 01:35:28]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-25 01:56:54]

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

[commit-bot: I haz the power, 2014-05-25 01:56:54]

Try jobs failed on following builders:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)

[jar (doing other things), 2014-05-25 05:52:19]

The CQ bit was checked by jar@chromium.org

[commit-bot: I haz the power, 2014-05-25 05:52:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jar@chromium.org/299423002/1

[commit-bot: I haz the power, 2014-05-25 07:44:37]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)

[jar (doing other things), 2014-05-25 07:58:51]

On 2014/05/25 07:44:37, I haz the power (commit-bot) wrote:
> FYI, CQ is re-trying this CL (attempt #1).
> Please consider checking whether the failures are real,
> and report flakes to mailto:chrome-troopers@google.com.
> The failing builders are:
>   android_aosp on tryserver.chromium
>
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)

I think this is a bot problem, as the patch applies nicely on other platforms.  

An attempt to rebase provided no changes.

The cl is quite simple... and if it compiles, it should only help.  The problem
with applying the patch seems unrelated.

I'm going to land with notry=true, as this is a top crasher, and it should
really by part of our next canary push.

[jar (doing other things), 2014-05-25 07:58:56]

The CQ bit was checked by jar@chromium.org

[commit-bot: I haz the power, 2014-05-25 07:59:13]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jar@chromium.org/299423002/20001

[jar (doing other things), 2014-05-25 08:29:35]

The CQ bit was unchecked by jar@chromium.org

[jar (doing other things), 2014-05-25 08:29:36]

The CQ bit was checked by jar@chromium.org

[commit-bot: I haz the power, 2014-05-25 08:29:40]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jar@chromium.org/299423002/20001

[commit-bot: I haz the power, 2014-05-26 04:31:15]

Change committed as 272791