Add more UMAs for Instance ID

Add more UMAs for Instance ID

BUG=477084
Committed: https://crrev.com/c15821554cb1f1e62fa8e6ba30e9942c17324402
Cr-Commit-Position: refs/heads/master@{#332937}

[jianli, 2015-06-02 19:04:03]

jianli@chromium.org changed reviewers:
+ asvitkine@chromium.org, fgorski@chromium.org

[fgorski, 2015-06-03 23:12:07]

LGTM

https://codereview.chromium.org/1167753002/diff/1/google_apis/gcm/engine/gcm_...
File google_apis/gcm/engine/gcm_store_impl.cc (right):

https://codereview.chromium.org/1167753002/diff/1/google_apis/gcm/engine/gcm_...
google_apis/gcm/engine/gcm_store_impl.cc:345: iter !=
result->registrations.end();
nit: indentatoin

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

https://codereview.chromium.org/1167753002/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:13725: +    the request is retried
multiple times, the length of time is counted for the
Please clarify the comment better (Fix the GetTokenXXX as well as GCM.* comments
for CompleteTime)

[jianli, 2015-06-03 23:31:17]

https://codereview.chromium.org/1167753002/diff/1/google_apis/gcm/engine/gcm_...
File google_apis/gcm/engine/gcm_store_impl.cc (right):

https://codereview.chromium.org/1167753002/diff/1/google_apis/gcm/engine/gcm_...
google_apis/gcm/engine/gcm_store_impl.cc:345: iter !=
result->registrations.end();
On 2015/06/03 23:12:07, fgorski wrote:
> nit: indentatoin

Done.

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

https://codereview.chromium.org/1167753002/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:13725: +    the request is retried
multiple times, the length of time is counted for the
On 2015/06/03 23:12:07, fgorski wrote:
> Please clarify the comment better (Fix the GetTokenXXX as well as GCM.*
comments
> for CompleteTime)

Per the offline talk, this comment looks fine.

[Alexei Svitkine (slow), 2015-06-04 18:59:33]

https://codereview.chromium.org/1167753002/diff/10001/google_apis/gcm/engine/...
File google_apis/gcm/engine/gcm_store_impl.cc (right):

https://codereview.chromium.org/1167753002/diff/10001/google_apis/gcm/engine/...
google_apis/gcm/engine/gcm_store_impl.cc:346: ++iter) {
Nit: Use C++ for loop:

for (const auto& entry : result->registrations) {
  if (StartsWithASCII(entry.first, ...
}

https://codereview.chromium.org/1167753002/diff/10001/google_apis/gcm/engine/...
File google_apis/gcm/engine/instance_id_get_token_request_handler.cc (right):

https://codereview.chromium.org/1167753002/diff/10001/google_apis/gcm/engine/...
google_apis/gcm/engine/instance_id_get_token_request_handler.cc:70:
UMA_HISTOGRAM_TIMES("InstanceID.GetTokenCompleteTime", complete_time);
Nit: Suggest putting an extra delimiter in all of these names, so that they're
better grouped together, i.e.:

InstanceID.GetToken.CompleteTime
InstanceID.GetToken.RetryCount

https://codereview.chromium.org/1167753002/diff/10001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1167753002/diff/10001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:13769: +<histogram
name="InstanceID.RestoredIDs">
Nit: RestoreIDCount .. so the name makes it obvious that it's a count, not e.g.
a boolean

https://codereview.chromium.org/1167753002/diff/10001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:13776: +<histogram
name="InstanceID.RestoredTokens">
Ditto.

[jianli, 2015-06-04 20:50:22]

https://codereview.chromium.org/1167753002/diff/10001/google_apis/gcm/engine/...
File google_apis/gcm/engine/gcm_store_impl.cc (right):

https://codereview.chromium.org/1167753002/diff/10001/google_apis/gcm/engine/...
google_apis/gcm/engine/gcm_store_impl.cc:346: ++iter) {
On 2015/06/04 18:59:32, Alexei Svitkine wrote:
> Nit: Use C++ for loop:
> 
> for (const auto& entry : result->registrations) {
>   if (StartsWithASCII(entry.first, ...
> }

Done.

https://codereview.chromium.org/1167753002/diff/10001/google_apis/gcm/engine/...
File google_apis/gcm/engine/instance_id_get_token_request_handler.cc (right):

https://codereview.chromium.org/1167753002/diff/10001/google_apis/gcm/engine/...
google_apis/gcm/engine/instance_id_get_token_request_handler.cc:70:
UMA_HISTOGRAM_TIMES("InstanceID.GetTokenCompleteTime", complete_time);
On 2015/06/04 18:59:32, Alexei Svitkine wrote:
> Nit: Suggest putting an extra delimiter in all of these names, so that they're
> better grouped together, i.e.:
> 
> InstanceID.GetToken.CompleteTime
> InstanceID.GetToken.RetryCount

Done.

https://codereview.chromium.org/1167753002/diff/10001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1167753002/diff/10001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:13769: +<histogram
name="InstanceID.RestoredIDs">
On 2015/06/04 18:59:32, Alexei Svitkine wrote:
> Nit: RestoreIDCount .. so the name makes it obvious that it's a count, not
e.g.
> a boolean

Done.

https://codereview.chromium.org/1167753002/diff/10001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:13776: +<histogram
name="InstanceID.RestoredTokens">
On 2015/06/04 18:59:32, Alexei Svitkine wrote:
> Ditto.

Done.

[jianli, 2015-06-04 20:50:51]

jianli@chromium.org changed reviewers:
+ zea@chromium.org

[jianli, 2015-06-04 20:50:52]

zea, could you please also take a look? thanks.

[Alexei Svitkine (slow), 2015-06-04 20:54:33]

lgtm

[Nicolas Zea, 2015-06-04 20:59:08]

lgtm

[jianli, 2015-06-04 22:11:12]

The CQ bit was checked by jianli@chromium.org

[jianli, 2015-06-04 22:11:13]

The patchset sent to the CQ was uploaded after l-g-t-m from fgorski@chromium.org
Link to the patchset: https://codereview.chromium.org/1167753002/#ps30001
(title: "Address more feedback")

[commit-bot: I haz the power, 2015-06-04 22:12:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1167753002/30001

[commit-bot: I haz the power, 2015-06-04 22:28:22]

Committed patchset #3 (id:30001)

[commit-bot: I haz the power, 2015-06-04 22:30:04]

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