diff --git a/components/sync/BUILD.gn b/components/sync/BUILD.gn index 9bed749fb9c4d..01e58e6ef02ad 100644 --- a/components/sync/BUILD.gn +++ b/components/sync/BUILD.gn @@ -560,7 +560,6 @@ source_set("unit_tests") { "base/bind_to_task_runner_unittest.cc", "base/cancelation_signal_unittest.cc", "base/client_tag_hash_unittest.cc", - "base/data_type_histogram_unittest.cc", "base/enum_set_unittest.cc", "base/immutable_unittest.cc", "base/model_type_unittest.cc", diff --git a/components/sync/base/data_type_histogram.h b/components/sync/base/data_type_histogram.h index 6aef5f83520b2..c602f8cd13b32 100644 --- a/components/sync/base/data_type_histogram.h +++ b/components/sync/base/data_type_histogram.h @@ -21,147 +21,4 @@ void SyncRecordModelTypeMemoryHistogram(syncer::ModelType model_type, void SyncRecordModelTypeCountHistogram(syncer::ModelType model_type, size_t count); -// Helper macro for datatype specific histograms. For each datatype, invokes -// a pre-defined PER_DATA_TYPE_MACRO(type_str), where |type_str| is the string -// version of the datatype. -// -// Example usage (ignoring newlines necessary for multiline macro): -// std::vector<syncer::ModelType> types = GetEntryTypes(); -// for (int i = 0; i < types.size(); ++i) { -// #define PER_DATA_TYPE_MACRO(type_str) -// UMA_HISTOGRAM_ENUMERATION("Sync." type_str "StartFailures", -// error, max_error); -// SYNC_DATA_TYPE_HISTOGRAM(types[i]); -// #undef PER_DATA_TYPE_MACRO -// } -// -// TODO(zea): Once visual studio supports proper variadic argument replacement -// in macros, pass in the histogram method directly as a parameter. -// See http://connect.microsoft.com/VisualStudio/feedback/details/380090/ -// variadic-macro-replacement#details -// When adding a new datatype in the switch below, also update the SyncModelType -// and SyncModelTypeByMacro histogram suffixes in histograms.xml. -#define SYNC_DATA_TYPE_HISTOGRAM(datatype) \ - do { \ - switch (datatype) { \ - case ::syncer::BOOKMARKS: \ - PER_DATA_TYPE_MACRO("Bookmarks"); \ - break; \ - case ::syncer::PREFERENCES: \ - PER_DATA_TYPE_MACRO("Preferences"); \ - break; \ - case ::syncer::PASSWORDS: \ - PER_DATA_TYPE_MACRO("Passwords"); \ - break; \ - case ::syncer::AUTOFILL_PROFILE: \ - PER_DATA_TYPE_MACRO("AutofillProfiles"); \ - break; \ - case ::syncer::AUTOFILL: \ - PER_DATA_TYPE_MACRO("Autofill"); \ - break; \ - case ::syncer::AUTOFILL_WALLET_DATA: \ - PER_DATA_TYPE_MACRO("AutofillWallet"); \ - break; \ - case ::syncer::AUTOFILL_WALLET_METADATA: \ - PER_DATA_TYPE_MACRO("AutofillWalletMetadata"); \ - break; \ - case ::syncer::THEMES: \ - PER_DATA_TYPE_MACRO("Themes"); \ - break; \ - case ::syncer::TYPED_URLS: \ - PER_DATA_TYPE_MACRO("TypedUrls"); \ - break; \ - case ::syncer::EXTENSIONS: \ - PER_DATA_TYPE_MACRO("Extensions"); \ - break; \ - case ::syncer::SEARCH_ENGINES: \ - PER_DATA_TYPE_MACRO("SearchEngines"); \ - break; \ - case ::syncer::SESSIONS: \ - PER_DATA_TYPE_MACRO("Sessions"); \ - break; \ - case ::syncer::APPS: \ - PER_DATA_TYPE_MACRO("Apps"); \ - break; \ - case ::syncer::APP_SETTINGS: \ - PER_DATA_TYPE_MACRO("AppSettings"); \ - break; \ - case ::syncer::EXTENSION_SETTINGS: \ - PER_DATA_TYPE_MACRO("ExtensionSettings"); \ - break; \ - case ::syncer::HISTORY_DELETE_DIRECTIVES: \ - PER_DATA_TYPE_MACRO("HistoryDeleteDirectives"); \ - break; \ - case ::syncer::DICTIONARY: \ - PER_DATA_TYPE_MACRO("Dictionary"); \ - break; \ - case ::syncer::FAVICON_IMAGES: \ - PER_DATA_TYPE_MACRO("FaviconImages"); \ - break; \ - case ::syncer::FAVICON_TRACKING: \ - PER_DATA_TYPE_MACRO("FaviconTracking"); \ - break; \ - case ::syncer::DEVICE_INFO: \ - PER_DATA_TYPE_MACRO("DeviceInfo"); \ - break; \ - case ::syncer::PRIORITY_PREFERENCES: \ - PER_DATA_TYPE_MACRO("PriorityPreferences"); \ - break; \ - case ::syncer::SUPERVISED_USER_SETTINGS: \ - PER_DATA_TYPE_MACRO("ManagedUserSetting"); \ - break; \ - case ::syncer::APP_LIST: \ - PER_DATA_TYPE_MACRO("AppList"); \ - break; \ - case ::syncer::SUPERVISED_USER_WHITELISTS: \ - PER_DATA_TYPE_MACRO("ManagedUserWhitelist"); \ - break; \ - case ::syncer::ARC_PACKAGE: \ - PER_DATA_TYPE_MACRO("ArcPackage"); \ - break; \ - case ::syncer::PRINTERS: \ - PER_DATA_TYPE_MACRO("Printers"); \ - break; \ - case ::syncer::READING_LIST: \ - PER_DATA_TYPE_MACRO("ReadingList"); \ - break; \ - case ::syncer::USER_CONSENTS: \ - PER_DATA_TYPE_MACRO("UserConsents"); \ - break; \ - case ::syncer::USER_EVENTS: \ - PER_DATA_TYPE_MACRO("UserEvents"); \ - break; \ - case ::syncer::PROXY_TABS: \ - PER_DATA_TYPE_MACRO("Tabs"); \ - break; \ - case ::syncer::NIGORI: \ - PER_DATA_TYPE_MACRO("Nigori"); \ - break; \ - case ::syncer::DEPRECATED_EXPERIMENTS: \ - PER_DATA_TYPE_MACRO("Experiments"); \ - break; \ - case ::syncer::SEND_TAB_TO_SELF: \ - PER_DATA_TYPE_MACRO("SendTabToSelf"); \ - break; \ - case ::syncer::SECURITY_EVENTS: \ - PER_DATA_TYPE_MACRO("SecurityEvents"); \ - break; \ - case ::syncer::WEB_APPS: \ - PER_DATA_TYPE_MACRO("WebApps"); \ - break; \ - case ::syncer::WIFI_CONFIGURATIONS: \ - PER_DATA_TYPE_MACRO("WifiConfigurations"); \ - break; \ - case ::syncer::OS_PREFERENCES: \ - PER_DATA_TYPE_MACRO("OsPreferences"); \ - break; \ - case ::syncer::OS_PRIORITY_PREFERENCES: \ - PER_DATA_TYPE_MACRO("OsPriorityPreferences"); \ - break; \ - default: \ - NOTREACHED() << "Unknown datatype " \ - << ::syncer::ModelTypeToString(datatype); \ - } \ - } while (0) - #endif // COMPONENTS_SYNC_BASE_DATA_TYPE_HISTOGRAM_H_ diff --git a/components/sync/base/data_type_histogram_unittest.cc b/components/sync/base/data_type_histogram_unittest.cc deleted file mode 100644 index 764738d2b2af8..0000000000000 --- a/components/sync/base/data_type_histogram_unittest.cc +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "components/sync/base/data_type_histogram.h" - -#include "testing/gtest/include/gtest/gtest.h" - -namespace syncer { -namespace { - -class DataTypeHistogramTest : public testing::Test {}; - -// Create a histogram of type LOCAL_HISTOGRAM_COUNTS for each model type. -// Nothing should break. -TEST(DataTypeHistogramTest, BasicCount) { - for (int i = FIRST_REAL_MODEL_TYPE; i <= LAST_REAL_MODEL_TYPE; ++i) { - ModelType type = ModelTypeFromInt(i); -#define PER_DATA_TYPE_MACRO(type_str) \ - LOCAL_HISTOGRAM_COUNTS("BasicCountPrefix" type_str "Suffix", 1); - SYNC_DATA_TYPE_HISTOGRAM(type); -#undef PER_DATA_TYPE_MACRO - } -} - -// Create a histogram of type UMA_HISTOGRAM_ENUMERATION for each model type. -// Nothing should break. -TEST(DataTypeHistogramTest, BasicEnum) { - enum HistTypes { - TYPE_1, - TYPE_2, - TYPE_COUNT, - }; - for (int i = FIRST_REAL_MODEL_TYPE; i <= LAST_REAL_MODEL_TYPE; ++i) { - ModelType type = ModelTypeFromInt(i); -#define PER_DATA_TYPE_MACRO(type_str) \ - UMA_HISTOGRAM_ENUMERATION("BasicEnumPrefix" type_str "Suffix", \ - (i % 2 ? TYPE_1 : TYPE_2), TYPE_COUNT); - SYNC_DATA_TYPE_HISTOGRAM(type); -#undef PER_DATA_TYPE_MACRO - } -} - -} // namespace -} // namespace syncer diff --git a/components/sync/driver/async_directory_type_controller.cc b/components/sync/driver/async_directory_type_controller.cc index 971301a1b9842..41f2b9d364ea0 100644 --- a/components/sync/driver/async_directory_type_controller.cc +++ b/components/sync/driver/async_directory_type_controller.cc @@ -209,14 +209,8 @@ void AsyncDirectoryTypeController::StartDone( void AsyncDirectoryTypeController::RecordStartFailure(ConfigureResult result) { DCHECK(CalledOnValidThread()); - // TODO(wychen): enum uma should be strongly typed. crbug.com/661401 UMA_HISTOGRAM_ENUMERATION("Sync.DataTypeStartFailures2", ModelTypeHistogramValue(type())); -#define PER_DATA_TYPE_MACRO(type_str) \ - UMA_HISTOGRAM_ENUMERATION("Sync." type_str "ConfigureFailure", result, \ - MAX_CONFIGURE_RESULT); - SYNC_DATA_TYPE_HISTOGRAM(type()); -#undef PER_DATA_TYPE_MACRO } void AsyncDirectoryTypeController::DisableImpl(const SyncError& error) { diff --git a/components/sync/driver/async_directory_type_controller_unittest.cc b/components/sync/driver/async_directory_type_controller_unittest.cc index 016a237d2c8e8..26538b25bce05 100644 --- a/components/sync/driver/async_directory_type_controller_unittest.cc +++ b/components/sync/driver/async_directory_type_controller_unittest.cc @@ -76,7 +76,6 @@ class SharedChangeProcessorMock : public SharedChangeProcessor { MOCK_METHOD1(SyncModelHasUserCreatedNodes, bool(bool*)); MOCK_METHOD0(CryptoReadyIfNecessary, bool()); MOCK_CONST_METHOD1(GetDataTypeContext, bool(std::string*)); - MOCK_METHOD1(RecordAssociationTime, void(base::TimeDelta time)); void SetConnectReturn(base::WeakPtr<SyncableService> service) { connect_return_ = service; @@ -221,7 +220,6 @@ class SyncAsyncDirectoryTypeControllerTest : public testing::Test { EXPECT_CALL(*change_processor_, GetAllSyncDataReturnError(_, _)) .WillOnce(Return(SyncError())); EXPECT_CALL(*change_processor_, GetSyncCount()).WillOnce(Return(0)); - EXPECT_CALL(*change_processor_, RecordAssociationTime(_)); } void SetActivateExpectations(DataTypeController::ConfigureResult result) { @@ -284,7 +282,6 @@ TEST_F(SyncAsyncDirectoryTypeControllerTest, StartFirstRun) { .WillOnce(DoAll(SetArgPointee<0>(false), Return(true))); EXPECT_CALL(*change_processor_, GetAllSyncDataReturnError(_, _)) .WillOnce(Return(SyncError())); - EXPECT_CALL(*change_processor_, RecordAssociationTime(_)); SetActivateExpectations(DataTypeController::OK_FIRST_RUN); EXPECT_EQ(DataTypeController::NOT_RUNNING, non_ui_dtc_->state()); Start(); @@ -319,7 +316,6 @@ TEST_F(SyncAsyncDirectoryTypeControllerTest, StartAssociationFailed) { .WillOnce(DoAll(SetArgPointee<0>(true), Return(true))); EXPECT_CALL(*change_processor_, GetAllSyncDataReturnError(_, _)) .WillOnce(Return(SyncError())); - EXPECT_CALL(*change_processor_, RecordAssociationTime(_)); SetStartFailExpectations(DataTypeController::ASSOCIATION_FAILED); // Set up association to fail with an association failed error. EXPECT_EQ(DataTypeController::NOT_RUNNING, non_ui_dtc_->state()); diff --git a/components/sync/driver/data_type_controller.cc b/components/sync/driver/data_type_controller.cc index 9ce9bf0ca0a0a..e903be84b2ef2 100644 --- a/components/sync/driver/data_type_controller.cc +++ b/components/sync/driver/data_type_controller.cc @@ -4,7 +4,6 @@ #include "components/sync/driver/data_type_controller.h" -#include "components/sync/base/data_type_histogram.h" #include "components/sync/syncable/user_share.h" namespace syncer { diff --git a/components/sync/driver/model_type_controller.cc b/components/sync/driver/model_type_controller.cc index 8133aada415c7..de85492a40efb 100644 --- a/components/sync/driver/model_type_controller.cc +++ b/components/sync/driver/model_type_controller.cc @@ -250,17 +250,6 @@ void ModelTypeController::ReportModelError(SyncError::ErrorType error_type, const ModelError& error) { DCHECK(CalledOnValidThread()); - // TODO(crbug.com/890729): This is obviously misplaced/misnamed as we report - // run-time failures as well. Rename the histogram to ConfigureResult and - // report it only after startup (also for success). - if (state_ != NOT_RUNNING) { -#define PER_DATA_TYPE_MACRO(type_str) \ - UMA_HISTOGRAM_ENUMERATION("Sync." type_str "ConfigureFailure", \ - UNRECOVERABLE_ERROR, MAX_CONFIGURE_RESULT); - SYNC_DATA_TYPE_HISTOGRAM(type()); -#undef PER_DATA_TYPE_MACRO - } - switch (state_) { case MODEL_LOADED: // Fall through. Before state_ is flipped to RUNNING, we treat it as a diff --git a/components/sync/driver/shared_change_processor.cc b/components/sync/driver/shared_change_processor.cc index 7b6e755f2479c..06a91b5a0fc3c 100644 --- a/components/sync/driver/shared_change_processor.cc +++ b/components/sync/driver/shared_change_processor.cc @@ -7,7 +7,6 @@ #include <utility> #include "base/threading/sequenced_task_runner_handle.h" -#include "components/sync/base/data_type_histogram.h" #include "components/sync/driver/generic_change_processor.h" #include "components/sync/driver/generic_change_processor_factory.h" #include "components/sync/driver/shared_change_processor_ref.h" @@ -98,7 +97,6 @@ void SharedChangeProcessor::StartAssociation( { SyncDataList initial_sync_data; - base::TimeTicks start_time = base::TimeTicks::Now(); SyncError error = GetAllSyncDataReturnError(type_, &initial_sync_data); if (error.IsSet()) { local_merge_result.set_error(error); @@ -114,7 +112,6 @@ void SharedChangeProcessor::StartAssociation( type_, initial_sync_data, std::unique_ptr<SyncChangeProcessor>( new SharedChangeProcessorRef(this)), std::unique_ptr<SyncErrorFactory>(new SharedChangeProcessorRef(this))); - RecordAssociationTime(base::TimeTicks::Now() - start_time); if (local_merge_result.error().IsSet()) { start_done.Run(DataTypeController::ASSOCIATION_FAILED, local_merge_result, syncer_merge_result); @@ -297,13 +294,6 @@ SyncError SharedChangeProcessor::CreateAndUploadError( } } -void SharedChangeProcessor::RecordAssociationTime(base::TimeDelta time) { -#define PER_DATA_TYPE_MACRO(type_str) \ - UMA_HISTOGRAM_TIMES("Sync." type_str "AssociationTime", time); - SYNC_DATA_TYPE_HISTOGRAM(type_); -#undef PER_DATA_TYPE_MACRO -} - void SharedChangeProcessor::StopLocalService() { if (local_service_) local_service_->StopSyncing(type_); diff --git a/components/sync/driver/shared_change_processor.h b/components/sync/driver/shared_change_processor.h index 7051ef761a12c..067b5ab0f98d7 100644 --- a/components/sync/driver/shared_change_processor.h +++ b/components/sync/driver/shared_change_processor.h @@ -123,9 +123,6 @@ class SharedChangeProcessor virtual ~SharedChangeProcessor(); private: - // Record association time. - virtual void RecordAssociationTime(base::TimeDelta time); - // Monitor lock for this object. All methods that interact with the change // processor must aquire this lock and check whether we're disconnected or // not. Once disconnected, all attempted changes to or loads from the change diff --git a/components/sync/model_impl/syncable_service_based_bridge.cc b/components/sync/model_impl/syncable_service_based_bridge.cc index 24c1e946c7f2f..864c537e6f404 100644 --- a/components/sync/model_impl/syncable_service_based_bridge.cc +++ b/components/sync/model_impl/syncable_service_based_bridge.cc @@ -12,7 +12,6 @@ #include "base/location.h" #include "base/trace_event/memory_usage_estimator.h" #include "components/sync/base/client_tag_hash.h" -#include "components/sync/base/data_type_histogram.h" #include "components/sync/model/mutable_data_batch.h" #include "components/sync/model/sync_change.h" #include "components/sync/model/sync_error_factory.h" @@ -533,8 +532,6 @@ base::Optional<ModelError> SyncableServiceBasedBridge::StartSyncableService() { DCHECK(!syncable_service_started_); DCHECK(change_processor()->IsTrackingMetadata()); - const base::TimeTicks start_time = base::TimeTicks::Now(); - // Sync enabled, so exercise MergeDataAndStartSyncing() immediately, since // this function is reached only if sync is starting already. SyncDataList initial_sync_data; @@ -560,8 +557,6 @@ base::Optional<ModelError> SyncableServiceBasedBridge::StartSyncableService() { std::make_unique<SyncErrorFactoryImpl>(type_)) .error()); - RecordAssociationTime(base::TimeTicks::Now() - start_time); - if (!merge_error) { syncable_service_started_ = true; } @@ -680,13 +675,4 @@ void SyncableServiceBasedBridge::ReportErrorIfSet( } } -void SyncableServiceBasedBridge::RecordAssociationTime( - base::TimeDelta time) const { -// This mimics the implementation in SharedChangeProcessor. -#define PER_DATA_TYPE_MACRO(type_str) \ - UMA_HISTOGRAM_TIMES("Sync." type_str "AssociationTime", time); - SYNC_DATA_TYPE_HISTOGRAM(type_); -#undef PER_DATA_TYPE_MACRO -} - } // namespace syncer diff --git a/components/sync/model_impl/syncable_service_based_bridge.h b/components/sync/model_impl/syncable_service_based_bridge.h index 3b2ffbd6fecbf..6afaef9a19521 100644 --- a/components/sync/model_impl/syncable_service_based_bridge.h +++ b/components/sync/model_impl/syncable_service_based_bridge.h @@ -96,7 +96,6 @@ class SyncableServiceBasedBridge : public ModelTypeSyncBridge { const base::Optional<ModelError>& error, std::unique_ptr<ModelTypeStore::RecordList> record_list); void ReportErrorIfSet(const base::Optional<ModelError>& error); - void RecordAssociationTime(base::TimeDelta time) const; const ModelType type_; SyncableService* const syncable_service_; diff --git a/docs/sync/model_api.md b/docs/sync/model_api.md index 68a53ccaf1ca5..49ac84d9a9014 100644 --- a/docs/sync/model_api.md +++ b/docs/sync/model_api.md @@ -278,7 +278,6 @@ the next client restart. [GetUserSelectableTypeInfo]. * Add to the `SyncModelTypes` enum in [`enums.xml`][enums] and to the `SyncModelType` suffix in [`histograms.xml`][histograms]. -* Add to the [`SYNC_DATA_TYPE_HISTOGRAM`][DataTypeHistogram] macro. [protocol]: https://cs.chromium.org/chromium/src/components/sync/protocol/ [ModelType]: https://cs.chromium.org/chromium/src/components/sync/base/model_type.h diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 8bc4646316a91..78efe25b60308 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -148198,6 +148198,9 @@ should be kept until we use this API. --> <histogram base="true" name="Sync.AssociationTime" units="ms" expires_after="2019-10-01"> + <obsolete> + Deprecated in M80. + </obsolete> <owner>jkrcal@chromium.org</owner> <owner>mastiz@chromium.org</owner> <summary>Time taken during association of this data type.</summary> @@ -148652,6 +148655,9 @@ should be kept until we use this API. --> <histogram base="true" name="Sync.ConfigureFailure" enum="SyncConfigureResult" expires_after="2020-04-01"> + <obsolete> + Deprecated in M80. + </obsolete> <owner>jkrcal@chromium.org</owner> <owner>mastiz@chromium.org</owner> <summary>