Chromium Code Reviews
[email protected] (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(409)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 294063002: [Metrics] Update MetricsLog to query MetricsServiceClient for several //chrome dependencies. (Closed)

Created:
6 years, 7 months ago by Ilya Sherman
Modified:
6 years, 7 months ago
CC:
chromium-reviews, jar (doing other things), asvitkine+watch_chromium.org, blundell
Visibility:
Public.

Description

[Metrics] Update MetricsLog to query MetricsServiceClient for several //chrome dependencies. Along the way, clean up the MetricsLogManager API to use a scoped_ptr to express ownership semantics. BUG=374233, 374737 TEST=compiles, unit_tests [email protected], [email protected] [email protected] Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272173

Patch Set 1 #

Total comments: 6

Patch Set 2 : CreateLog and scoped_ptr #

Total comments: 1

Patch Set 3 : Rebase #

Patch Set 4 : Rebase better #

Patch Set 5 : Rebase #

Patch Set 6 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+164 lines, -181 lines) Patch
M chrome/browser/chrome_browser_main.cc View 1 2 2 chunks +0 lines, -4 lines 0 comments Download
M chrome/browser/metrics/chrome_metrics_service_client.cc View 1 2 3 4 5 1 chunk +13 lines, -2 lines 0 comments Download
M chrome/browser/metrics/metrics_log.h View 1 2 3 4 5 3 chunks +15 lines, -11 lines 0 comments Download
M chrome/browser/metrics/metrics_log.cc View 1 2 3 4 5 7 chunks +11 lines, -59 lines 0 comments Download
M chrome/browser/metrics/metrics_log_unittest.cc View 1 2 3 4 5 21 chunks +49 lines, -28 lines 0 comments Download
M chrome/browser/metrics/metrics_service.h View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/metrics/metrics_service.cc View 1 2 3 4 5 8 chunks +14 lines, -11 lines 0 comments Download
M chrome/browser/metrics/metrics_service_unittest.cc View 1 2 3 4 5 7 chunks +13 lines, -10 lines 0 comments Download
M chrome/browser/metrics/metrics_services_manager.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M components/metrics/metrics_log_manager.h View 1 1 chunk +3 lines, -3 lines 0 comments Download
M components/metrics/metrics_log_manager.cc View 1 1 chunk +3 lines, -3 lines 0 comments Download
M components/metrics/metrics_log_manager_unittest.cc View 1 12 chunks +33 lines, -48 lines 0 comments Download
M components/metrics/test_metrics_service_client.h View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M components/metrics/test_metrics_service_client.cc View 1 2 3 4 5 2 chunks +4 lines, -1 line 0 comments Download

Messages

Total messages: 23 (0 generated)
Ilya Sherman
6 years, 7 months ago (2014-05-20 12:50:17 UTC) #1
Ilya Sherman
Note: depends on https://chromiumcodereview.appspot.com/290343005/
6 years, 7 months ago (2014-05-20 12:52:55 UTC) #2
Alexei Svitkine (slow)
https://codereview.chromium.org/294063002/diff/1/chrome/browser/metrics/chrome_metrics_service_client.cc File chrome/browser/metrics/chrome_metrics_service_client.cc (right): https://codereview.chromium.org/294063002/diff/1/chrome/browser/metrics/chrome_metrics_service_client.cc#newcode71 chrome/browser/metrics/chrome_metrics_service_client.cc:71: version += "-64"; Nice! I hadn't realised this was ...
6 years, 7 months ago (2014-05-20 14:11:04 UTC) #3
Ilya Sherman
https://codereview.chromium.org/294063002/diff/1/chrome/browser/metrics/chrome_metrics_service_client.cc File chrome/browser/metrics/chrome_metrics_service_client.cc (right): https://codereview.chromium.org/294063002/diff/1/chrome/browser/metrics/chrome_metrics_service_client.cc#newcode71 chrome/browser/metrics/chrome_metrics_service_client.cc:71: version += "-64"; On 2014/05/20 14:11:04, Alexei Svitkine wrote: ...
6 years, 7 months ago (2014-05-20 15:10:21 UTC) #4
Alexei Svitkine (slow)
LGTM, thanks! https://codereview.chromium.org/294063002/diff/10001/chrome/browser/metrics/metrics_service.cc File chrome/browser/metrics/metrics_service.cc (right): https://codereview.chromium.org/294063002/diff/10001/chrome/browser/metrics/metrics_service.cc#newcode1743 chrome/browser/metrics/metrics_service.cc:1743: return make_scoped_ptr(new MetricsLog( Neat, I didn't know ...
6 years, 7 months ago (2014-05-20 15:14:30 UTC) #5
Ilya Sherman
TBR [email protected] for code removal from chrome_browser_main.cc
6 years, 7 months ago (2014-05-21 09:26:05 UTC) #6
Ilya Sherman
The CQ bit was checked by [email protected]
6 years, 7 months ago (2014-05-21 09:43:27 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/[email protected]/294063002/30001
6 years, 7 months ago (2014-05-21 09:46:36 UTC) #8
Ilya Sherman
The CQ bit was unchecked by [email protected]
6 years, 7 months ago (2014-05-21 09:48:38 UTC) #9
Ilya Sherman
The CQ bit was checked by [email protected]
6 years, 7 months ago (2014-05-21 09:49:09 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/[email protected]/294063002/50001
6 years, 7 months ago (2014-05-21 09:52:03 UTC) #11
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are ...
6 years, 7 months ago (2014-05-21 20:03:08 UTC) #12
commit-bot: I haz the power
The CQ bit was unchecked by [email protected]
6 years, 7 months ago (2014-05-21 20:21:45 UTC) #13
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/builds/18476)
6 years, 7 months ago (2014-05-21 20:21:46 UTC) #14
Lei Zhang
chrome/browser/chrome_browser_main.cc lgtm
6 years, 7 months ago (2014-05-21 20:53:20 UTC) #15
Ilya Sherman
The CQ bit was checked by [email protected]
6 years, 7 months ago (2014-05-21 22:34:17 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/[email protected]/294063002/70001
6 years, 7 months ago (2014-05-21 22:39:10 UTC) #17
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are ...
6 years, 7 months ago (2014-05-22 08:54:14 UTC) #18
commit-bot: I haz the power
The CQ bit was unchecked by [email protected]
6 years, 7 months ago (2014-05-22 09:04:31 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/builds/18774)
6 years, 7 months ago (2014-05-22 09:04:32 UTC) #20
Ilya Sherman
The CQ bit was checked by [email protected]
6 years, 7 months ago (2014-05-22 11:42:20 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/[email protected]/294063002/90001
6 years, 7 months ago (2014-05-22 11:43:59 UTC) #22
Ilya Sherman
6 years, 7 months ago (2014-05-22 14:01:03 UTC) #23
Message was sent while issue was closed. 
Committed patchset #6 manually as r272173 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698