metricsd: Specify directory for persistent integers.

Instead of using a global directory for persistent integers, specify the
directory to use in the constructor.
This will make changing the backing directory easier.

Bug: 25886951

Change-Id: I590816b195fa81b179a5ec78b9cdf41bc86353dc

9 files changed, 63 insertions, 92 deletions

diff --git a/metrics_collector.cc b/metrics_collector.cc
index 28194a1..ddf9bc1 100644
--- a/metrics_collector.cc
+++ b/metrics_collector.cc
@@ -149,51 +149,52 @@ uint32_t MetricsCollector::GetOsVersionHash() {
   return version_hash;
 }
 
-void MetricsCollector::Init(bool testing,
-                         MetricsLibraryInterface* metrics_lib,
-                         const string& diskstats_path,
-                         const base::FilePath& metrics_directory) {
+void MetricsCollector::Init(bool testing, MetricsLibraryInterface* metrics_lib,
+                            const string& diskstats_path,
+                            const base::FilePath& metrics_directory) {
   CHECK(metrics_lib);
   testing_ = testing;
   metrics_directory_ = metrics_directory;
   metrics_lib_ = metrics_lib;
 
   daily_active_use_.reset(
-      new PersistentInteger("Platform.UseTime.PerDay"));
+      new PersistentInteger("Platform.UseTime.PerDay", metrics_directory_));
   version_cumulative_active_use_.reset(
-      new PersistentInteger("Platform.CumulativeUseTime"));
+      new PersistentInteger("Platform.CumulativeUseTime", metrics_directory_));
   version_cumulative_cpu_use_.reset(
-      new PersistentInteger("Platform.CumulativeCpuTime"));
+      new PersistentInteger("Platform.CumulativeCpuTime", metrics_directory_));
 
-  kernel_crash_interval_.reset(
-      new PersistentInteger("Platform.KernelCrashInterval"));
-  unclean_shutdown_interval_.reset(
-      new PersistentInteger("Platform.UncleanShutdownInterval"));
+  kernel_crash_interval_.reset(new PersistentInteger(
+      "Platform.KernelCrashInterval", metrics_directory_));
+  unclean_shutdown_interval_.reset(new PersistentInteger(
+      "Platform.UncleanShutdownInterval", metrics_directory_));
   user_crash_interval_.reset(
-      new PersistentInteger("Platform.UserCrashInterval"));
+      new PersistentInteger("Platform.UserCrashInterval", metrics_directory_));
 
   any_crashes_daily_count_.reset(
-      new PersistentInteger("Platform.AnyCrashes.PerDay"));
+      new PersistentInteger("Platform.AnyCrashes.PerDay", metrics_directory_));
   any_crashes_weekly_count_.reset(
-      new PersistentInteger("Platform.AnyCrashes.PerWeek"));
+      new PersistentInteger("Platform.AnyCrashes.PerWeek", metrics_directory_));
   user_crashes_daily_count_.reset(
-      new PersistentInteger("Platform.UserCrashes.PerDay"));
-  user_crashes_weekly_count_.reset(
-      new PersistentInteger("Platform.UserCrashes.PerWeek"));
-  kernel_crashes_daily_count_.reset(
-      new PersistentInteger("Platform.KernelCrashes.PerDay"));
-  kernel_crashes_weekly_count_.reset(
-      new PersistentInteger("Platform.KernelCrashes.PerWeek"));
-  kernel_crashes_version_count_.reset(
-      new PersistentInteger("Platform.KernelCrashesSinceUpdate"));
-  unclean_shutdowns_daily_count_.reset(
-      new PersistentInteger("Platform.UncleanShutdown.PerDay"));
-  unclean_shutdowns_weekly_count_.reset(
-      new PersistentInteger("Platform.UncleanShutdowns.PerWeek"));
-
-  daily_cycle_.reset(new PersistentInteger("daily.cycle"));
-  weekly_cycle_.reset(new PersistentInteger("weekly.cycle"));
-  version_cycle_.reset(new PersistentInteger("version.cycle"));
+      new PersistentInteger("Platform.UserCrashes.PerDay", metrics_directory_));
+  user_crashes_weekly_count_.reset(new PersistentInteger(
+      "Platform.UserCrashes.PerWeek", metrics_directory_));
+  kernel_crashes_daily_count_.reset(new PersistentInteger(
+      "Platform.KernelCrashes.PerDay", metrics_directory_));
+  kernel_crashes_weekly_count_.reset(new PersistentInteger(
+      "Platform.KernelCrashes.PerWeek", metrics_directory_));
+  kernel_crashes_version_count_.reset(new PersistentInteger(
+      "Platform.KernelCrashesSinceUpdate", metrics_directory_));
+  unclean_shutdowns_daily_count_.reset(new PersistentInteger(
+      "Platform.UncleanShutdown.PerDay", metrics_directory_));
+  unclean_shutdowns_weekly_count_.reset(new PersistentInteger(
+      "Platform.UncleanShutdowns.PerWeek", metrics_directory_));
+
+  daily_cycle_.reset(new PersistentInteger("daily.cycle", metrics_directory_));
+  weekly_cycle_.reset(
+      new PersistentInteger("weekly.cycle", metrics_directory_));
+  version_cycle_.reset(
+      new PersistentInteger("version.cycle", metrics_directory_));
 
   disk_usage_collector_.reset(new DiskUsageCollector(metrics_lib_));
   averaged_stats_collector_.reset(
diff --git a/metrics_collector_test.cc b/metrics_collector_test.cc
index a0e7087..0046360 100644
--- a/metrics_collector_test.cc
+++ b/metrics_collector_test.cc
@@ -45,9 +45,6 @@ class MetricsCollectorTest : public testing::Test {
   virtual void SetUp() {
     brillo::FlagHelper::Init(0, nullptr, "");
     EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
-
-    chromeos_metrics::PersistentInteger::SetMetricsDirectory(
-        temp_dir_.path().value());
     daemon_.Init(true, &metrics_lib_, "", temp_dir_.path());
   }
 
diff --git a/persistent_integer.cc b/persistent_integer.cc
index ddc4b50..7fe355e 100644
--- a/persistent_integer.cc
+++ b/persistent_integer.cc
@@ -23,19 +23,15 @@
 
 #include "constants.h"
 
-
 namespace chromeos_metrics {
 
-// Static class member instantiation.
-std::string PersistentInteger::metrics_directory_ = metrics::kMetricsDirectory;
-
-PersistentInteger::PersistentInteger(const std::string& name) :
-      value_(0),
+PersistentInteger::PersistentInteger(const std::string& name,
+                                     const base::FilePath& directory)
+    : value_(0),
       version_(kVersion),
       name_(name),
-      synced_(false) {
-  backing_file_name_ = metrics_directory_ + name_;
-}
+      backing_file_path_(directory.Append(name_)),
+      synced_(false) {}
 
 PersistentInteger::~PersistentInteger() {}
 
@@ -62,23 +58,25 @@ void PersistentInteger::Add(int64_t x) {
 }
 
 void PersistentInteger::Write() {
-  int fd = HANDLE_EINTR(open(backing_file_name_.c_str(),
+  int fd = HANDLE_EINTR(open(backing_file_path_.value().c_str(),
                              O_WRONLY | O_CREAT | O_TRUNC,
                              S_IWUSR | S_IRUSR | S_IRGRP | S_IROTH));
-  PCHECK(fd >= 0) << "cannot open " << backing_file_name_ << " for writing";
+  PCHECK(fd >= 0) << "cannot open " << backing_file_path_.value()
+                  << " for writing";
   PCHECK((HANDLE_EINTR(write(fd, &version_, sizeof(version_))) ==
           sizeof(version_)) &&
          (HANDLE_EINTR(write(fd, &value_, sizeof(value_))) ==
           sizeof(value_)))
-      << "cannot write to " << backing_file_name_;
+      << "cannot write to " << backing_file_path_.value();
   close(fd);
   synced_ = true;
 }
 
 bool PersistentInteger::Read() {
-  int fd = HANDLE_EINTR(open(backing_file_name_.c_str(), O_RDONLY));
+  int fd = HANDLE_EINTR(open(backing_file_path_.value().c_str(), O_RDONLY));
   if (fd < 0) {
-    PLOG(WARNING) << "cannot open " << backing_file_name_ << " for reading";
+    PLOG(WARNING) << "cannot open " << backing_file_path_.value()
+                  << " for reading";
     return false;
   }
   int32_t version;
@@ -95,9 +93,4 @@ bool PersistentInteger::Read() {
   return read_succeeded;
 }
 
-void PersistentInteger::SetMetricsDirectory(const std::string& directory) {
-  metrics_directory_ = directory;
-}
-
-
 }  // namespace chromeos_metrics
diff --git a/persistent_integer.h b/persistent_integer.h
index ecef3d1..96d9fc0 100644
--- a/persistent_integer.h
+++ b/persistent_integer.h
@@ -21,6 +21,8 @@
 
 #include <string>
 
+#include <base/files/file_path.h>
+
 namespace chromeos_metrics {
 
 // PersistentIntegers is a named 64-bit integer value backed by a file.
@@ -29,7 +31,7 @@ namespace chromeos_metrics {
 
 class PersistentInteger {
  public:
-  explicit PersistentInteger(const std::string& name);
+  PersistentInteger(const std::string& name, const base::FilePath& directory);
 
   // Virtual only because of mock.
   virtual ~PersistentInteger();
@@ -50,10 +52,6 @@ class PersistentInteger {
   // Virtual only because of mock.
   virtual void Add(int64_t x);
 
-  // Sets the directory path for all persistent integers.
-  // This is used in unittests to change where the counters are stored.
-  static void SetMetricsDirectory(const std::string& directory);
-
  private:
   static const int kVersion = 1001;
 
@@ -68,8 +66,7 @@ class PersistentInteger {
   int64_t value_;
   int32_t version_;
   std::string name_;
-  std::string backing_file_name_;
-  static std::string metrics_directory_;
+  base::FilePath backing_file_path_;
   bool synced_;
 };
 
diff --git a/persistent_integer_mock.h b/persistent_integer_mock.h
index acc5389..0be54d4 100644
--- a/persistent_integer_mock.h
+++ b/persistent_integer_mock.h
@@ -27,9 +27,10 @@ namespace chromeos_metrics {
 
 class PersistentIntegerMock : public PersistentInteger {
  public:
-  explicit PersistentIntegerMock(const std::string& name)
-      : PersistentInteger(name) {}
-    MOCK_METHOD1(Add, void(int64_t count));
+  explicit PersistentIntegerMock(const std::string& name,
+                                 const base::FilePath& directory)
+      : PersistentInteger(name, directory) {}
+  MOCK_METHOD1(Add, void(int64_t count));
 };
 
 }  // namespace chromeos_metrics
diff --git a/persistent_integer_test.cc b/persistent_integer_test.cc
index 5e2067f..55d6cbc 100644
--- a/persistent_integer_test.cc
+++ b/persistent_integer_test.cc
@@ -24,7 +24,6 @@
 #include "persistent_integer.h"
 
 const char kBackingFileName[] = "1.pibakf";
-const char kBackingFilePattern[] = "*.pibakf";
 
 using chromeos_metrics::PersistentInteger;
 
@@ -32,28 +31,15 @@ class PersistentIntegerTest : public testing::Test {
   void SetUp() override {
     // Set testing mode.
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-    chromeos_metrics::PersistentInteger::SetMetricsDirectory(
-        temp_dir_.path().value());
-  }
-
-  void TearDown() override {
-    // Remove backing files.  The convention is that they all end in ".pibakf".
-    base::FileEnumerator f_enum(base::FilePath("."),
-                                false,
-                                base::FileEnumerator::FILES,
-                                FILE_PATH_LITERAL(kBackingFilePattern));
-    for (base::FilePath name = f_enum.Next();
-         !name.empty();
-         name = f_enum.Next()) {
-      base::DeleteFile(name, false);
-    }
   }
 
+ protected:
   base::ScopedTempDir temp_dir_;
 };
 
 TEST_F(PersistentIntegerTest, BasicChecks) {
-  scoped_ptr<PersistentInteger> pi(new PersistentInteger(kBackingFileName));
+  scoped_ptr<PersistentInteger> pi(
+      new PersistentInteger(kBackingFileName, temp_dir_.path()));
 
   // Test initialization.
   EXPECT_EQ(0, pi->Get());
@@ -65,7 +51,7 @@ TEST_F(PersistentIntegerTest, BasicChecks) {
   EXPECT_EQ(5, pi->Get());
 
   // Test persistence.
-  pi.reset(new PersistentInteger(kBackingFileName));
+  pi.reset(new PersistentInteger(kBackingFileName, temp_dir_.path()));
   EXPECT_EQ(5, pi->Get());
 
   // Test GetAndClear.
@@ -73,6 +59,6 @@ TEST_F(PersistentIntegerTest, BasicChecks) {
   EXPECT_EQ(pi->Get(), 0);
 
   // Another persistence test.
-  pi.reset(new PersistentInteger(kBackingFileName));
+  pi.reset(new PersistentInteger(kBackingFileName, temp_dir_.path()));
   EXPECT_EQ(0, pi->Get());
 }
diff --git a/uploader/system_profile_cache.cc b/uploader/system_profile_cache.cc
index 8928a0d..6afc86c 100644
--- a/uploader/system_profile_cache.cc
+++ b/uploader/system_profile_cache.cc
@@ -55,11 +55,10 @@ std::string ChannelToString(
 
 SystemProfileCache::SystemProfileCache()
     : initialized_(false),
-    testing_(false),
-    metrics_directory_(metrics::kMetricsDirectory),
-    session_id_(new chromeos_metrics::PersistentInteger(
-        kPersistentSessionIdFilename)) {
-}
+      testing_(false),
+      metrics_directory_(metrics::kMetricsDirectory),
+      session_id_(new chromeos_metrics::PersistentInteger(
+          kPersistentSessionIdFilename, metrics_directory_)) {}
 
 SystemProfileCache::SystemProfileCache(bool testing,
                                        const base::FilePath& metrics_directory)
@@ -67,8 +66,7 @@ SystemProfileCache::SystemProfileCache(bool testing,
       testing_(testing),
       metrics_directory_(metrics_directory),
       session_id_(new chromeos_metrics::PersistentInteger(
-          kPersistentSessionIdFilename)) {
-}
+          kPersistentSessionIdFilename, metrics_directory)) {}
 
 bool SystemProfileCache::Initialize() {
   CHECK(!initialized_)
diff --git a/uploader/upload_service.cc b/uploader/upload_service.cc
index ca5024e..b4731e9 100644
--- a/uploader/upload_service.cc
+++ b/uploader/upload_service.cc
@@ -46,7 +46,7 @@ UploadService::UploadService(const std::string& server,
                              const base::FilePath& metrics_directory)
     : histogram_snapshot_manager_(this),
       sender_(new HttpSender(server)),
-      failed_upload_count_(metrics::kFailedUploadCountName),
+      failed_upload_count_(metrics::kFailedUploadCountName, metrics_directory),
       upload_interval_(upload_interval) {
   metrics_file_ = metrics_directory.Append(metrics::kMetricsEventsFileName);
   staged_log_path_ = metrics_directory.Append(metrics::kStagedLogName);
diff --git a/uploader/upload_service_test.cc b/uploader/upload_service_test.cc
index 24e3127..c77b342 100644
--- a/uploader/upload_service_test.cc
+++ b/uploader/upload_service_test.cc
@@ -39,8 +39,6 @@ class UploadServiceTest : public testing::Test {
  protected:
   virtual void SetUp() {
     CHECK(dir_.CreateUniqueTempDir());
-    chromeos_metrics::PersistentInteger::SetMetricsDirectory(
-        dir_.path().value());
     metrics_lib_.InitForTest(dir_.path());
     ASSERT_EQ(0, base::WriteFile(
         dir_.path().Append(metrics::kConsentFileName), "", 0));