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

Issue 25112004: Move statistics_provider to chromeos/system (Closed)

Created:
7 years, 2 months ago by stevenjb
Modified:
7 years, 2 months ago
CC:
chromium-reviews, tim+watch_chromium.org, extensions-reviews_chromium.org, nkostylev+watch_chromium.org, tbarzic+watch_chromium.org, haitaol+watch_chromium.org, dkrahn+watch_chromium.org, asvitkine+watch_chromium.org, rginda+watch_chromium.org, oshima+watch_chromium.org, chromium-apps-reviews_chromium.org, rsimha+watch_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org, jar (doing other things)
Visibility:
Public.

Description

Move statistics_provider to chromeos/system BUG=270135 For sync/util/get_session_name.cc (comment change only): TBR=tim@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=227700

Patch Set 1 #

Patch Set 2 : Rebase, fix DeviceManagement #

Total comments: 13

Patch Set 3 : Rebase + remove Lock #

Patch Set 4 : Similarity=33 #

Total comments: 7

Patch Set 5 : Rebase #

Patch Set 6 : Elim. WeakPtr, add CancellationFlag, elim Stub Impl #

Patch Set 7 : Compile fixes #

Patch Set 8 : Fix unit tests #

Patch Set 9 : Fix unit tests #

Patch Set 10 : Fix browser tests #

Total comments: 16

Patch Set 11 : Rebase #

Patch Set 12 : Add comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+258 lines, -663 lines) Patch
M chrome/browser/chromeos/attestation/platform_verification_flow.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/attestation/platform_verification_flow_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/chrome_browser_main_chromeos.cc View 1 2 3 4 5 6 chunks +9 lines, -14 lines 0 comments Download
M chrome/browser/chromeos/customization_document.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/customization_document_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/extensions/echo_private_api.cc View 1 2 3 4 2 chunks +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/extensions/file_manager/private_api_strings.cc View 1 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/chromeos/extensions/info_private_api.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/imageburner/burn_manager.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/login/existing_user_controller.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/chromeos/login/hwid_checker.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/login/login_utils_browsertest.cc View 1 2 3 4 5 6 7 8 9 3 chunks +9 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc View 1 2 3 4 5 6 7 8 4 chunks +17 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/policy/device_status_collector.cc View 2 chunks +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/policy/device_status_collector_browsertest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/chromeos/preferences.cc View 2 chunks +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/system/input_device_settings.cc View 1 chunk +1 line, -1 line 0 comments Download
D chrome/browser/chromeos/system/mock_statistics_provider.h View 1 chunk +0 lines, -35 lines 0 comments Download
D chrome/browser/chromeos/system/mock_statistics_provider.cc View 1 chunk +0 lines, -15 lines 0 comments Download
D chrome/browser/chromeos/system/statistics_provider.h View 1 chunk +0 lines, -70 lines 0 comments Download
D chrome/browser/chromeos/system/statistics_provider.cc View 1 2 1 chunk +0 lines, -311 lines 0 comments Download
M chrome/browser/metrics/metrics_service.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/policy/browser_policy_connector.cc View 1 2 3 4 3 chunks +2 lines, -2 lines 0 comments Download
M chrome/chrome_browser_chromeos.gypi View 1 2 3 4 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/chrome_tests_unit.gypi View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -2 lines 0 comments Download
M chromeos/chromeos.gyp View 1 2 3 4 2 chunks +4 lines, -0 lines 0 comments Download
A + chromeos/system/mock_statistics_provider.h View 1 2 3 4 5 6 2 chunks +9 lines, -8 lines 0 comments Download
A + chromeos/system/mock_statistics_provider.cc View 1 chunk +1 line, -1 line 0 comments Download
A + chromeos/system/statistics_provider.h View 1 2 3 4 5 6 7 2 chunks +38 lines, -26 lines 0 comments Download
A + chromeos/system/statistics_provider.cc View 1 2 3 4 5 6 7 8 9 10 11 6 chunks +149 lines, -156 lines 0 comments Download
M sync/util/get_session_name.cc View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 22 (0 generated)
stevenjb
Still doing a bit of testing, but mostly ready. This should clean up some dependency ...
7 years, 2 months ago (2013-10-02 01:19:06 UTC) #1
satorux1
https://codereview.chromium.org/25112004/diff/3001/chrome/browser/chromeos/chrome_browser_main_chromeos.cc File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right): https://codereview.chromium.org/25112004/diff/3001/chrome/browser/chromeos/chrome_browser_main_chromeos.cc#newcode390 chrome/browser/chromeos/chrome_browser_main_chromeos.cc:390: content::BrowserThread::IO)); Did you mean FILE? IO is not for ...
7 years, 2 months ago (2013-10-02 01:36:55 UTC) #2
stevenjb
7 years, 2 months ago (2013-10-02 04:07:33 UTC) #3
Ilya Sherman
Drive-by: Please update the WATCHLIST file as well. Thanks.
7 years, 2 months ago (2013-10-02 05:58:12 UTC) #4
Joao da Silva
https://codereview.chromium.org/25112004/diff/3001/chrome/browser/policy/cloud/device_management_service.cc File chrome/browser/policy/cloud/device_management_service.cc (right): https://codereview.chromium.org/25112004/diff/3001/chrome/browser/policy/cloud/device_management_service.cc#newcode33 chrome/browser/policy/cloud/device_management_service.cc:33: #include "chromeos/system/statistics_provider.h" Unfortunately this is a no-go. We are ...
7 years, 2 months ago (2013-10-02 10:06:25 UTC) #5
stevenjb
https://codereview.chromium.org/25112004/diff/3001/chrome/browser/policy/cloud/device_management_service.cc File chrome/browser/policy/cloud/device_management_service.cc (right): https://codereview.chromium.org/25112004/diff/3001/chrome/browser/policy/cloud/device_management_service.cc#newcode33 chrome/browser/policy/cloud/device_management_service.cc:33: #include "chromeos/system/statistics_provider.h" On 2013/10/02 10:06:26, Joao da Silva wrote: ...
7 years, 2 months ago (2013-10-02 16:59:48 UTC) #6
stevenjb
On 2013/10/02 05:58:12, Ilya Sherman wrote: > Drive-by: Please update the WATCHLIST file as well. ...
7 years, 2 months ago (2013-10-02 18:34:23 UTC) #7
Ilya Sherman
On 2013/10/02 18:34:23, stevenjb wrote: > On 2013/10/02 05:58:12, Ilya Sherman wrote: > > Drive-by: ...
7 years, 2 months ago (2013-10-02 23:20:39 UTC) #8
stevenjb
Did some non trivial changes to StatisticsProvider and hopefully aligned it better to get deltas. ...
7 years, 2 months ago (2013-10-03 01:17:22 UTC) #9
satorux1
https://codereview.chromium.org/25112004/diff/21001/chromeos/system/statistics_provider.cc File chromeos/system/statistics_provider.cc (right): https://codereview.chromium.org/25112004/diff/21001/chromeos/system/statistics_provider.cc#newcode189 chromeos/system/statistics_provider.cc:189: CHECK(!load_statistics_started_); DCHECK? https://codereview.chromium.org/25112004/diff/21001/chromeos/system/statistics_provider.cc#newcode198 chromeos/system/statistics_provider.cc:198: weak_ptr_factory_.GetWeakPtr(), you cannot use a ...
7 years, 2 months ago (2013-10-04 08:07:55 UTC) #10
satorux1
https://codereview.chromium.org/25112004/diff/21001/chromeos/system/statistics_provider.cc File chromeos/system/statistics_provider.cc (right): https://codereview.chromium.org/25112004/diff/21001/chromeos/system/statistics_provider.cc#newcode198 chromeos/system/statistics_provider.cc:198: weak_ptr_factory_.GetWeakPtr(), On 2013/10/04 08:07:56, satorux1 wrote: > you cannot ...
7 years, 2 months ago (2013-10-04 08:12:59 UTC) #11
stevenjb
PTAL Changes to: chrome_browser_main_chromeos.cc + statistics_provider.cc/h. https://codereview.chromium.org/25112004/diff/21001/chromeos/system/statistics_provider.cc File chromeos/system/statistics_provider.cc (right): https://codereview.chromium.org/25112004/diff/21001/chromeos/system/statistics_provider.cc#newcode189 chromeos/system/statistics_provider.cc:189: CHECK(!load_statistics_started_); On 2013/10/04 ...
7 years, 2 months ago (2013-10-04 18:48:18 UTC) #12
satorux1
https://codereview.chromium.org/25112004/diff/47001/chrome/browser/chromeos/chrome_browser_main_chromeos.cc File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right): https://codereview.chromium.org/25112004/diff/47001/chrome/browser/chromeos/chrome_browser_main_chromeos.cc#newcode752 chrome/browser/chromeos/chrome_browser_main_chromeos.cc:752: system::StatisticsProvider::GetInstance()->Shutdown(); Other classes just provide static Shutdown() and Initialize() ...
7 years, 2 months ago (2013-10-07 05:01:53 UTC) #13
stevenjb
https://codereview.chromium.org/25112004/diff/47001/chrome/browser/chromeos/chrome_browser_main_chromeos.cc File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right): https://codereview.chromium.org/25112004/diff/47001/chrome/browser/chromeos/chrome_browser_main_chromeos.cc#newcode752 chrome/browser/chromeos/chrome_browser_main_chromeos.cc:752: system::StatisticsProvider::GetInstance()->Shutdown(); On 2013/10/07 05:01:54, satorux1 wrote: > Other classes ...
7 years, 2 months ago (2013-10-07 18:07:29 UTC) #14
stevenjb
https://codereview.chromium.org/25112004/diff/47001/chromeos/system/statistics_provider.cc File chromeos/system/statistics_provider.cc (right): https://codereview.chromium.org/25112004/diff/47001/chromeos/system/statistics_provider.cc#newcode208 chromeos/system/statistics_provider.cc:208: void StatisticsProviderImpl::LoadMachineStatistics(bool load_oem_manifest) { On 2013/10/07 18:07:30, stevenjb wrote: ...
7 years, 2 months ago (2013-10-07 18:43:09 UTC) #15
satorux1
LGTM https://codereview.chromium.org/25112004/diff/47001/chromeos/system/statistics_provider.cc File chromeos/system/statistics_provider.cc (right): https://codereview.chromium.org/25112004/diff/47001/chromeos/system/statistics_provider.cc#newcode209 chromeos/system/statistics_provider.cc:209: if (cancellation_flag_.IsSet()) On 2013/10/07 18:07:30, stevenjb wrote: > ...
7 years, 2 months ago (2013-10-08 01:47:30 UTC) #16
Joao da Silva
policy lgtm
7 years, 2 months ago (2013-10-08 12:45:48 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/stevenjb@chromium.org/25112004/60001
7 years, 2 months ago (2013-10-08 15:43:08 UTC) #18
commit-bot: I haz the power
Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_presubmit&number=29286
7 years, 2 months ago (2013-10-08 15:58:34 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/stevenjb@chromium.org/25112004/60001
7 years, 2 months ago (2013-10-08 16:01:58 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/stevenjb@chromium.org/25112004/60001
7 years, 2 months ago (2013-10-09 02:10:26 UTC) #21
commit-bot: I haz the power
7 years, 2 months ago (2013-10-09 08:23:54 UTC) #22
Message was sent while issue was closed.
Change committed as 227700

Powered by Google App Engine
This is Rietveld 408576698