diff options
| -rw-r--r-- | chrome/browser/sync/engine/sync_scheduler.cc | 8 | ||||
| -rw-r--r-- | chrome/browser/sync/syncable/model_type.cc | 78 | ||||
| -rw-r--r-- | chrome/browser/sync/syncable/model_type.h | 6 | ||||
| -rw-r--r-- | chrome/browser/sync/util/data_type_histogram.h | 91 | ||||
| -rw-r--r-- | chrome/browser/sync/util/data_type_histogram_unittest.cc | 63 | ||||
| -rw-r--r-- | chrome/chrome.gyp | 1 | ||||
| -rw-r--r-- | chrome/chrome_tests.gypi | 1 |
7 files changed, 164 insertions, 84 deletions
diff --git a/chrome/browser/sync/engine/sync_scheduler.cc b/chrome/browser/sync/engine/sync_scheduler.cc index 8b23c01..f98341c 100644 --- a/chrome/browser/sync/engine/sync_scheduler.cc +++ b/chrome/browser/sync/engine/sync_scheduler.cc @@ -16,6 +16,7 @@ #include "chrome/browser/sync/engine/syncer.h" #include "chrome/browser/sync/protocol/sync.pb.h" #include "chrome/browser/sync/protocol/proto_enum_conversions.h" +#include "chrome/browser/sync/util/data_type_histogram.h" #include "chrome/browser/sync/util/logging.h" using base::TimeDelta; @@ -846,8 +847,11 @@ void SyncScheduler::FinishSyncSessionJob(const SyncSessionJob& job) { for (iter = job.session->source().types.begin(); iter != job.session->source().types.end(); ++iter) { - syncable::PostTimeToTypeHistogram(iter->first, - now - last_sync_session_end_time_); +#define PER_DATA_TYPE_MACRO(type_str) \ + SYNC_FREQ_HISTOGRAM("Sync.Freq" type_str, \ + now - last_sync_session_end_time_); + SYNC_DATA_TYPE_HISTOGRAM(iter->first); +#undef PER_DATA_TYPE_MACRO } } last_sync_session_end_time_ = now; diff --git a/chrome/browser/sync/syncable/model_type.cc b/chrome/browser/sync/syncable/model_type.cc index 6620275..7faf4523 100644 --- a/chrome/browser/sync/syncable/model_type.cc +++ b/chrome/browser/sync/syncable/model_type.cc @@ -1,10 +1,9 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// 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 "chrome/browser/sync/syncable/model_type.h" -#include "base/metrics/histogram.h" #include "base/string_split.h" #include "base/values.h" #include "chrome/browser/sync/engine/syncproto.h" @@ -408,81 +407,6 @@ std::string ModelTypeToRootTag(ModelType type) { return "INVALID"; } -// For now, this just implements UMA_HISTOGRAM_LONG_TIMES. This can be adjusted -// if we feel the min, max, or bucket count amount are not appropriate. -#define SYNC_FREQ_HISTOGRAM(name, time) UMA_HISTOGRAM_CUSTOM_TIMES( \ - name, time, base::TimeDelta::FromMilliseconds(1), \ - base::TimeDelta::FromHours(1), 50) - -void PostTimeToTypeHistogram(ModelType model_type, base::TimeDelta time) { - switch (model_type) { - case BOOKMARKS: { - SYNC_FREQ_HISTOGRAM("Sync.FreqBookmarks", time); - return; - } - case PREFERENCES: { - SYNC_FREQ_HISTOGRAM("Sync.FreqPreferences", time); - return; - } - case PASSWORDS: { - SYNC_FREQ_HISTOGRAM("Sync.FreqPasswords", time); - return; - } - case AUTOFILL: { - SYNC_FREQ_HISTOGRAM("Sync.FreqAutofill", time); - return; - } - case AUTOFILL_PROFILE: { - SYNC_FREQ_HISTOGRAM("Sync.FreqAutofillProfiles", time); - return; - } - case THEMES: { - SYNC_FREQ_HISTOGRAM("Sync.FreqThemes", time); - return; - } - case TYPED_URLS: { - SYNC_FREQ_HISTOGRAM("Sync.FreqTypedUrls", time); - return; - } - case EXTENSIONS: { - SYNC_FREQ_HISTOGRAM("Sync.FreqExtensions", time); - return; - } - case NIGORI: { - SYNC_FREQ_HISTOGRAM("Sync.FreqNigori", time); - return; - } - case SEARCH_ENGINES: { - SYNC_FREQ_HISTOGRAM("Sync.FreqSearchEngines", time); - return; - } - case SESSIONS: { - SYNC_FREQ_HISTOGRAM("Sync.FreqSessions", time); - return; - } - case APPS: { - SYNC_FREQ_HISTOGRAM("Sync.FreqApps", time); - return; - } - case APP_SETTINGS: { - SYNC_FREQ_HISTOGRAM("Sync.FreqAppSettings", time); - return; - } - case EXTENSION_SETTINGS: { - SYNC_FREQ_HISTOGRAM("Sync.FreqExtensionSettings", time); - return; - } - case APP_NOTIFICATIONS: { - SYNC_FREQ_HISTOGRAM("Sync.FreqAppNotifications", time); - return; - } - default: - LOG(ERROR) << "No known extension for model type."; - } -} - -#undef SYNC_FREQ_HISTOGRAM - // TODO(akalin): Figure out a better way to do these mappings. namespace { diff --git a/chrome/browser/sync/syncable/model_type.h b/chrome/browser/sync/syncable/model_type.h index 2669602..506de66 100644 --- a/chrome/browser/sync/syncable/model_type.h +++ b/chrome/browser/sync/syncable/model_type.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// 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. @@ -154,10 +154,6 @@ ModelTypeSet ModelTypeSetFromValue(const base::ListValue& value); // Returns a string corresponding to the syncable tag for this datatype. std::string ModelTypeToRootTag(ModelType type); -// Posts timedeltas to histogram of datatypes. Allows tracking of the frequency -// at which datatypes cause syncs. -void PostTimeToTypeHistogram(ModelType model_type, base::TimeDelta time); - // Convert a real model type to a notification type (used for // subscribing to server-issued notifications). Returns true iff // |model_type| was a real model type and |notification_type| was diff --git a/chrome/browser/sync/util/data_type_histogram.h b/chrome/browser/sync/util/data_type_histogram.h new file mode 100644 index 0000000..00478222 --- /dev/null +++ b/chrome/browser/sync/util/data_type_histogram.h @@ -0,0 +1,91 @@ +// 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. + +#ifndef CHROME_BROWSER_SYNC_UTIL_DATA_TYPE_HISTOGRAM_H_ +#define CHROME_BROWSER_SYNC_UTIL_DATA_TYPE_HISTOGRAM_H_ +#pragma once + +#include "base/metrics/histogram.h" +#include "base/time.h" +#include "chrome/browser/sync/syncable/model_type.h" + +// For now, this just implements UMA_HISTOGRAM_LONG_TIMES. This can be adjusted +// if we feel the min, max, or bucket count amount are not appropriate. +#define SYNC_FREQ_HISTOGRAM(name, time) UMA_HISTOGRAM_CUSTOM_TIMES( \ + name, time, base::TimeDelta::FromMilliseconds(1), \ + base::TimeDelta::FromHours(1), 50) + +// 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<syncable::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 +#define SYNC_DATA_TYPE_HISTOGRAM(datatype) \ + do { \ + switch (datatype) { \ + case syncable::BOOKMARKS: \ + PER_DATA_TYPE_MACRO("Bookmarks"); \ + break; \ + case syncable::PREFERENCES: \ + PER_DATA_TYPE_MACRO("Preferences"); \ + break; \ + case syncable::PASSWORDS: \ + PER_DATA_TYPE_MACRO("Passwords"); \ + break; \ + case syncable::AUTOFILL: \ + PER_DATA_TYPE_MACRO("Autofill"); \ + break; \ + case syncable::AUTOFILL_PROFILE: \ + PER_DATA_TYPE_MACRO("AutofillProfiles"); \ + break; \ + case syncable::THEMES: \ + PER_DATA_TYPE_MACRO("Themes"); \ + break; \ + case syncable::TYPED_URLS: \ + PER_DATA_TYPE_MACRO("TypedUrls"); \ + break; \ + case syncable::EXTENSIONS: \ + PER_DATA_TYPE_MACRO("Extensions"); \ + break; \ + case syncable::NIGORI: \ + PER_DATA_TYPE_MACRO("Nigori"); \ + break; \ + case syncable::SEARCH_ENGINES: \ + PER_DATA_TYPE_MACRO("SearchEngines"); \ + break; \ + case syncable::SESSIONS: \ + PER_DATA_TYPE_MACRO("Sessions"); \ + break; \ + case syncable::APPS: \ + PER_DATA_TYPE_MACRO("Apps"); \ + break; \ + case syncable::APP_SETTINGS: \ + PER_DATA_TYPE_MACRO("AppSettings"); \ + break; \ + case syncable::EXTENSION_SETTINGS: \ + PER_DATA_TYPE_MACRO("ExtensionSettings"); \ + break; \ + case syncable::APP_NOTIFICATIONS: \ + PER_DATA_TYPE_MACRO("AppNotifications"); \ + break; \ + default: \ + NOTREACHED() << "Unknown datatype " \ + << syncable::ModelTypeToString(datatype); \ + } \ + } while (0) + +#endif // CHROME_BROWSER_SYNC_UTIL_DATA_TYPE_HISTOGRAM_H_ diff --git a/chrome/browser/sync/util/data_type_histogram_unittest.cc b/chrome/browser/sync/util/data_type_histogram_unittest.cc new file mode 100644 index 0000000..6b7547e --- /dev/null +++ b/chrome/browser/sync/util/data_type_histogram_unittest.cc @@ -0,0 +1,63 @@ +// 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 "chrome/browser/sync/util/data_type_histogram.h" + +#include "base/time.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace browser_sync { +namespace { + +class DataTypeHistogramTest : public testing::Test { +}; + +// Create a histogram of type HISTOGRAM_COUNTS for each model type. Nothing +// should break. +TEST(DataTypeHistogramTest, BasicCount) { + for (int i = syncable::FIRST_REAL_MODEL_TYPE; + i <= syncable::LAST_REAL_MODEL_TYPE; ++i) { + syncable::ModelType type = syncable::ModelTypeFromInt(i); +#define PER_DATA_TYPE_MACRO(type_str) \ + HISTOGRAM_COUNTS("Prefix" type_str "Suffix", 1); + SYNC_DATA_TYPE_HISTOGRAM(type); +#undef PER_DATA_TYPE_MACRO + } +} + +// Create a histogram of type SYNC_FREQ_HISTOGRAM for each model type. Nothing +// should break. +TEST(DataTypeHistogramTest, BasicFreq) { + for (int i = syncable::FIRST_REAL_MODEL_TYPE; + i <= syncable::LAST_REAL_MODEL_TYPE; ++i) { + syncable::ModelType type = syncable::ModelTypeFromInt(i); +#define PER_DATA_TYPE_MACRO(type_str) \ + SYNC_FREQ_HISTOGRAM("Prefix" type_str "Suffix", \ + base::TimeDelta::FromSeconds(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 = syncable::FIRST_REAL_MODEL_TYPE; + i <= syncable::LAST_REAL_MODEL_TYPE; ++i) { + syncable::ModelType type = syncable::ModelTypeFromInt(i); +#define PER_DATA_TYPE_MACRO(type_str) \ + UMA_HISTOGRAM_ENUMERATION("Prefix" type_str "Suffix", \ + (i % 2 ? TYPE_1 : TYPE_2), TYPE_COUNT); + SYNC_DATA_TYPE_HISTOGRAM(type); +#undef PER_DATA_TYPE_MACRO + } +} + +} // namespace +} // namespace browser_sync diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp index 5930235..5b2adfb 100644 --- a/chrome/chrome.gyp +++ b/chrome/chrome.gyp @@ -478,6 +478,7 @@ 'browser/sync/syncable/transaction_observer.h', 'browser/sync/util/cryptographer.cc', 'browser/sync/util/cryptographer.h', + 'browser/sync/util/data_type_histogram.h', 'browser/sync/util/enum_set.h', 'browser/sync/util/extensions_activity_monitor.cc', 'browser/sync/util/extensions_activity_monitor.h', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index c56ec1a..c0efce2 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -3637,6 +3637,7 @@ 'browser/sync/test/engine/test_syncable_utils.h', 'browser/sync/test/sessions/test_scoped_session_event_listener.h', 'browser/sync/util/data_encryption_unittest.cc', + 'browser/sync/util/data_type_histogram_unittest.cc', 'browser/sync/util/enum_set_unittest.cc', 'browser/sync/util/protobuf_unittest.cc', 'browser/sync/util/immutable_unittest.cc', |