[M116] The signal handler observes all necessary histogram with type

If the same histogram is used as enum and value, then the handler only
stores value and not enum. This affects the aggregates depending on
when a new models with the value type was added. Make sure the handler
stores the histogram with all the necessary types.

(cherry picked from commit 4e0a7fb)

Bug: 1456749
Change-Id: Ia9182545f33116f3922e4f27ee36334f937fd7c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4632397
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1160943}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4641629
Cr-Commit-Position: refs/branch-heads/5845@{#107}
Cr-Branched-From: 5a5dff6-refs/heads/main@{#1160321}

components/segmentation_platform/internal/signals/histogram_signal_handler.cc

@@ -30,7 +30,7 @@ void HistogramSignalHandler::SetRelevantHistograms(
         histogram_name,
         base::BindRepeating(&HistogramSignalHandler::OnHistogramSample,
                             weak_ptr_factory_.GetWeakPtr(), signal_type));
-    histogram_observers_[histogram_name] = std::move(histogram_observer);
+    histogram_observers_[pair] = std::move(histogram_observer);
   }
 }
 

components/segmentation_platform/internal/signals/histogram_signal_handler.h

@@ -78,8 +78,9 @@ class HistogramSignalHandler {
 
   // Tracks the histogram names we are currently listening to along with their
   // corresponding observers.
+  using HistogramSignal = std::pair<std::string, proto::SignalType>;
   std::map<
-      std::string,
+      HistogramSignal,
       std::unique_ptr<base::StatisticsRecorder::ScopedHistogramSampleObserver>>
       histogram_observers_;
 

components/segmentation_platform/internal/signals/histogram_signal_handler_unittest.cc

@@ -79,6 +79,28 @@ TEST_F(HistogramSignalHandlerTest, HistogramsAreRecorded) {
   task_environment_.RunUntilIdle();
 }
 
+TEST_F(HistogramSignalHandlerTest, HistogramAsValueAndEnum) {
+  histogram_signal_handler_->EnableMetrics(true);
+
+  // Add the same histogram as enum and as value type, the observer should write
+  // 2 samples to database.
+  std::set<std::pair<std::string, proto::SignalType>> histograms;
+  histograms.insert(
+      std::make_pair(kExpectedHistogram, proto::SignalType::HISTOGRAM_ENUM));
+  histograms.insert(
+      std::make_pair(kExpectedHistogram, proto::SignalType::HISTOGRAM_VALUE));
+  histogram_signal_handler_->SetRelevantHistograms(histograms);
+
+  // Record a registered histogram sample. It should be recorded.
+  EXPECT_CALL(*signal_database_, WriteSample(proto::SignalType::HISTOGRAM_ENUM,
+                                             kExpectedHash, Eq(1), _));
+  EXPECT_CALL(*signal_database_, WriteSample(proto::SignalType::HISTOGRAM_VALUE,
+                                             kExpectedHash, Eq(1), _));
+
+  UMA_HISTOGRAM_BOOLEAN(kExpectedHistogram, true);
+  task_environment_.RunUntilIdle();
+}
+
 TEST_F(HistogramSignalHandlerTest, DisableMetrics) {
   SetupHistograms();