Commit 4345a7dc authored by Thanh Nguyen's avatar Thanh Nguyen Committed by Commit Bot

[local-search-service] Rename SearchMetricReporter files

Rename SearchMetricsReporter to SearchMetricsReporterSync. We need to do
this to make the metrics reporter working while we make changes to
local search service. See go/lss-sandboxing-impl for mor details.

Bug: 1137560
Change-Id: I38300e031cdfd6f9f11c33fe320705845087c9b3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2469648Reviewed-by: default avatarJia Meng <jiameng@chromium.org>
Reviewed-by: default avatarColin Blundell <blundell@chromium.org>
Commit-Queue: Thanh Nguyen <thanhdng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817443}
parent 502aadea
...@@ -339,7 +339,7 @@ ...@@ -339,7 +339,7 @@
#include "chrome/browser/upgrade_detector/upgrade_detector_chromeos.h" #include "chrome/browser/upgrade_detector/upgrade_detector_chromeos.h"
#include "chromeos/audio/audio_devices_pref_handler_impl.h" #include "chromeos/audio/audio_devices_pref_handler_impl.h"
#include "chromeos/components/account_manager/account_manager.h" #include "chromeos/components/account_manager/account_manager.h"
#include "chromeos/components/local_search_service/search_metrics_reporter.h" #include "chromeos/components/local_search_service/search_metrics_reporter_sync.h"
#include "chromeos/components/quick_answers/public/cpp/quick_answers_prefs.h" #include "chromeos/components/quick_answers/public/cpp/quick_answers_prefs.h"
#include "chromeos/constants/chromeos_switches.h" #include "chromeos/constants/chromeos_switches.h"
#include "chromeos/network/fast_transition_observer.h" #include "chromeos/network/fast_transition_observer.h"
...@@ -654,7 +654,7 @@ void RegisterLocalState(PrefRegistrySimple* registry) { ...@@ -654,7 +654,7 @@ void RegisterLocalState(PrefRegistrySimple* registry) {
chromeos::KioskAppManager::RegisterPrefs(registry); chromeos::KioskAppManager::RegisterPrefs(registry);
chromeos::KioskCryptohomeRemover::RegisterPrefs(registry); chromeos::KioskCryptohomeRemover::RegisterPrefs(registry);
chromeos::language_prefs::RegisterPrefs(registry); chromeos::language_prefs::RegisterPrefs(registry);
chromeos::local_search_service::SearchMetricsReporter:: chromeos::local_search_service::SearchMetricsReporterSync::
RegisterLocalStatePrefs(registry); RegisterLocalStatePrefs(registry);
chromeos::MultiProfileUserController::RegisterPrefs(registry); chromeos::MultiProfileUserController::RegisterPrefs(registry);
chromeos::NetworkMetadataStore::RegisterPrefs(registry); chromeos::NetworkMetadataStore::RegisterPrefs(registry);
......
...@@ -22,8 +22,8 @@ source_set("local_search_service") { ...@@ -22,8 +22,8 @@ source_set("local_search_service") {
"local_search_service_sync_factory.h", "local_search_service_sync_factory.h",
"pref_names.cc", "pref_names.cc",
"pref_names.h", "pref_names.h",
"search_metrics_reporter.cc", "search_metrics_reporter_sync.cc",
"search_metrics_reporter.h", "search_metrics_reporter_sync.h",
"search_utils.cc", "search_utils.cc",
"search_utils.h", "search_utils.h",
"shared_structs.cc", "shared_structs.cc",
...@@ -94,7 +94,7 @@ source_set("unit_tests") { ...@@ -94,7 +94,7 @@ source_set("unit_tests") {
"linear_map_search_unittest.cc", "linear_map_search_unittest.cc",
"local_search_service_proxy_unittest.cc", "local_search_service_proxy_unittest.cc",
"local_search_service_sync_unittest.cc", "local_search_service_sync_unittest.cc",
"search_metrics_reporter_unittest.cc", "search_metrics_reporter_sync_unittest.cc",
"search_utils_unittest.cc", "search_utils_unittest.cc",
] ]
......
...@@ -20,7 +20,7 @@ void LogIndexIdAndBackendType(const std::string& histogram_prefix, ...@@ -20,7 +20,7 @@ void LogIndexIdAndBackendType(const std::string& histogram_prefix,
} }
std::string IndexIdBasedHistogramPrefix(IndexId index_id) { std::string IndexIdBasedHistogramPrefix(IndexId index_id) {
const std::string prefix = "LocalSearchServiceSync."; const std::string prefix = "LocalSearchService.";
switch (index_id) { switch (index_id) {
case IndexId::kCrosSettings: case IndexId::kCrosSettings:
return prefix + "CrosSettings"; return prefix + "CrosSettings";
...@@ -42,7 +42,7 @@ IndexSync::IndexSync(IndexId index_id, ...@@ -42,7 +42,7 @@ IndexSync::IndexSync(IndexId index_id,
return; return;
} }
reporter_ = std::make_unique<SearchMetricsReporter>(local_state); reporter_ = std::make_unique<SearchMetricsReporterSync>(local_state);
DCHECK(reporter_); DCHECK(reporter_);
reporter_->SetIndexId(index_id); reporter_->SetIndexId(index_id);
} }
......
...@@ -15,7 +15,7 @@ ...@@ -15,7 +15,7 @@
#include "base/optional.h" #include "base/optional.h"
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "chromeos/components/local_search_service/search_metrics_reporter.h" #include "chromeos/components/local_search_service/search_metrics_reporter_sync.h"
#include "chromeos/components/local_search_service/shared_structs.h" #include "chromeos/components/local_search_service/shared_structs.h"
class PrefService; class PrefService;
...@@ -78,7 +78,7 @@ class IndexSync { ...@@ -78,7 +78,7 @@ class IndexSync {
private: private:
std::string histogram_prefix_; std::string histogram_prefix_;
std::unique_ptr<SearchMetricsReporter> reporter_; std::unique_ptr<SearchMetricsReporterSync> reporter_;
base::WeakPtrFactory<IndexSync> weak_ptr_factory_{this}; base::WeakPtrFactory<IndexSync> weak_ptr_factory_{this};
}; };
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#include "chromeos/components/local_search_service/search_metrics_reporter.h" #include "chromeos/components/local_search_service/search_metrics_reporter_sync.h"
#include "base/check_op.h" #include "base/check_op.h"
#include "base/metrics/histogram_functions.h" #include "base/metrics/histogram_functions.h"
...@@ -20,33 +20,33 @@ constexpr base::TimeDelta kCheckDailyEventInternal = ...@@ -20,33 +20,33 @@ constexpr base::TimeDelta kCheckDailyEventInternal =
base::TimeDelta::FromMinutes(30); base::TimeDelta::FromMinutes(30);
// Prefs corresponding to IndexId values. // Prefs corresponding to IndexId values.
constexpr std::array<const char*, SearchMetricsReporter::kNumberIndexIds> constexpr std::array<const char*, SearchMetricsReporterSync::kNumberIndexIds>
kDailyCountPrefs = { kDailyCountPrefs = {
prefs::kLocalSearchServiceMetricsCrosSettingsCount, prefs::kLocalSearchServiceMetricsCrosSettingsCount,
prefs::kLocalSearchServiceMetricsHelpAppCount, prefs::kLocalSearchServiceMetricsHelpAppCount,
}; };
// Histograms corresponding to IndexId values. // Histograms corresponding to IndexId values.
constexpr std::array<const char*, SearchMetricsReporter::kNumberIndexIds> constexpr std::array<const char*, SearchMetricsReporterSync::kNumberIndexIds>
kDailyCountHistograms = { kDailyCountHistograms = {
SearchMetricsReporter::kCrosSettingsName, SearchMetricsReporterSync::kCrosSettingsName,
SearchMetricsReporter::kHelpAppName, SearchMetricsReporterSync::kHelpAppName,
}; };
} // namespace } // namespace
constexpr char SearchMetricsReporter::kDailyEventIntervalName[]; constexpr char SearchMetricsReporterSync::kDailyEventIntervalName[];
constexpr char SearchMetricsReporter::kCrosSettingsName[]; constexpr char SearchMetricsReporterSync::kCrosSettingsName[];
constexpr char SearchMetricsReporter::kHelpAppName[]; constexpr char SearchMetricsReporterSync::kHelpAppName[];
constexpr int SearchMetricsReporter::kNumberIndexIds; constexpr int SearchMetricsReporterSync::kNumberIndexIds;
// This class is needed since metrics::DailyEvent requires taking ownership // This class is needed since metrics::DailyEvent requires taking ownership
// of its observers. It just forwards events to SearchMetricsReporter. // of its observers. It just forwards events to SearchMetricsReporter.
class SearchMetricsReporter::DailyEventObserver class SearchMetricsReporterSync::DailyEventObserver
: public metrics::DailyEvent::Observer { : public metrics::DailyEvent::Observer {
public: public:
explicit DailyEventObserver(SearchMetricsReporter* reporter) explicit DailyEventObserver(SearchMetricsReporterSync* reporter)
: reporter_(reporter) { : reporter_(reporter) {
DCHECK(reporter_); DCHECK(reporter_);
} }
...@@ -61,11 +61,11 @@ class SearchMetricsReporter::DailyEventObserver ...@@ -61,11 +61,11 @@ class SearchMetricsReporter::DailyEventObserver
} }
private: private:
SearchMetricsReporter* reporter_; // Not owned. SearchMetricsReporterSync* reporter_; // Not owned.
}; };
// static: // static:
void SearchMetricsReporter::RegisterLocalStatePrefs( void SearchMetricsReporterSync::RegisterLocalStatePrefs(
PrefRegistrySimple* registry) { PrefRegistrySimple* registry) {
metrics::DailyEvent::RegisterPref( metrics::DailyEvent::RegisterPref(
registry, prefs::kLocalSearchServiceMetricsDailySample); registry, prefs::kLocalSearchServiceMetricsDailySample);
...@@ -74,7 +74,7 @@ void SearchMetricsReporter::RegisterLocalStatePrefs( ...@@ -74,7 +74,7 @@ void SearchMetricsReporter::RegisterLocalStatePrefs(
} }
} }
SearchMetricsReporter::SearchMetricsReporter( SearchMetricsReporterSync::SearchMetricsReporterSync(
PrefService* local_state_pref_service) PrefService* local_state_pref_service)
: pref_service_(local_state_pref_service), : pref_service_(local_state_pref_service),
daily_event_(std::make_unique<metrics::DailyEvent>( daily_event_(std::make_unique<metrics::DailyEvent>(
...@@ -91,15 +91,15 @@ SearchMetricsReporter::SearchMetricsReporter( ...@@ -91,15 +91,15 @@ SearchMetricsReporter::SearchMetricsReporter(
&metrics::DailyEvent::CheckInterval); &metrics::DailyEvent::CheckInterval);
} }
SearchMetricsReporter::~SearchMetricsReporter() = default; SearchMetricsReporterSync::~SearchMetricsReporterSync() = default;
void SearchMetricsReporter::SetIndexId(IndexId index_id) { void SearchMetricsReporterSync::SetIndexId(IndexId index_id) {
DCHECK(!index_id_); DCHECK(!index_id_);
index_id_ = index_id; index_id_ = index_id;
DCHECK_LT(static_cast<size_t>(index_id), kDailyCountPrefs.size()); DCHECK_LT(static_cast<size_t>(index_id), kDailyCountPrefs.size());
} }
void SearchMetricsReporter::OnSearchPerformed() { void SearchMetricsReporterSync::OnSearchPerformed() {
DCHECK(index_id_); DCHECK(index_id_);
const size_t index = static_cast<size_t>(*index_id_); const size_t index = static_cast<size_t>(*index_id_);
const char* daily_count_pref = kDailyCountPrefs[index]; const char* daily_count_pref = kDailyCountPrefs[index];
...@@ -107,12 +107,12 @@ void SearchMetricsReporter::OnSearchPerformed() { ...@@ -107,12 +107,12 @@ void SearchMetricsReporter::OnSearchPerformed() {
pref_service_->SetInteger(daily_count_pref, daily_counts_[index]); pref_service_->SetInteger(daily_count_pref, daily_counts_[index]);
} }
void SearchMetricsReporter::ReportDailyMetricsForTesting( void SearchMetricsReporterSync::ReportDailyMetricsForTesting(
metrics::DailyEvent::IntervalType type) { metrics::DailyEvent::IntervalType type) {
ReportDailyMetrics(type); ReportDailyMetrics(type);
} }
void SearchMetricsReporter::ReportDailyMetrics( void SearchMetricsReporterSync::ReportDailyMetrics(
metrics::DailyEvent::IntervalType type) { metrics::DailyEvent::IntervalType type) {
if (!index_id_) if (!index_id_)
return; return;
......
...@@ -2,8 +2,8 @@ ...@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#ifndef CHROMEOS_COMPONENTS_LOCAL_SEARCH_SERVICE_SEARCH_METRICS_REPORTER_H_ #ifndef CHROMEOS_COMPONENTS_LOCAL_SEARCH_SERVICE_SEARCH_METRICS_REPORTER_SYNC_H_
#define CHROMEOS_COMPONENTS_LOCAL_SEARCH_SERVICE_SEARCH_METRICS_REPORTER_H_ #define CHROMEOS_COMPONENTS_LOCAL_SEARCH_SERVICE_SEARCH_METRICS_REPORTER_SYNC_H_
#include <array> #include <array>
#include <memory> #include <memory>
...@@ -22,7 +22,7 @@ namespace chromeos { ...@@ -22,7 +22,7 @@ namespace chromeos {
namespace local_search_service { namespace local_search_service {
// SearchMetricsReporter logs daily search requests to UMA. // SearchMetricsReporter logs daily search requests to UMA.
class SearchMetricsReporter { class SearchMetricsReporterSync {
public: public:
static constexpr int kNumberIndexIds = static constexpr int kNumberIndexIds =
static_cast<int>(IndexId::kMaxValue) + 1; static_cast<int>(IndexId::kMaxValue) + 1;
...@@ -42,11 +42,12 @@ class SearchMetricsReporter { ...@@ -42,11 +42,12 @@ class SearchMetricsReporter {
static void RegisterLocalStatePrefs(PrefRegistrySimple* registry); static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
// RegisterLocalStatePrefs() must be called before instantiating this class. // RegisterLocalStatePrefs() must be called before instantiating this class.
explicit SearchMetricsReporter(PrefService* local_state_pref_service); explicit SearchMetricsReporterSync(PrefService* local_state_pref_service);
~SearchMetricsReporter(); ~SearchMetricsReporterSync();
SearchMetricsReporter(const SearchMetricsReporter&) = delete; SearchMetricsReporterSync(const SearchMetricsReporterSync&) = delete;
SearchMetricsReporter& operator=(const SearchMetricsReporter&) = delete; SearchMetricsReporterSync& operator=(const SearchMetricsReporterSync&) =
delete;
// Sets |index_id_|. // Sets |index_id_|.
void SetIndexId(IndexId index_id); void SetIndexId(IndexId index_id);
...@@ -83,4 +84,4 @@ class SearchMetricsReporter { ...@@ -83,4 +84,4 @@ class SearchMetricsReporter {
} // namespace local_search_service } // namespace local_search_service
} // namespace chromeos } // namespace chromeos
#endif // CHROMEOS_COMPONENTS_LOCAL_SEARCH_SERVICE_SEARCH_METRICS_REPORTER_H_ #endif // CHROMEOS_COMPONENTS_LOCAL_SEARCH_SERVICE_SEARCH_METRICS_REPORTER_SYNC_H_
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#include "chromeos/components/local_search_service/search_metrics_reporter.h" #include "chromeos/components/local_search_service/search_metrics_reporter_sync.h"
#include <memory> #include <memory>
...@@ -18,20 +18,21 @@ ...@@ -18,20 +18,21 @@
namespace chromeos { namespace chromeos {
namespace local_search_service { namespace local_search_service {
class SearchMetricsReporterTest : public testing::Test { class SearchMetricsReporterSyncTest : public testing::Test {
public: public:
SearchMetricsReporterTest() = default; SearchMetricsReporterSyncTest() = default;
~SearchMetricsReporterTest() override = default; ~SearchMetricsReporterSyncTest() override = default;
void SetUp() override { void SetUp() override {
SearchMetricsReporter::RegisterLocalStatePrefs(pref_service_.registry()); SearchMetricsReporterSync::RegisterLocalStatePrefs(
pref_service_.registry());
} }
void TearDown() override { reporter_.reset(); } void TearDown() override { reporter_.reset(); }
protected: protected:
void SetReporter(IndexId index_id) { void SetReporter(IndexId index_id) {
reporter_ = std::make_unique<SearchMetricsReporter>(&pref_service_); reporter_ = std::make_unique<SearchMetricsReporterSync>(&pref_service_);
reporter_->SetIndexId(index_id); reporter_->SetIndexId(index_id);
} }
...@@ -55,50 +56,50 @@ class SearchMetricsReporterTest : public testing::Test { ...@@ -55,50 +56,50 @@ class SearchMetricsReporterTest : public testing::Test {
base::test::TaskEnvironment task_environment_; base::test::TaskEnvironment task_environment_;
TestingPrefServiceSimple pref_service_; TestingPrefServiceSimple pref_service_;
std::unique_ptr<SearchMetricsReporter> reporter_; std::unique_ptr<SearchMetricsReporterSync> reporter_;
}; };
TEST_F(SearchMetricsReporterTest, CountAndReportEvents) { TEST_F(SearchMetricsReporterSyncTest, CountAndReportEvents) {
SetReporter(IndexId::kCrosSettings); SetReporter(IndexId::kCrosSettings);
SendOnSearchPerformed(); SendOnSearchPerformed();
SendOnSearchPerformed(); SendOnSearchPerformed();
SendOnSearchPerformed(); SendOnSearchPerformed();
TriggerDailyEventAndVerifyHistograms(SearchMetricsReporter::kCrosSettingsName, TriggerDailyEventAndVerifyHistograms(
3); SearchMetricsReporterSync::kCrosSettingsName, 3);
// The next day, another two searches. // The next day, another two searches.
SendOnSearchPerformed(); SendOnSearchPerformed();
SendOnSearchPerformed(); SendOnSearchPerformed();
TriggerDailyEventAndVerifyHistograms(SearchMetricsReporter::kCrosSettingsName, TriggerDailyEventAndVerifyHistograms(
2); SearchMetricsReporterSync::kCrosSettingsName, 2);
} }
TEST_F(SearchMetricsReporterTest, LoadInitialCountsFromPrefs) { TEST_F(SearchMetricsReporterSyncTest, LoadInitialCountsFromPrefs) {
// Create a new reporter and check that it loads its initial event counts from // Create a new reporter and check that it loads its initial event counts from
// prefs. // prefs.
pref_service_.SetInteger(prefs::kLocalSearchServiceMetricsCrosSettingsCount, pref_service_.SetInteger(prefs::kLocalSearchServiceMetricsCrosSettingsCount,
2); 2);
SetReporter(IndexId::kCrosSettings); SetReporter(IndexId::kCrosSettings);
TriggerDailyEventAndVerifyHistograms(SearchMetricsReporter::kCrosSettingsName, TriggerDailyEventAndVerifyHistograms(
2); SearchMetricsReporterSync::kCrosSettingsName, 2);
// The previous report should've cleared the prefs, so a new reporter should // The previous report should've cleared the prefs, so a new reporter should
// start out at zero. // start out at zero.
TriggerDailyEventAndVerifyHistograms(SearchMetricsReporter::kCrosSettingsName, TriggerDailyEventAndVerifyHistograms(
0); SearchMetricsReporterSync::kCrosSettingsName, 0);
} }
TEST_F(SearchMetricsReporterTest, IgnoreDailyEventFirstRun) { TEST_F(SearchMetricsReporterSyncTest, IgnoreDailyEventFirstRun) {
SetReporter(IndexId::kCrosSettings); SetReporter(IndexId::kCrosSettings);
// metrics::DailyEvent notifies observers immediately on first run. Histograms // metrics::DailyEvent notifies observers immediately on first run. Histograms
// shouldn't be sent in this case. // shouldn't be sent in this case.
base::HistogramTester tester; base::HistogramTester tester;
TriggerDailyEvent(metrics::DailyEvent::IntervalType::FIRST_RUN); TriggerDailyEvent(metrics::DailyEvent::IntervalType::FIRST_RUN);
tester.ExpectTotalCount(SearchMetricsReporter::kCrosSettingsName, 0); tester.ExpectTotalCount(SearchMetricsReporterSync::kCrosSettingsName, 0);
} }
TEST_F(SearchMetricsReporterTest, IgnoreDailyEventClockChanged) { TEST_F(SearchMetricsReporterSyncTest, IgnoreDailyEventClockChanged) {
SetReporter(IndexId::kCrosSettings); SetReporter(IndexId::kCrosSettings);
SendOnSearchPerformed(); SendOnSearchPerformed();
...@@ -106,12 +107,12 @@ TEST_F(SearchMetricsReporterTest, IgnoreDailyEventClockChanged) { ...@@ -106,12 +107,12 @@ TEST_F(SearchMetricsReporterTest, IgnoreDailyEventClockChanged) {
// jumped back. Histograms shouldn't be sent in this case. // jumped back. Histograms shouldn't be sent in this case.
base::HistogramTester tester; base::HistogramTester tester;
TriggerDailyEvent(metrics::DailyEvent::IntervalType::CLOCK_CHANGED); TriggerDailyEvent(metrics::DailyEvent::IntervalType::CLOCK_CHANGED);
tester.ExpectTotalCount(SearchMetricsReporter::kCrosSettingsName, 0); tester.ExpectTotalCount(SearchMetricsReporterSync::kCrosSettingsName, 0);
// The existing stats should be cleared when the clock change notification is // The existing stats should be cleared when the clock change notification is
// received, so the next report should only contain zeros. // received, so the next report should only contain zeros.
TriggerDailyEventAndVerifyHistograms(SearchMetricsReporter::kCrosSettingsName, TriggerDailyEventAndVerifyHistograms(
0); SearchMetricsReporterSync::kCrosSettingsName, 0);
} }
} // namespace local_search_service } // namespace local_search_service
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment