ServiceWorker: Add UMA for ServiceWorkerDatabase

content/browser/service_worker/service_worker_database.cc

Index: content/browser/service_worker/service_worker_database.cc
diff --git a/content/browser/service_worker/service_worker_database.cc b/content/browser/service_worker/service_worker_database.cc
index e6bd79f6a2ebd17b8d25d31703425161a73a9f80..14e55682b79ea9d2cee5aab2a1ac0a0f9bfd8c56 100644
--- a/content/browser/service_worker/service_worker_database.cc
+++ b/content/browser/service_worker/service_worker_database.cc
@@ -9,6 +9,7 @@
 #include "base/file_util.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
@@ -76,6 +77,12 @@ const char kPurgeableResIdKeyPrefix[] = "PRES:";
 
 const int64 kCurrentSchemaVersion = 1;
 
+// For histogram.
+const char kOpenHistogramLabel[] =
+    "ServiceWorker.Database.Open";
+const char kOperationErrorHistogramLabel[] =
+    "ServiceWorker.Database.OperationError";
+
 bool RemovePrefix(const std::string& str,
                   const std::string& prefix,
                   std::string* out) {
@@ -218,6 +225,8 @@ ServiceWorkerDatabase::Status LevelDBStatusToStatus(
     return ServiceWorkerDatabase::STATUS_OK;
   else if (status.IsNotFound())
     return ServiceWorkerDatabase::STATUS_ERROR_NOT_FOUND;
+  else if (status.IsIOError())
+    return ServiceWorkerDatabase::STATUS_ERROR_IO_ERROR;
   else if (status.IsCorruption())
     return ServiceWorkerDatabase::STATUS_ERROR_CORRUPTED;
   else
@@ -676,16 +685,18 @@ ServiceWorkerDatabase::Status ServiceWorkerDatabase::LazyOpen(
   leveldb::DB* db = NULL;
   leveldb::Status db_status =
       leveldb::DB::Open(options, path_.AsUTF8Unsafe(), &db);
+  Status status = LevelDBStatusToStatus(db_status);
+  UMA_HISTOGRAM_ENUMERATION(kOpenHistogramLabel, status, STATUS_ERROR_MAX);
   if (!db_status.ok()) {
     DCHECK(!db);
     // TODO(nhiroki): Should we retry to open the database?
     HandleError(FROM_HERE, db_status);
-    return LevelDBStatusToStatus(db_status);
+    return status;
   }
   db_.reset(db);
 
   int64 db_version;
-  Status status = ReadDatabaseVersion(&db_version);
+  status = ReadDatabaseVersion(&db_version);
   if (status != STATUS_OK)
     return status;
   DCHECK_LE(0, db_version);
@@ -994,9 +1005,12 @@ bool ServiceWorkerDatabase::IsOpen() {
 void ServiceWorkerDatabase::HandleError(
     const tracked_objects::Location& from_here,
     const leveldb::Status& status) {
-  // TODO(nhiroki): Add an UMA histogram.
+  DCHECK(!status.ok());
   DLOG(ERROR) << "Failed at: " << from_here.ToString()
               << " with error: " << status.ToString();
+  UMA_HISTOGRAM_ENUMERATION(kOperationErrorHistogramLabel,

[michaeln, 2014/05/29 23:46:28]

I'm not sure how fine grained we want this to be?

Having the result of open be separate is definitely good and also reporting OK
events for that case is good so we can see rates.

Would it make sense to split ReadResults from WriteResult and to also log the OK
events so we can see rates?

[nhiroki, 2014/06/02 04:54:47]

SGTM. The latest patchset records read/write results separately.

+                            LevelDBStatusToStatus(status),
+                            STATUS_ERROR_MAX);
   state_ = DISABLED;
   db_.reset();
 }