[privacy_budget] Some renames for readability.

We are using the word "sample" in too many ways. Try to move to less
ambiguous terminology.

Bug: 973801
Change-Id: I4d1feeeef22e756e82952e4b53a2a6615dac8715
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2510240
Auto-Submit: Asanka Herath <asanka@chromium.org>
Commit-Queue: Alex Turner <alexmt@chromium.org>
Reviewed-by: Alex Turner <alexmt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823303}

chrome/browser/privacy_budget/BUILD.gn

@@ -4,10 +4,10 @@
 
 source_set("headers") {
   sources = [
+    "encountered_surface_tracker.h",
     "identifiability_study_state.h",
     "privacy_budget_prefs.h",
     "privacy_budget_ukm_entry_filter.h",
-    "sampled_surface_tracker.h",
   ]
 
   deps = [
@@ -21,10 +21,10 @@ source_set("headers") {
 
 source_set("privacy_budget") {
   sources = [
+    "encountered_surface_tracker.cc",
     "identifiability_study_state.cc",
     "privacy_budget_prefs.cc",
     "privacy_budget_ukm_entry_filter.cc",
-    "sampled_surface_tracker.cc",
   ]
 
   public_deps = [ ":headers" ]
@@ -45,9 +45,9 @@ source_set("unit_tests") {
   testonly = true
 
   sources = [
+    "encountered_surface_tracker_unittest.cc",
     "identifiability_study_state_unittest.cc",
     "privacy_budget_ukm_entry_filter_unittest.cc",
-    "sampled_surface_tracker_unittest.cc",
   ]
 
   deps = [

chrome/browser/privacy_budget/sampled_surface_tracker.cc → chrome/browser/privacy_budget/encountered_surface_tracker.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/privacy_budget/sampled_surface_tracker.h"
+#include "chrome/browser/privacy_budget/encountered_surface_tracker.h"
 
 #include <vector>
 
@@ -28,20 +28,21 @@ uint64_t hash(uint64_t x) {
 
 }  // namespace
 
-const unsigned SampledSurfaceTracker::kMaxTrackedSurfaces;
+const unsigned EncounteredSurfaceTracker::kMaxTrackedSurfaces;
 
-SampledSurfaceTracker::SampledSurfaceTracker() {
+EncounteredSurfaceTracker::EncounteredSurfaceTracker() {
   Reset();
 }
 
-SampledSurfaceTracker::~SampledSurfaceTracker() = default;
+EncounteredSurfaceTracker::~EncounteredSurfaceTracker() = default;
 
-void SampledSurfaceTracker::Reset() {
+void EncounteredSurfaceTracker::Reset() {
   seed_ = base::RandUint64();
   surfaces_.clear();
 }
 
-bool SampledSurfaceTracker::ShouldRecord(uint64_t source_id, uint64_t surface) {
+bool EncounteredSurfaceTracker::IsNewEncounter(uint64_t source_id,
+                                               uint64_t surface) {
   if (blink::IdentifiableSurface::FromMetricHash(surface).GetType() ==
       blink::IdentifiableSurface::Type::kReservedInternal)
     return false;

chrome/browser/privacy_budget/sampled_surface_tracker.h → chrome/browser/privacy_budget/encountered_surface_tracker.h

@@ -2,24 +2,24 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_PRIVACY_BUDGET_SAMPLED_SURFACE_TRACKER_H_
-#define CHROME_BROWSER_PRIVACY_BUDGET_SAMPLED_SURFACE_TRACKER_H_
+#ifndef CHROME_BROWSER_PRIVACY_BUDGET_ENCOUNTERED_SURFACE_TRACKER_H_
+#define CHROME_BROWSER_PRIVACY_BUDGET_ENCOUNTERED_SURFACE_TRACKER_H_
 
 #include <map>
 
 #include "base/containers/flat_set.h"
 #include "base/time/time.h"
 
-class SampledSurfaceTracker {
+class EncounteredSurfaceTracker {
  public:
   // Maximum number of surfaces that this class can track. Prevents unbounded
   // memory growth.
   static constexpr unsigned kMaxTrackedSurfaces = 1000;
 
-  SampledSurfaceTracker();
-  ~SampledSurfaceTracker();
+  EncounteredSurfaceTracker();
+  ~EncounteredSurfaceTracker();
 
-  bool ShouldRecord(uint64_t source_id, uint64_t surface);
+  bool IsNewEncounter(uint64_t source_id, uint64_t surface);
 
   void Reset();
 
@@ -30,4 +30,4 @@ class SampledSurfaceTracker {
   uint64_t seed_;
 };
 
-#endif  // CHROME_BROWSER_PRIVACY_BUDGET_SAMPLED_SURFACE_TRACKER_H_
+#endif  // CHROME_BROWSER_PRIVACY_BUDGET_ENCOUNTERED_SURFACE_TRACKER_H_

chrome/browser/privacy_budget/sampled_surface_tracker_unittest.cc → chrome/browser/privacy_budget/encountered_surface_tracker_unittest.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/privacy_budget/sampled_surface_tracker.h"
+#include "chrome/browser/privacy_budget/encountered_surface_tracker.h"
 
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/common/privacy_budget/identifiable_surface.h"
@@ -15,48 +15,50 @@ uint64_t metric(int i) {
 }
 }  // namespace
 
-TEST(SampledSurfaceTrackerTest, Dedup) {
-  SampledSurfaceTracker t;
-  EXPECT_TRUE(t.ShouldRecord(0, metric(1)));
-  EXPECT_FALSE(t.ShouldRecord(0, metric(1)));
-  EXPECT_TRUE(t.ShouldRecord(1, metric(1)));
+TEST(EncounteredSurfaceTrackerTest, Dedup) {
+  EncounteredSurfaceTracker t;
+  EXPECT_TRUE(t.IsNewEncounter(0, metric(1)));
+  EXPECT_FALSE(t.IsNewEncounter(0, metric(1)));
+  EXPECT_TRUE(t.IsNewEncounter(1, metric(1)));
 }
 
-TEST(SampledSurfaceTrackerTest, SizeLimit) {
-  SampledSurfaceTracker t;
-  for (uint64_t i = 0; i < SampledSurfaceTracker::kMaxTrackedSurfaces; i++) {
-    EXPECT_TRUE(t.ShouldRecord(0, metric(i))) << ": 0," << i;
+TEST(EncounteredSurfaceTrackerTest, SizeLimit) {
+  EncounteredSurfaceTracker t;
+  for (uint64_t i = 0; i < EncounteredSurfaceTracker::kMaxTrackedSurfaces;
+       i++) {
+    EXPECT_TRUE(t.IsNewEncounter(0, metric(i))) << ": 0," << i;
   }
   // Now the tracker should be full, but we will still allow new sources.
-  for (uint64_t i = 0; i < SampledSurfaceTracker::kMaxTrackedSurfaces; i++) {
-    EXPECT_FALSE(t.ShouldRecord(0, metric(i))) << ": 0," << i;
-    EXPECT_TRUE(t.ShouldRecord(1, metric(i))) << ": 1," << i;
+  for (uint64_t i = 0; i < EncounteredSurfaceTracker::kMaxTrackedSurfaces;
+       i++) {
+    EXPECT_FALSE(t.IsNewEncounter(0, metric(i))) << ": 0," << i;
+    EXPECT_TRUE(t.IsNewEncounter(1, metric(i))) << ": 1," << i;
   }
 
   // Add an extra one. This should bump one of the surfaces out.
-  t.ShouldRecord(0, SampledSurfaceTracker::kMaxTrackedSurfaces + 1);
+  t.IsNewEncounter(0, EncounteredSurfaceTracker::kMaxTrackedSurfaces + 1);
 
   // We expect only kMaxTrackedSurfaces to return true for a new surface.
   unsigned num_true = 0;
-  for (uint64_t i = 0; i < SampledSurfaceTracker::kMaxTrackedSurfaces + 1;
+  for (uint64_t i = 0; i < EncounteredSurfaceTracker::kMaxTrackedSurfaces + 1;
        i++) {
-    if (t.ShouldRecord(2, metric(i)))
+    if (t.IsNewEncounter(2, metric(i)))
       num_true++;
   }
-  EXPECT_EQ(SampledSurfaceTracker::kMaxTrackedSurfaces, num_true);
+  EXPECT_EQ(EncounteredSurfaceTracker::kMaxTrackedSurfaces, num_true);
 }
 
-TEST(SampledSurfaceTrackerTest, Reset) {
-  SampledSurfaceTracker t;
-  EXPECT_TRUE(t.ShouldRecord(0, metric(0))) << ": 0,0";
-  EXPECT_FALSE(t.ShouldRecord(0, metric(0))) << ": 0,0";
+TEST(EncounteredSurfaceTrackerTest, Reset) {
+  EncounteredSurfaceTracker t;
+  EXPECT_TRUE(t.IsNewEncounter(0, metric(0))) << ": 0,0";
+  EXPECT_FALSE(t.IsNewEncounter(0, metric(0))) << ": 0,0";
   t.Reset();
-  EXPECT_TRUE(t.ShouldRecord(0, metric(0))) << ": 0,0";
+  EXPECT_TRUE(t.IsNewEncounter(0, metric(0))) << ": 0,0";
 }
 
-TEST(SampledSurfaceTrackerTest, InvalidMetric) {
-  SampledSurfaceTracker t;
-  EXPECT_FALSE(t.ShouldRecord(
+TEST(EncounteredSurfaceTrackerTest, InvalidMetric) {
+  EncounteredSurfaceTracker t;
+  EXPECT_FALSE(t.IsNewEncounter(
       0, blink::IdentifiableSurface::FromTypeAndToken(
              blink::IdentifiableSurface::Type::kReservedInternal, 1)
              .ToUkmMetricHash()));

chrome/browser/privacy_budget/identifiability_study_state.cc

@@ -77,7 +77,7 @@ int IdentifiabilityStudyState::generation() const {
   return generation_;
 }
 
-bool IdentifiabilityStudyState::ShouldSampleSurface(
+bool IdentifiabilityStudyState::ShouldRecordSurface(
     blink::IdentifiableSurface surface) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (LIKELY(!IsStudyActive()))
@@ -310,14 +310,14 @@ void IdentifiabilityStudyState::ReconcileLoadedPrefs() {
     WriteToPrefs();
 }
 
-bool IdentifiabilityStudyState::ShouldRecordSurface(
+bool IdentifiabilityStudyState::ShouldReportEncounteredSurface(
     uint64_t source_id,
     blink::IdentifiableSurface surface) {
   if (!blink::IdentifiabilityStudySettings::Get()->IsTypeAllowed(
           blink::IdentifiableSurface::Type::kMeasuredSurface)) {
     return false;
   }
-  return tracked_surfaces_.ShouldRecord(source_id, surface.ToUkmMetricHash());
+  return tracked_surfaces_.IsNewEncounter(source_id, surface.ToUkmMetricHash());
 }
 
 void IdentifiabilityStudyState::ResetRecordedSurfaces() {

chrome/browser/privacy_budget/identifiability_study_state.h

@@ -9,8 +9,8 @@
 #include "base/sequence_checker.h"
 #include "base/strings/string_piece_forward.h"
 #include "base/thread_annotations.h"
+#include "chrome/browser/privacy_budget/encountered_surface_tracker.h"
 #include "chrome/browser/privacy_budget/privacy_budget_prefs.h"
-#include "chrome/browser/privacy_budget/sampled_surface_tracker.h"
 #include "chrome/common/privacy_budget/privacy_budget_settings_provider.h"
 #include "components/prefs/pref_service.h"
 #include "third_party/blink/public/common/privacy_budget/identifiability_study_settings.h"
@@ -64,7 +64,7 @@ class IdentifiabilityStudyState {
   // Returns true if metrics collection is enabled for `surface`.
   //
   // Calling this method may alter the state of the study settings.
-  bool ShouldSampleSurface(blink::IdentifiableSurface surface);
+  bool ShouldRecordSurface(blink::IdentifiableSurface surface);
 
   // Should be called from unit-tests if multiple IdentifiabilityStudyState
   // instances are to be constructed.
@@ -72,7 +72,7 @@ class IdentifiabilityStudyState {
 
   // Returns true if tracking metrics should be recorded for this
   // source_id/surface combination.
-  bool ShouldRecordSurface(uint64_t source_id,
+  bool ShouldReportEncounteredSurface(uint64_t source_id,
                                       blink::IdentifiableSurface surface);
 
   // Clear the sampled surface state from the state tracker. Ideally this would
@@ -229,7 +229,7 @@ class IdentifiabilityStudyState {
   //
   //   * tracked_surfaces_.GetType() ∩ settings_.blocked_types() = Ø.
   //
-  SampledSurfaceTracker tracked_surfaces_;
+  EncounteredSurfaceTracker tracked_surfaces_;
 
   SEQUENCE_CHECKER(sequence_checker_);
 };

chrome/browser/privacy_budget/identifiability_study_state_unittest.cc

@@ -191,9 +191,9 @@ TEST_F(IdentifiabilityStudyStateTest, AllowsActive) {
   auto settings =
       std::make_unique<test_utils::InspectableIdentifiabilityStudyState>(
           pref_service());
-  EXPECT_TRUE(settings->ShouldSampleSurface(kRegularSurface1));
-  EXPECT_TRUE(settings->ShouldSampleSurface(kRegularSurface2));
-  EXPECT_TRUE(settings->ShouldSampleSurface(kRegularSurface3));
+  EXPECT_TRUE(settings->ShouldRecordSurface(kRegularSurface1));
+  EXPECT_TRUE(settings->ShouldRecordSurface(kRegularSurface2));
+  EXPECT_TRUE(settings->ShouldRecordSurface(kRegularSurface3));
   EXPECT_EQ((IdentifiableSurfaceSet{kRegularSurface1, kRegularSurface2,
                                     kRegularSurface3}),
             settings->active_surfaces());
@@ -209,15 +209,15 @@ TEST_F(IdentifiabilityStudyStateTest, BlocksBlocked) {
   auto settings =
       std::make_unique<test_utils::InspectableIdentifiabilityStudyState>(
           pref_service());
-  EXPECT_FALSE(settings->ShouldSampleSurface(kBlockedSurface1));
-  EXPECT_FALSE(settings->ShouldSampleSurface(kBlockedTypeSurface1));
+  EXPECT_FALSE(settings->ShouldRecordSurface(kBlockedSurface1));
+  EXPECT_FALSE(settings->ShouldRecordSurface(kBlockedTypeSurface1));
 }
 
 TEST_F(IdentifiabilityStudyStateTest, UpdatesActive) {
   auto settings =
       std::make_unique<test_utils::InspectableIdentifiabilityStudyState>(
           pref_service());
-  EXPECT_TRUE(settings->ShouldSampleSurface(kRegularSurface1));
+  EXPECT_TRUE(settings->ShouldRecordSurface(kRegularSurface1));
   EXPECT_EQ((IdentifiableSurfaceSet{kRegularSurface1}),
             settings->active_surfaces());
   EXPECT_EQ(SurfaceListString({kRegularSurface1}),
@@ -267,7 +267,7 @@ TEST(IdentifiabilityStudyStateStandaloneTest, Disabled) {
   prefs::RegisterPrivacyBudgetPrefs(pref_service.registry());
   test_utils::InspectableIdentifiabilityStudyState settings(&pref_service);
 
-  EXPECT_FALSE(settings.ShouldSampleSurface(kRegularSurface1));
-  EXPECT_FALSE(settings.ShouldSampleSurface(kRegularSurface2));
-  EXPECT_FALSE(settings.ShouldSampleSurface(kRegularSurface3));
+  EXPECT_FALSE(settings.ShouldRecordSurface(kRegularSurface1));
+  EXPECT_FALSE(settings.ShouldRecordSurface(kRegularSurface2));
+  EXPECT_FALSE(settings.ShouldRecordSurface(kRegularSurface3));
 }

chrome/browser/privacy_budget/privacy_budget_ukm_entry_filter.cc

@@ -11,7 +11,7 @@
 #include "base/rand_util.h"
 #include "base/stl_util.h"
 #include "base/time/time.h"
-#include "chrome/browser/privacy_budget/sampled_surface_tracker.h"
+#include "chrome/browser/privacy_budget/encountered_surface_tracker.h"
 #include "services/metrics/public/cpp/ukm_builders.h"
 #include "services/metrics/public/mojom/ukm_interface.mojom.h"
 #include "third_party/blink/public/common/privacy_budget/identifiability_study_settings.h"
@@ -35,36 +35,34 @@ bool PrivacyBudgetUkmEntryFilter::FilterEntry(
   if (!enabled || entry->metrics.empty())
     return false;
 
-  std::vector<blink::IdentifiableSurface> sampled_surfaces;
-  sampled_surfaces.reserve(entry->metrics.size());
+  // Contains newly encountered surfaces in entry->metrics.
+  std::vector<blink::IdentifiableSurface> encountered_surfaces;
+  encountered_surfaces.reserve(entry->metrics.size());
+
   base::EraseIf(entry->metrics, [&](auto metric) {
     const auto surface =
         blink::IdentifiableSurface::FromMetricHash(metric.first);
-    // Exclude the set that are blocked from all measurements.
     if (!blink::IdentifiabilityStudySettings::Get()->IsSurfaceAllowed(surface))
       return true;
 
-    // Record the set of surfaces sampled by the site.
-    if (identifiability_study_state_->ShouldRecordSurface(entry->source_id,
-                                                          surface))
-      sampled_surfaces.push_back(surface);
+    if (identifiability_study_state_->ShouldReportEncounteredSurface(
+            entry->source_id, surface)) {
+      encountered_surfaces.push_back(surface);
+    }
 
-    // Exclude the set that are disabled for this user
-    return !identifiability_study_state_->ShouldSampleSurface(surface);
+    return !identifiability_study_state_->ShouldRecordSurface(surface);
   });
 
-  uint64_t sample_idx = 0;
-  for (const auto& v : sampled_surfaces) {
-    // Add entries marking the surfaces that were sampled by the source as
-    // sampled.
+  uint64_t index = 0;
+  for (const auto& v : encountered_surfaces) {
     blink::IdentifiableSurface s = blink::IdentifiableSurface::FromTypeAndToken(
-        blink::IdentifiableSurface::Type::kMeasuredSurface, sample_idx++);
+        blink::IdentifiableSurface::Type::kMeasuredSurface, index++);
     entry->metrics.insert_or_assign(entry->metrics.end(), s.ToUkmMetricHash(),
                                     v.ToUkmMetricHash());
   }
 
   // Identifiability metrics can leak information simply by being measured.
-  // Hence the metrics that are filtered out aren't returning in
+  // Hence the metrics that are filtered out aren't returned in
   // |removed_metric_hashes|.
   return !entry->metrics.empty();
 }