Adding location events to the histogram

Adding location events to the histogram.

BUG=164227
TEST=Check that LOCATION_REQUEST and LOCATION_EVENT buckets appeared in the dashboard.

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

[Ilya Sherman, 2013-05-21 22:50:56]

histograms.xml LGTM

[skare_, 2013-05-22 00:22:30]

lgtm

[rgustafson, 2013-05-22 01:18:29]

On the higher level, what exactly are we hoping to get from these numbers?
watchLocation should be able to get multiple updates per watch call, so just
that location_event > location_request?

https://codereview.chromium.org/15306005/diff/1/chrome/browser/resources/goog...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/15306005/diff/1/chrome/browser/resources/goog...
chrome/browser/resources/google_now/background.js:145: LOCATION_EVENT: 6,
Why location_event? location_update?

https://codereview.chromium.org/15306005/diff/1/chrome/browser/resources/goog...
chrome/browser/resources/google_now/background.js:382:
recordEvent(DiagnosticEvent.LOCATION_EVENT);
It's not clear that this should only be called from onLocationUpdate. I don't
know if it's worth it to create another method that can easily be identified as
happening on location update then call this line and updateNotificationCards
from there, but just wanted to point that out. Don't need to change anything if
you don't think it will cause confusion later.

[vadimt, 2013-05-22 02:02:36]

On 2013/05/22 01:18:29, rgustafson wrote:
> On the higher level, what exactly are we hoping to get from these numbers?
> watchLocation should be able to get multiple updates per watch call, so just
> that location_event > location_request?

This is to protect from degradation in the location retrieval. Suppose in a next
version of Chrome they decrease LBS request timeout without letting us know, and
thanks to this more location requests will return error. This will cause
LOCATION_UPDATE to drop relatively to LOCATION_REQUEST, and we can catch this.

[vadimt, 2013-05-22 02:02:52]

https://codereview.chromium.org/15306005/diff/1/chrome/browser/resources/goog...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/15306005/diff/1/chrome/browser/resources/goog...
chrome/browser/resources/google_now/background.js:145: LOCATION_EVENT: 6,
On 2013/05/22 01:18:29, rgustafson wrote:
> Why location_event? location_update?

Done.

https://codereview.chromium.org/15306005/diff/1/chrome/browser/resources/goog...
chrome/browser/resources/google_now/background.js:382:
recordEvent(DiagnosticEvent.LOCATION_EVENT);
On 2013/05/22 01:18:29, rgustafson wrote:
> It's not clear that this should only be called from onLocationUpdate. I don't
> know if it's worth it to create another method that can easily be identified
as
> happening on location update then call this line and updateNotificationCards
> from there, but just wanted to point that out. Don't need to change anything
if
> you don't think it will cause confusion later.

Laziness :)

[rgustafson, 2013-05-22 21:21:31]

lgtm

[vadimt, 2013-05-22 21:28:49]

estade@, please provide OWNERs approval for
chrome/browser/resources/google_now/background.js

[Evan Stade, 2013-05-23 00:50:57]

rslgtm

[commit-bot: I haz the power, 2013-05-23 17:41:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vadimt@chromium.org/15306005/7001

[commit-bot: I haz the power, 2013-05-24 06:43:37]

Change committed as 201988