Issue 6841: Drop unused P3A values from local state.

Author: Ivan Efremov

components/p3a/brave_p3a_log_store.cc

@@ -34,10 +34,10 @@ void RecordP3A(uint64_t answers_count) {
 
 }  // namespace
 
-BraveP3ALogStore::BraveP3ALogStore(LogSerializer* serializer,
+BraveP3ALogStore::BraveP3ALogStore(Delegate* delegate,
                                    PrefService* local_state)
-    : serializer_(serializer), local_state_(local_state) {
-  DCHECK(serializer_);
+    : delegate_(delegate), local_state_(local_state) {
+  DCHECK(delegate_);
   DCHECK(local_state);
 }
 
@@ -123,7 +123,8 @@ void BraveP3ALogStore::StageNextLog() {
   DCHECK(!log_.find(staged_entry_key_)->second.sent);
 
   uint64_t staged_entry_value = log_[staged_entry_key_].value;
-  staged_log_ = serializer_->Serialize(staged_entry_key_, staged_entry_value);
+  staged_log_ = delegate_->Serialize(staged_entry_key_, staged_entry_value);
+
   VLOG(2) << "BraveP3ALogStore::StageNextLog: staged " << staged_entry_key_;
 }
 
@@ -161,10 +162,17 @@ void BraveP3ALogStore::LoadPersistedUnsentLogs() {
   DCHECK(log_.empty());
   DCHECK(unsent_entries_.empty());
 
-  const base::DictionaryValue* list = local_state_->GetDictionary(kPrefName);
+  DictionaryPrefUpdate update(local_state_, kPrefName);
+  base::DictionaryValue* list = update.Get();
   for (auto dict_item : list->DictItems()) {
     LogEntry entry;
-    std::string name = dict_item.first;
+    const std::string name = dict_item.first;
+    // Check if the metric is obsolete.
+    if (!delegate_->IsActualMetric(name)) {
+      // Drop it from the local state.
+      list->RemoveKey(name);
+      continue;
+    }
     const base::Value& dict = dict_item.second;
     // Value.
     if (const base::Value* v =

components/p3a/brave_p3a_log_store.h

@@ -25,15 +25,17 @@ namespace brave {
 // for now persisted entries never expire.
 class BraveP3ALogStore : public metrics::LogStore {
  public:
-  // Prepares a string representaion of an entry.
-  class LogSerializer {
+  class Delegate {
    public:
+    // Prepares a string representaion of an entry.
     virtual std::string Serialize(base::StringPiece histogram_name,
                                   uint64_t value) const = 0;
-    virtual ~LogSerializer() {}
+    // Returns false if the metric is obsolete and should be cleaned up.
+    virtual bool IsActualMetric(base::StringPiece histogram_name) const = 0;
+    virtual ~Delegate() {}
   };
 
-  explicit BraveP3ALogStore(LogSerializer* serializer,
+  explicit BraveP3ALogStore(Delegate* delegate,
                             PrefService* local_state);
 
   // TODO(iefremov): Make parent destructor virtual?
@@ -78,7 +80,7 @@ class BraveP3ALogStore : public metrics::LogStore {
     base::Time sent_timestamp;  // At the moment only for debugging purposes.
   };
 
-  const LogSerializer* const serializer_ = nullptr;  // Weak.
+  const Delegate* const delegate_ = nullptr;  // Weak.
   PrefService* const local_state_ = nullptr;
 
   // TODO(iefremov): Try to replace with base::StringPiece?

components/p3a/brave_p3a_service.cc

@@ -207,6 +207,14 @@ std::string BraveP3AService::Serialize(base::StringPiece histogram_name,
   return message.SerializeAsString();
 }
 
+bool
+BraveP3AService::IsActualMetric(base::StringPiece histogram_name) const {
+  static const base::NoDestructor<base::flat_set<base::StringPiece>>
+      metric_names {std::begin(kCollectedHistograms),
+                    std::end(kCollectedHistograms)};
+  return metric_names->contains(histogram_name);
+}
+
 void BraveP3AService::MaybeOverrideSettingsFromCommandLine() {
   base::CommandLine* cmdline = base::CommandLine::ForCurrentProcess();
 

components/p3a/brave_p3a_service.h

@@ -30,7 +30,7 @@ class BraveP3AUploader;
 // on any thread.
 // TODO(iefremov): It should be possible to get rid of refcounted here.
 class BraveP3AService : public base::RefCountedThreadSafe<BraveP3AService>,
-                        public BraveP3ALogStore::LogSerializer {
+                        public BraveP3ALogStore::Delegate {
  public:
   explicit BraveP3AService(PrefService* local_state);
 
@@ -43,10 +43,12 @@ class BraveP3AService : public base::RefCountedThreadSafe<BraveP3AService>,
   // Needs a living browser process to complete the initialization.
   void Init();
 
-  // BraveP3ALogStore::LogSerializer
+  // BraveP3ALogStore::Delegate
   std::string Serialize(base::StringPiece histogram_name,
                         uint64_t value) const override;
 
+  bool IsActualMetric(base::StringPiece histogram_name) const override;
+
  private:
   friend class base::RefCountedThreadSafe<BraveP3AService>;
   ~BraveP3AService() override;