Segmentation platform: Add observer for histogram event.
Histogram events will be propagated after db is updated, so other
systems can listen to it and safely use the db data. The caching
functionality of FeatureListQueryProcessor and TrainingDataCollector
needs to listen to these events.
Bug: 1295447
Change-Id: Ia2f8bf07affebe942119b5cd3498b9cc9cd25b7b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3469772
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#972124}
diff --git a/components/segmentation_platform/internal/signals/histogram_signal_handler.cc b/components/segmentation_platform/internal/signals/histogram_signal_handler.cc
index 933794c..f2adb38 100644
--- a/components/segmentation_platform/internal/signals/histogram_signal_handler.cc
+++ b/components/segmentation_platform/internal/signals/histogram_signal_handler.cc
@@ -4,6 +4,7 @@
#include "components/segmentation_platform/internal/signals/histogram_signal_handler.h"
+#include "base/bind.h"
#include "base/callback_helpers.h"
#include "base/metrics/metrics_hashes.h"
#include "components/segmentation_platform/internal/database/signal_database.h"
@@ -45,7 +46,27 @@
if (!metrics_enabled_)
return;
- db_->WriteSample(signal_type, name_hash, sample, base::DoNothing());
+ db_->WriteSample(signal_type, name_hash, sample,
+ base::BindOnce(&HistogramSignalHandler::OnSampleWritten,
+ weak_ptr_factory_.GetWeakPtr(),
+ std::string(histogram_name)));
+}
+
+void HistogramSignalHandler::AddObserver(Observer* observer) {
+ observers_.AddObserver(observer);
+}
+
+void HistogramSignalHandler::RemoveObserver(Observer* observer) {
+ observers_.RemoveObserver(observer);
+}
+
+void HistogramSignalHandler::OnSampleWritten(const std::string& histogram_name,
+ bool success) {
+ if (!success)
+ return;
+
+ for (Observer& ob : observers_)
+ ob.OnHistogramSignalUpdated(histogram_name);
}
} // namespace segmentation_platform
diff --git a/components/segmentation_platform/internal/signals/histogram_signal_handler.h b/components/segmentation_platform/internal/signals/histogram_signal_handler.h
index 6144992..a704990 100644
--- a/components/segmentation_platform/internal/signals/histogram_signal_handler.h
+++ b/components/segmentation_platform/internal/signals/histogram_signal_handler.h
@@ -16,6 +16,7 @@
#include "base/memory/weak_ptr.h"
#include "base/metrics/histogram_base.h"
#include "base/metrics/statistics_recorder.h"
+#include "base/observer_list.h"
#include "components/segmentation_platform/internal/proto/types.pb.h"
namespace segmentation_platform {
@@ -26,6 +27,18 @@
// persisting them to the internal database for future processing.
class HistogramSignalHandler {
public:
+ class Observer : public base::CheckedObserver {
+ public:
+ // Called when a histogram signal tracked by segmentation platform is
+ // updated and written to database.
+ virtual void OnHistogramSignalUpdated(
+ const std::string& histogram_name) = 0;
+ ~Observer() override = default;
+
+ protected:
+ Observer() = default;
+ };
+
explicit HistogramSignalHandler(SignalDatabase* signal_database);
virtual ~HistogramSignalHandler();
@@ -41,12 +54,18 @@
// Called to enable or disable metrics collection for segmentation platform.
virtual void EnableMetrics(bool enable_metrics);
+ // Add/Remove observer for histogram update events.
+ virtual void AddObserver(Observer* observer);
+ virtual void RemoveObserver(Observer* observer);
+
private:
void OnHistogramSample(proto::SignalType signal_type,
const char* histogram_name,
uint64_t name_hash,
base::HistogramBase::Sample sample);
+ void OnSampleWritten(const std::string& histogram_name, bool success);
+
// The database storing relevant histogram samples.
raw_ptr<SignalDatabase> db_;
@@ -60,6 +79,8 @@
std::unique_ptr<base::StatisticsRecorder::ScopedHistogramSampleObserver>>
histogram_observers_;
+ base::ObserverList<Observer> observers_;
+
base::WeakPtrFactory<HistogramSignalHandler> weak_ptr_factory_{this};
};
diff --git a/components/segmentation_platform/internal/signals/histogram_signal_handler_unittest.cc b/components/segmentation_platform/internal/signals/histogram_signal_handler_unittest.cc
index 28a06f6..a6bc8ae 100644
--- a/components/segmentation_platform/internal/signals/histogram_signal_handler_unittest.cc
+++ b/components/segmentation_platform/internal/signals/histogram_signal_handler_unittest.cc
@@ -12,8 +12,10 @@
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
-using testing::_;
-using testing::Eq;
+using ::testing::_;
+using ::testing::Eq;
+using ::testing::Invoke;
+using ::testing::WithArgs;
namespace segmentation_platform {
@@ -22,7 +24,12 @@
constexpr char kExpectedHistogram[] = "some_histogram";
const uint64_t kExpectedHash = base::HashMetricName(kExpectedHistogram);
-} // namespace
+class MockObserver : public HistogramSignalHandler::Observer {
+ public:
+ MockObserver() = default;
+ ~MockObserver() override = default;
+ MOCK_METHOD(void, OnHistogramSignalUpdated, (const std::string&), (override));
+};
class HistogramSignalHandlerTest : public testing::Test {
public:
@@ -45,6 +52,7 @@
base::test::TaskEnvironment task_environment_;
std::unique_ptr<MockSignalDatabase> signal_database_;
std::unique_ptr<HistogramSignalHandler> histogram_signal_handler_;
+ MockObserver observer_;
};
TEST_F(HistogramSignalHandlerTest, HistogramsAreRecorded) {
@@ -104,4 +112,26 @@
task_environment_.RunUntilIdle();
}
+TEST_F(HistogramSignalHandlerTest, ObserversNotified) {
+ histogram_signal_handler_->EnableMetrics(true);
+ SetupHistograms();
+ histogram_signal_handler_->AddObserver(&observer_);
+
+ EXPECT_CALL(*signal_database_, WriteSample(proto::SignalType::HISTOGRAM_ENUM,
+ kExpectedHash, Eq(1), _))
+ .WillOnce(
+ WithArgs<3>(Invoke([](MockSignalDatabase::SuccessCallback callback) {
+ std::move(callback).Run(true);
+ })));
+ EXPECT_CALL(observer_,
+ OnHistogramSignalUpdated(std::string(kExpectedHistogram)));
+
+ // Record a registered histogram sample. |observer_| should be notified.
+ UMA_HISTOGRAM_BOOLEAN(kExpectedHistogram, true);
+ task_environment_.RunUntilIdle();
+
+ histogram_signal_handler_->RemoveObserver(&observer_);
+}
+
+} // namespace
} // namespace segmentation_platform