Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(619)

Issue 2722113004: Flush UKM metrics when backgrounded on Android and iOS. (Closed)

Created:
3 years, 9 months ago by Bryan McQuade
Modified:
3 years, 9 months ago
CC:
chromium-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Flush UKM metrics when backgrounded on Android and iOS. BUG=697632 Review-Url: https://codereview.chromium.org/2722113004 Cr-Commit-Position: refs/heads/master@{#454242} Committed: https://chromium.googlesource.com/chromium/src/+/919ff542cdde626f265f87cb8888a1a9d88bd797

Patch Set 1 #

Patch Set 2 : rebase #

Patch Set 3 : add missing newline #

Total comments: 4

Patch Set 4 : address asvitkine comments. #

Patch Set 5 : fix ios build #

Total comments: 2

Patch Set 6 : fix ios #

Patch Set 7 : address comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+75 lines, -9 lines) Patch
M chrome/browser/android/metrics/uma_session_stats.cc View 1 2 3 3 chunks +11 lines, -6 lines 0 comments Download
M components/ukm/ukm_service.h View 1 1 chunk +5 lines, -0 lines 0 comments Download
M components/ukm/ukm_service.cc View 1 2 3 4 5 6 1 chunk +30 lines, -0 lines 0 comments Download
M ios/chrome/browser/BUILD.gn View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M ios/chrome/browser/application_context.h View 1 2 3 4 5 2 chunks +7 lines, -0 lines 0 comments Download
M ios/chrome/browser/application_context_impl.h View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M ios/chrome/browser/application_context_impl.cc View 1 2 3 4 5 4 chunks +14 lines, -3 lines 0 comments Download
M ios/chrome/test/testing_application_context.h View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M ios/chrome/test/testing_application_context.mm View 1 2 3 4 5 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 39 (28 generated)
Bryan McQuade
Hi Steven, Here's a basic implementation that hooks into the app start/stop flow on Android, ...
3 years, 9 months ago (2017-03-01 22:23:11 UTC) #8
Steven Holte
lgtm
3 years, 9 months ago (2017-03-01 22:26:42 UTC) #9
Bryan McQuade
asvitkine, PTAL, thanks!
3 years, 9 months ago (2017-03-01 22:36:39 UTC) #14
Alexei Svitkine (slow)
https://codereview.chromium.org/2722113004/diff/40001/chrome/browser/android/metrics/uma_session_stats.cc File chrome/browser/android/metrics/uma_session_stats.cc (right): https://codereview.chromium.org/2722113004/diff/40001/chrome/browser/android/metrics/uma_session_stats.cc#newcode58 chrome/browser/android/metrics/uma_session_stats.cc:58: if (ukm_service) { Nit: No {}s Remove them from ...
3 years, 9 months ago (2017-03-01 22:56:13 UTC) #17
Bryan McQuade
Addressed comments. PTAL, thanks! https://codereview.chromium.org/2722113004/diff/40001/chrome/browser/android/metrics/uma_session_stats.cc File chrome/browser/android/metrics/uma_session_stats.cc (right): https://codereview.chromium.org/2722113004/diff/40001/chrome/browser/android/metrics/uma_session_stats.cc#newcode58 chrome/browser/android/metrics/uma_session_stats.cc:58: if (ukm_service) { On 2017/03/01 ...
3 years, 9 months ago (2017-03-01 23:06:51 UTC) #20
Alexei Svitkine (slow)
lgtm https://codereview.chromium.org/2722113004/diff/80001/components/ukm/ukm_service.cc File components/ukm/ukm_service.cc (right): https://codereview.chromium.org/2722113004/diff/80001/components/ukm/ukm_service.cc#newcode206 components/ukm/ukm_service.cc:206: // If initialize_started_ is false, UKM has not ...
3 years, 9 months ago (2017-03-01 23:24:02 UTC) #25
Bryan McQuade
https://codereview.chromium.org/2722113004/diff/80001/components/ukm/ukm_service.cc File components/ukm/ukm_service.cc (right): https://codereview.chromium.org/2722113004/diff/80001/components/ukm/ukm_service.cc#newcode206 components/ukm/ukm_service.cc:206: // If initialize_started_ is false, UKM has not yet ...
3 years, 9 months ago (2017-03-01 23:37:11 UTC) #28
Bryan McQuade
droger, PTAL for ios/chrome changes, thanks!
3 years, 9 months ago (2017-03-01 23:39:09 UTC) #30
droger
lgtm
3 years, 9 months ago (2017-03-02 09:59:42 UTC) #33
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2722113004/120001
3 years, 9 months ago (2017-03-02 13:00:51 UTC) #36
commit-bot: I haz the power
3 years, 9 months ago (2017-03-02 13:06:31 UTC) #39
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/919ff542cdde626f265f87cb8888...

Powered by Google App Engine
This is Rietveld 408576698