UMA stats to count RenderFrameProxyHosts

Add UMA stats which count the number of RenderFrameProxyHosts

BUG=542921
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/de8b027d315ecb3a372242c4791be5143123aade
Cr-Commit-Position: refs/heads/master@{#374855}

[ncarter (slow), 2016-02-10 22:00:35]

Description was changed from

==========
Add UMA stats which count the number of RenderFrameProxyHosts

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

to

==========
Add UMA stats which count the number of RenderFrameProxyHosts

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[ncarter (slow), 2016-02-10 22:04:08]

Description was changed from

==========
Add UMA stats which count the number of RenderFrameProxyHosts

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add UMA stats which count the number of RenderFrameProxyHosts

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[ncarter (slow), 2016-02-10 22:04:40]

nick@chromium.org changed reviewers:
+ creis@chromium.org, nasko@chromium.org

[Charlie Reis, 2016-02-10 22:37:51]

Nice.  Just a few small things below.

nit: Let's use 542921 as the bug number, since that mentions both the number of
proxies per BrowsingInstance (#2 in comment 0) and the number of proxies overall
(comment 4).

https://codereview.chromium.org/1688963002/diff/1/chrome/browser/site_details...
File chrome/browser/site_details_browsertest.cc (right):

https://codereview.chromium.org/1688963002/diff/1/chrome/browser/site_details...
chrome/browser/site_details_browsertest.cc:380:
HasOneSample(DependingOnPolicy(0, 0, 100)));
Looks like we have 15*(9-1) proxies in this case (120).  Perhaps we should make
the max 1000 for the histogram?

https://codereview.chromium.org/1688963002/diff/1/chrome/browser/site_details...
chrome/browser/site_details_browsertest.cc:438:
HasOneSample(DependingOnPolicy(0, 0, 75)));
No discrepancy expected here, right?

https://codereview.chromium.org/1688963002/diff/1/chrome/browser/site_details...
chrome/browser/site_details_browsertest.cc:1135:
HasOneSample(DependingOnPolicy(0, 0, 100)));
Let's add the per-BrowsingInstance count expectations as well.

https://codereview.chromium.org/1688963002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/render_frame_host_manager.cc (right):

https://codereview.chromium.org/1688963002/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_manager.cc:2340: return
proxy_hosts_.size();
I suppose this also includes RFPHs that aren't live, and thus aren't affecting
renderer memory stats.  That's an edge case, though, so we should probably
ignore it.

https://codereview.chromium.org/1688963002/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1688963002/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:46742: +    The count of
RenderFrameProxyHosts in each BrowsingInstance.  Recorded once
Is this once per BrowsingInstance per UMA ping?

[ncarter (slow), 2016-02-10 23:22:14]

all done

https://codereview.chromium.org/1688963002/diff/1/chrome/browser/site_details...
File chrome/browser/site_details_browsertest.cc (right):

https://codereview.chromium.org/1688963002/diff/1/chrome/browser/site_details...
chrome/browser/site_details_browsertest.cc:380:
HasOneSample(DependingOnPolicy(0, 0, 100)));
On 2016/02/10 22:37:52, Charlie Reis wrote:
> Looks like we have 15*(9-1) proxies in this case (120).  Perhaps we should
make
> the max 1000 for the histogram?

Increased both to 10000. The higher max doesn't actually cost us much fidelity
at the lower end, since the buckets are exponentially spaced.

https://codereview.chromium.org/1688963002/diff/1/chrome/browser/site_details...
chrome/browser/site_details_browsertest.cc:438:
HasOneSample(DependingOnPolicy(0, 0, 75)));
On 2016/02/10 22:37:51, Charlie Reis wrote:
> No discrepancy expected here, right?

They are the same input value, but the histograms were scaled differently, so
the discrepancy reflects the histogram bucketing.

I think using the same bucketing seems like a good idea for future analysis, so
I'm doing that.

https://codereview.chromium.org/1688963002/diff/1/chrome/browser/site_details...
chrome/browser/site_details_browsertest.cc:1135:
HasOneSample(DependingOnPolicy(0, 0, 100)));
On 2016/02/10 22:37:51, Charlie Reis wrote:
> Let's add the per-BrowsingInstance count expectations as well.

Done.

https://codereview.chromium.org/1688963002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/render_frame_host_manager.cc (right):

https://codereview.chromium.org/1688963002/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_manager.cc:2340: return
proxy_hosts_.size();
On 2016/02/10 22:37:52, Charlie Reis wrote:
> I suppose this also includes RFPHs that aren't live, and thus aren't affecting
> renderer memory stats.  That's an edge case, though, so we should probably
> ignore it.

Acknowledged.

https://codereview.chromium.org/1688963002/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1688963002/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:46742: +    The count of
RenderFrameProxyHosts in each BrowsingInstance.  Recorded once
On 2016/02/10 22:37:52, Charlie Reis wrote:
> Is this once per BrowsingInstance per UMA ping?

Yes. I've clarified this comment as well as others.

[ncarter (slow), 2016-02-10 23:22:29]

Description was changed from

==========
Add UMA stats which count the number of RenderFrameProxyHosts

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add UMA stats which count the number of RenderFrameProxyHosts

BUG=542921 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[Charlie Reis, 2016-02-10 23:46:31]

Thanks.  LGTM!

[ncarter (slow), 2016-02-10 23:49:06]

The CQ bit was checked by nick@chromium.org

[ncarter (slow), 2016-02-10 23:49:09]

The CQ bit was unchecked by nick@chromium.org

[ncarter (slow), 2016-02-10 23:50:15]

nick@chromium.org changed reviewers:
+ isherman@chromium.org, thestig@chromium.org
- nasko@chromium.org

[ncarter (slow), 2016-02-10 23:50:16]

+thestig for chrome/
+isherman for histograms.xml

[Lei Zhang, 2016-02-11 01:10:50]

lgtm

[Ilya Sherman, 2016-02-11 02:51:08]

histograms lgtm

[Ilya Sherman, 2016-02-11 02:51:15]

The CQ bit was checked by isherman@chromium.org

[commit-bot: I haz the power, 2016-02-11 02:52:08]

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

[commit-bot: I haz the power, 2016-02-11 04:29:40]

Description was changed from

==========
Add UMA stats which count the number of RenderFrameProxyHosts

BUG=542921 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add UMA stats which count the number of RenderFrameProxyHosts

BUG=542921 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-02-11 04:29:40]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-16 22:34:31]

Description was changed from

==========
Add UMA stats which count the number of RenderFrameProxyHosts

BUG=542921 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add UMA stats which count the number of RenderFrameProxyHosts

BUG=542921
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/de8b027d315ecb3a372242c4791be5143123aade
Cr-Commit-Position: refs/heads/master@{#374855}
==========

[commit-bot: I haz the power, 2016-02-16 22:34:32]

Patchset 3 (id:??) landed as
https://crrev.com/de8b027d315ecb3a372242c4791be5143123aade
Cr-Commit-Position: refs/heads/master@{#374855}