Fix LevelDB histogram code.

third_party/leveldatabase/env_chromium.cc

Index: third_party/leveldatabase/env_chromium.cc
diff --git a/third_party/leveldatabase/env_chromium.cc b/third_party/leveldatabase/env_chromium.cc
index 94895e346894359252b762d9e45ded75147264bd..2d7a936cdea4f4793e29fa29c70d27dd0db67c77 100644
--- a/third_party/leveldatabase/env_chromium.cc
+++ b/third_party/leveldatabase/env_chromium.cc
@@ -126,32 +126,11 @@ enum UmaEntry {
 
 class UMALogger {
  public:
-  UMALogger(std::string uma_title);
-  void RecordErrorAt(UmaEntry entry) const;
-  void LogRandomAccessFileError(base::PlatformFileError error_code) const;
-
- private:
-  std::string uma_title_;
+  virtual void RecordErrorAt(UmaEntry entry) const = 0;
+  virtual void LogRandomAccessFileError(base::PlatformFileError error_code)
+      const = 0;
 };
 
-UMALogger::UMALogger(std::string uma_title) : uma_title_(uma_title) {}
-
-void UMALogger::RecordErrorAt(UmaEntry entry) const {
-  std::string uma_name(uma_title_);
-  uma_name.append(".IOError");
-  UMA_HISTOGRAM_ENUMERATION(uma_name, entry, kNumEntries);
-}
-
-void UMALogger::LogRandomAccessFileError(base::PlatformFileError error_code)
-    const {
-  DCHECK(error_code < 0);
-  std::string uma_name(uma_title_);
-  uma_name.append(".IOError.RandomAccessFile");
-  UMA_HISTOGRAM_ENUMERATION(uma_name,
-                            -error_code,
-                            -base::PLATFORM_FILE_ERROR_MAX);
-}
-
 }  // namespace
 
 namespace leveldb {
@@ -338,7 +317,7 @@ class ChromiumFileLock : public FileLock {
   ::base::PlatformFile file_;
 };
 
-class ChromiumEnv : public Env {
+class ChromiumEnv : public Env, public UMALogger {
  public:
   ChromiumEnv();
   virtual ~ChromiumEnv() {
@@ -350,10 +329,10 @@ class ChromiumEnv : public Env {
     FILE* f = fopen_internal(fname.c_str(), "rb");
     if (f == NULL) {
       *result = NULL;
-      uma_logger_->RecordErrorAt(kNewSequentialFile);
+      RecordErrorAt(kNewSequentialFile);
       return Status::IOError(fname, strerror(errno));
     } else {
-      *result = new ChromiumSequentialFile(fname, f, uma_logger_.get());
+      *result = new ChromiumSequentialFile(fname, f, this);
       return Status::OK();
     }
   }
@@ -367,11 +346,11 @@ class ChromiumEnv : public Env {
         CreateFilePath(fname), flags, &created, &error_code);
     if (error_code != ::base::PLATFORM_FILE_OK) {
       *result = NULL;
-      uma_logger_->RecordErrorAt(kNewRandomAccessFile);
-      uma_logger_->LogRandomAccessFileError(error_code);
+      RecordErrorAt(kNewRandomAccessFile);
+      LogRandomAccessFileError(error_code);
       return Status::IOError(fname, PlatformFileErrorString(error_code));
     }
-    *result = new ChromiumRandomAccessFile(fname, file, uma_logger_.get());
+    *result = new ChromiumRandomAccessFile(fname, file, this);
     return Status::OK();
   }
 
@@ -380,14 +359,14 @@ class ChromiumEnv : public Env {
     *result = NULL;
     FILE* f = fopen_internal(fname.c_str(), "wb");
     if (f == NULL) {
-      uma_logger_->RecordErrorAt(kNewWritableFile);
+      RecordErrorAt(kNewWritableFile);
       return Status::IOError(fname, strerror(errno));
     } else {
       if (!sync_parent(fname)) {
         fclose(f);
         return Status::IOError(fname, strerror(errno));
       }
-      *result = new ChromiumWritableFile(fname, f, uma_logger_.get());
+      *result = new ChromiumWritableFile(fname, f, this);
       return Status::OK();
     }
   }
@@ -417,7 +396,7 @@ class ChromiumEnv : public Env {
     // TODO(jorlow): Should we assert this is a file?
     if (!::file_util::Delete(CreateFilePath(fname), false)) {
       result = Status::IOError(fname, "Could not delete file.");
-      uma_logger_->RecordErrorAt(kDeleteFile);
+      RecordErrorAt(kDeleteFile);
     }
     return result;
   };
@@ -426,7 +405,7 @@ class ChromiumEnv : public Env {
     Status result;
     if (!::file_util::CreateDirectory(CreateFilePath(name))) {
       result = Status::IOError(name, "Could not create directory.");
-      uma_logger_->RecordErrorAt(kCreateDir);
+      RecordErrorAt(kCreateDir);
     }
     return result;
   };
@@ -436,7 +415,7 @@ class ChromiumEnv : public Env {
     // TODO(jorlow): Should we assert this is a directory?
     if (!::file_util::Delete(CreateFilePath(name), false)) {
       result = Status::IOError(name, "Could not delete directory.");
-      uma_logger_->RecordErrorAt(kDeleteDir);
+      RecordErrorAt(kDeleteDir);
     }
     return result;
   };
@@ -447,7 +426,7 @@ class ChromiumEnv : public Env {
     if (!::file_util::GetFileSize(CreateFilePath(fname), &signed_size)) {
       *size = 0;
       s = Status::IOError(fname, "Could not determine file size.");
-      uma_logger_->RecordErrorAt(kGetFileSize);
+      RecordErrorAt(kGetFileSize);
     } else {
       *size = static_cast<uint64_t>(signed_size);
     }
@@ -458,7 +437,7 @@ class ChromiumEnv : public Env {
     Status result;
     if (!::file_util::ReplaceFile(CreateFilePath(src), CreateFilePath(dst))) {
       result = Status::IOError(src, "Could not rename file.");
-      uma_logger_->RecordErrorAt(kRenamefile);
+      RecordErrorAt(kRenamefile);
     } else {
       sync_parent(dst);
       if (src != dst)
@@ -481,7 +460,7 @@ class ChromiumEnv : public Env {
         CreateFilePath(fname), flags, &created, &error_code);
     if (error_code != ::base::PLATFORM_FILE_OK) {
       result = Status::IOError(fname, PlatformFileErrorString(error_code));
-      uma_logger_->RecordErrorAt(kLockFile);
+      RecordErrorAt(kLockFile);
     } else {
       ChromiumFileLock* my_lock = new ChromiumFileLock;
       my_lock->file_ = file;
@@ -495,7 +474,7 @@ class ChromiumEnv : public Env {
     Status result;
     if (!::base::ClosePlatformFile(my_lock->file_)) {
       result = Status::IOError("Could not close lock file.");
-      uma_logger_->RecordErrorAt(kUnlockFile);
+      RecordErrorAt(kUnlockFile);
     }
     delete my_lock;
     return result;
@@ -524,7 +503,7 @@ class ChromiumEnv : public Env {
       if (!::file_util::CreateNewTempDirectory(kLevelDBTestDirectoryPrefix,
                                                &test_directory_)) {
         mu_.Release();
-        uma_logger_->RecordErrorAt(kGetTestDirectory);
+        RecordErrorAt(kGetTestDirectory);
         return Status::IOError("Could not create temp directory.");
       }
     }
@@ -537,7 +516,7 @@ class ChromiumEnv : public Env {
     FILE* f = fopen_internal(fname.c_str(), "w");
     if (f == NULL) {
       *result = NULL;
-      uma_logger_->RecordErrorAt(kNewLogger);
+      RecordErrorAt(kNewLogger);
       return Status::IOError(fname, strerror(errno));
     } else {
       if (!sync_parent(fname)) {
@@ -558,8 +537,17 @@ class ChromiumEnv : public Env {
     ::base::PlatformThread::Sleep(::base::TimeDelta::FromMicroseconds(micros));
   }
 
+  void RecordErrorAt(UmaEntry entry) const {
+    io_error_histogram_->Add(entry);
+  }
+
+  void LogRandomAccessFileError(base::PlatformFileError error_code) const {
+    DCHECK(error_code < 0);
+    random_access_file_histogram_->Add(-error_code);
+  }
+
  protected:
-  scoped_ptr<UMALogger> uma_logger_;
+  void InitHistograms(const std::string& uma_title);
 
  private:
   // BGThread() is the body of the background thread
@@ -579,13 +567,31 @@ class ChromiumEnv : public Env {
   struct BGItem { void* arg; void (*function)(void*); };
   typedef std::deque<BGItem> BGQueue;
   BGQueue queue_;
+
+  base::Histogram* io_error_histogram_;
+  base::Histogram* random_access_file_histogram_;
 };
 
 ChromiumEnv::ChromiumEnv()
     : page_size_(::base::SysInfo::VMAllocationGranularity()),
       bgsignal_(&mu_),
-      started_bgthread_(false),
-      uma_logger_(new UMALogger("LevelDBEnv")) {
+      started_bgthread_(false) {
+  InitHistograms("LevelDBEnv");
+}
+
+void ChromiumEnv::InitHistograms(const std::string& uma_title) {
+  std::string uma_name(uma_title);
+  uma_name.append(".IOError");
+  // Note: The calls to FactoryGet aren't thread-safe. It's ok to call them here
+  // because this method is only called from LazyInstance, which provides
+  // thread-safety.
+  io_error_histogram_ = base::LinearHistogram::FactoryGet(uma_name, 1,
+      kNumEntries, kNumEntries + 1, base::Histogram::kUmaTargetedHistogramFlag);
+
+  uma_name.append(".RandomAccessFile");
+  random_access_file_histogram_ = base::LinearHistogram::FactoryGet(uma_name, 1,
+      -base::PLATFORM_FILE_ERROR_MAX, -base::PLATFORM_FILE_ERROR_MAX + 1,
+      base::Histogram::kUmaTargetedHistogramFlag);
 }
 
 class Thread : public ::base::PlatformThread::Delegate {
@@ -654,7 +660,7 @@ void ChromiumEnv::StartThread(void (*function)(void* arg), void* arg) {
 class IDBEnv : public ChromiumEnv {
  public:
   IDBEnv() : ChromiumEnv() {
-    uma_logger_.reset(new UMALogger("LevelDBEnv.IDB"));
+    InitHistograms("LevelDBEnv.IDB");
   }
 };