Add some unit tests to ReceiveStatsticsProxy.

Add some unit tests to ReceiveStatsticsProxy.

Add logging of some histogram stats at the end of a call.

BUG=none

Review-Url: https://codereview.webrtc.org/2873303002
Cr-Commit-Position: refs/heads/master@{#18099}
Committed: https://chromium.googlesource.com/external/webrtc/+/2077f2f9fcea5c514619fa5c16994cd6e80b3595

[åsapersson, 2017-05-11 06:53:10]

Description was changed from

==========
Add some unit tests to ReceiveStatsticsProxy.

Add logging of some histogram stats at the end of a call.

BUG=none
==========

to

==========
Add some unit tests to ReceiveStatsticsProxy.

Add logging of some histogram stats at the end of a call.

BUG=none
==========

[åsapersson, 2017-05-11 06:53:10]

asapersson@webrtc.org changed reviewers:
+ sprang@webrtc.org

[åsapersson, 2017-05-11 06:56:33]

Patchset #1 (id:1) has been deleted

[sprang_webrtc, 2017-05-11 09:17:57]

Great that you're extending test coverage!
Just a few questions..

https://codereview.webrtc.org/2873303002/diff/40001/webrtc/video/receive_stat...
File webrtc/video/receive_statistics_proxy.cc (right):

https://codereview.webrtc.org/2873303002/diff/40001/webrtc/video/receive_stat...
webrtc/video/receive_statistics_proxy.cc:108: << fraction_lost;
Is there any way we could add func to RTC_HISTOGRAM_PERCENTAGE and the other
macros so they do this logging instead? Feels like a pain to have to duplicate
these histogram names everywhere...

https://codereview.webrtc.org/2873303002/diff/40001/webrtc/video/receive_stat...
File webrtc/video/receive_statistics_proxy_unittest.cc (right):

https://codereview.webrtc.org/2873303002/diff/40001/webrtc/video/receive_stat...
webrtc/video/receive_statistics_proxy_unittest.cc:563: EXPECT_EQ(0,
metrics::NumSamples("WebRTC.Video.RenderSqrtPixelsPerSecond"));
Should WebRTC.Video.Received(Width|Height)InPixels be here too?

https://codereview.webrtc.org/2873303002/diff/40001/webrtc/video/receive_stat...
webrtc/video/receive_statistics_proxy_unittest.cc:587:
fake_clock_.AdvanceTimeMilliseconds(metrics::kMinRunTimeInSeconds * 1000 - 1);
nit: add parenthesis around (metrics::kMinRunTimeInSeconds * 1000) for clarity

[åsapersson, 2017-05-11 09:50:01]

https://codereview.webrtc.org/2873303002/diff/40001/webrtc/video/receive_stat...
File webrtc/video/receive_statistics_proxy.cc (right):

https://codereview.webrtc.org/2873303002/diff/40001/webrtc/video/receive_stat...
webrtc/video/receive_statistics_proxy.cc:108: << fraction_lost;
On 2017/05/11 09:17:57, sprang_webrtc wrote:
> Is there any way we could add func to RTC_HISTOGRAM_PERCENTAGE and the other
> macros so they do this logging instead? Feels like a pain to have to duplicate
> these histogram names everywhere...

There used to be a RTC_LOGGED_* macro but it has been removed:
https://codereview.webrtc.org/1788783002

https://codereview.webrtc.org/2873303002/diff/40001/webrtc/video/receive_stat...
File webrtc/video/receive_statistics_proxy_unittest.cc (right):

https://codereview.webrtc.org/2873303002/diff/40001/webrtc/video/receive_stat...
webrtc/video/receive_statistics_proxy_unittest.cc:563: EXPECT_EQ(0,
metrics::NumSamples("WebRTC.Video.RenderSqrtPixelsPerSecond"));
On 2017/05/11 09:17:57, sprang_webrtc wrote:
> Should WebRTC.Video.Received(Width|Height)InPixels be here too?

On line 560, 561?

https://codereview.webrtc.org/2873303002/diff/40001/webrtc/video/receive_stat...
webrtc/video/receive_statistics_proxy_unittest.cc:587:
fake_clock_.AdvanceTimeMilliseconds(metrics::kMinRunTimeInSeconds * 1000 - 1);
On 2017/05/11 09:17:57, sprang_webrtc wrote:
> nit: add parenthesis around (metrics::kMinRunTimeInSeconds * 1000) for clarity

Done.

[sprang_webrtc, 2017-05-11 10:54:58]

lgtm

https://codereview.webrtc.org/2873303002/diff/40001/webrtc/video/receive_stat...
File webrtc/video/receive_statistics_proxy.cc (right):

https://codereview.webrtc.org/2873303002/diff/40001/webrtc/video/receive_stat...
webrtc/video/receive_statistics_proxy.cc:108: << fraction_lost;
On 2017/05/11 09:50:00, åsapersson wrote:
> On 2017/05/11 09:17:57, sprang_webrtc wrote:
> > Is there any way we could add func to RTC_HISTOGRAM_PERCENTAGE and the other
> > macros so they do this logging instead? Feels like a pain to have to
duplicate
> > these histogram names everywhere...
> 
> There used to be a RTC_LOGGED_* macro but it has been removed:
> https://codereview.webrtc.org/1788783002

Acknowledged.

https://codereview.webrtc.org/2873303002/diff/40001/webrtc/video/receive_stat...
File webrtc/video/receive_statistics_proxy_unittest.cc (right):

https://codereview.webrtc.org/2873303002/diff/40001/webrtc/video/receive_stat...
webrtc/video/receive_statistics_proxy_unittest.cc:563: EXPECT_EQ(0,
metrics::NumSamples("WebRTC.Video.RenderSqrtPixelsPerSecond"));
On 2017/05/11 09:50:01, åsapersson wrote:
> On 2017/05/11 09:17:57, sprang_webrtc wrote:
> > Should WebRTC.Video.Received(Width|Height)InPixels be here too?
> 
> On line 560, 561?

Right, read too fast. Sorry!

[åsapersson, 2017-05-11 11:03:08]

The CQ bit was checked by asapersson@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-11 11:03:14]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-05-11 12:26:44]

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

[commit-bot: I haz the power, 2017-05-11 12:26:45]

Dry run: This issue passed the CQ dry run.

[åsapersson, 2017-05-11 12:35:01]

The CQ bit was checked by asapersson@webrtc.org

[commit-bot: I haz the power, 2017-05-11 12:35:06]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-05-11 12:37:36]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1494506101876560,
"parent_rev": "712338eed24a8d68699b331acad12edf340ac929", "commit_rev":
"2077f2f9fcea5c514619fa5c16994cd6e80b3595"}

[commit-bot: I haz the power, 2017-05-11 12:37:39]

Description was changed from

==========
Add some unit tests to ReceiveStatsticsProxy.

Add logging of some histogram stats at the end of a call.

BUG=none
==========

to

==========
Add some unit tests to ReceiveStatsticsProxy.

Add logging of some histogram stats at the end of a call.

BUG=none

Review-Url: https://codereview.webrtc.org/2873303002
Cr-Commit-Position: refs/heads/master@{#18099}
Committed:
https://chromium.googlesource.com/external/webrtc/+/2077f2f9fcea5c514619fa5c1...
==========

[commit-bot: I haz the power, 2017-05-11 12:37:40]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/external/webrtc/+/2077f2f9fcea5c514619fa5c1...