Move some histogram recordings to ServiceWorkerStorage

This CL removes ServiceWorkerMetrics dependency from
ServiceWorkerStorage by embedding UMA recordings into it.
ServiceWorkerStorage will be moved to //components/services/storage
but ServiceWorkerMetrics will remain //content. This means that
ServiceWorkerStorage cannot use ServiceWorkerMetrics.

This CL also applies histogram guidelines
(//tools/metrics/histograms/README.md).

Bug: 1016064
Change-Id: I79ba99c096fc422166b0f0f23d27728804701113
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2394899Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804710}

content/browser/service_worker/service_worker_metrics.cc

@@ -238,10 +238,6 @@ ServiceWorkerMetrics::Site ServiceWorkerMetrics::SiteFromURL(const GURL& url) {
   return ServiceWorkerMetrics::Site::OTHER;
 }
 
-void ServiceWorkerMetrics::CountInitDiskCacheResult(bool result) {
-  UMA_HISTOGRAM_BOOLEAN("ServiceWorker.DiskCache.InitResult", result);
-}
-
 void ServiceWorkerMetrics::CountReadResponseResult(
     ServiceWorkerMetrics::ReadResponseResult result) {
   UMA_HISTOGRAM_ENUMERATION("ServiceWorker.DiskCache.ReadResponseResult",
@@ -254,17 +250,6 @@ void ServiceWorkerMetrics::CountWriteResponseResult(
                             result, NUM_WRITE_RESPONSE_RESULT_TYPES);
 }
 
-void ServiceWorkerMetrics::RecordPurgeResourceResult(int net_error) {
-  base::UmaHistogramSparse("ServiceWorker.Storage.PurgeResourceResult",
-                           std::abs(net_error));
-}
-
-void ServiceWorkerMetrics::RecordDeleteAndStartOverResult(
-    DeleteAndStartOverResult result) {
-  UMA_HISTOGRAM_ENUMERATION("ServiceWorker.Storage.DeleteAndStartOverResult",
-                            result, NUM_DELETE_AND_START_OVER_RESULT_TYPES);
-}
-
 void ServiceWorkerMetrics::CountControlledPageLoad(Site site,
                                                    bool is_main_frame_load) {
   DCHECK_NE(site, Site::OTHER);
@@ -565,10 +550,6 @@ void ServiceWorkerMetrics::RecordStartServiceWorkerForNavigationHintResult(
                             result);
 }
 
-void ServiceWorkerMetrics::RecordRegisteredOriginCount(size_t origin_count) {
-  UMA_HISTOGRAM_COUNTS_1M("ServiceWorker.RegisteredOriginCount", origin_count);
-}
-
 void ServiceWorkerMetrics::RecordLookupRegistrationTime(
     blink::ServiceWorkerStatusCode status,
     base::TimeDelta duration) {

content/browser/service_worker/service_worker_metrics.h

@@ -39,14 +39,6 @@ class ServiceWorkerMetrics {
     NUM_WRITE_RESPONSE_RESULT_TYPES,
   };
 
-  // Used for UMA. Append-only.
-  enum DeleteAndStartOverResult {
-    DELETE_OK,
-    DELETE_DATABASE_ERROR,
-    DELETE_DISK_CACHE_ERROR,
-    NUM_DELETE_AND_START_OVER_RESULT_TYPES,
-  };
-
   // Used for UMA. Append-only.
   enum class StopStatus {
     NORMAL,
@@ -179,15 +171,11 @@ class ServiceWorkerMetrics {
   // If the |url| is not a special site, returns Site::OTHER.
   static Site SiteFromURL(const GURL& url);
 
-  // Used for ServiceWorkerDiskCache.
-  static void CountInitDiskCacheResult(bool result);
+  // Counts the result of reading a service worker script from storage.
   static void CountReadResponseResult(ReadResponseResult result);
+  // Counts the result of writing a service worker script to storage.
   static void CountWriteResponseResult(WriteResponseResult result);
 
-  // Used for ServiceWorkerStorage.
-  static void RecordPurgeResourceResult(int net_error);
-  static void RecordDeleteAndStartOverResult(DeleteAndStartOverResult result);
-
   // Counts the number of page loads controlled by a Service Worker.
   static void CountControlledPageLoad(Site site,
                                       bool is_main_frame_load);
@@ -246,9 +234,6 @@ class ServiceWorkerMetrics {
   static void RecordStartServiceWorkerForNavigationHintResult(
       StartServiceWorkerForNavigationHintResult result);
 
-  // Records the number of origins with a registered service worker.
-  static void RecordRegisteredOriginCount(size_t origin_count);
-
   // Records the duration of looking up an existing registration.
   // |status| is the result of lookup. The records for the cases where
   // the registration is found (kOk), not found (kErrorNotFound), or an error

content/browser/service_worker/service_worker_storage.cc

@@ -12,6 +12,7 @@
 #include "base/bind_helpers.h"
 #include "base/files/file_util.h"
 #include "base/memory/ptr_util.h"
+#include "base/metrics/histogram_functions.h"
 #include "base/run_loop.h"
 #include "base/sequenced_task_runner.h"
 #include "base/task/post_task.h"
@@ -60,6 +61,19 @@ const base::FilePath::CharType kDatabaseName[] = FILE_PATH_LITERAL("Database");
 const base::FilePath::CharType kDiskCacheName[] =
     FILE_PATH_LITERAL("ScriptCache");
 
+// Used for UMA. Append-only.
+enum class DeleteAndStartOverResult {
+  kDeleteOk = 0,
+  kDeleteDatabaseError = 1,
+  kDeleteDiskCacheError = 2,
+  kMaxValue = kDeleteDiskCacheError,
+};
+
+void RecordDeleteAndStartOverResult(DeleteAndStartOverResult result) {
+  base::UmaHistogramEnumeration(
+      "ServiceWorker.Storage.DeleteAndStartOverResult", result);
+}
+
 }  // namespace
 
 ServiceWorkerStorage::InitialData::InitialData()
@@ -1069,8 +1083,8 @@ void ServiceWorkerStorage::DidReadInitialData(
     next_resource_id_ = data->next_resource_id;
     registered_origins_.swap(data->origins);
     state_ = STORAGE_STATE_INITIALIZED;
-    ServiceWorkerMetrics::RecordRegisteredOriginCount(
-        registered_origins_.size());
+    base::UmaHistogramCounts1M("ServiceWorker.RegisteredOriginCount",
+                               registered_origins_.size());
   } else {
     DVLOG(2) << "Failed to initialize: "
              << ServiceWorkerDatabase::StatusToString(status);
@@ -1232,7 +1246,8 @@ void ServiceWorkerStorage::OnDiskCacheInitialized(int rv) {
                << net::ErrorToString(rv);
     Disable();
   }
-  ServiceWorkerMetrics::CountInitDiskCacheResult(rv == net::OK);
+  base::UmaHistogramBoolean("ServiceWorker.DiskCache.InitResult",
+                            rv == net::OK);
 }
 
 void ServiceWorkerStorage::StartPurgingResources(
@@ -1282,7 +1297,8 @@ void ServiceWorkerStorage::OnResourcePurged(int64_t id, int rv) {
   DCHECK(is_purge_pending_);
   is_purge_pending_ = false;
 
-  ServiceWorkerMetrics::RecordPurgeResourceResult(rv);
+  base::UmaHistogramSparse("ServiceWorker.Storage.PurgeResourceResult",
+                           std::abs(rv));
 
   // TODO(falken): Is it always OK to ClearPurgeableResourceIds if |rv| is
   // failure? The disk cache entry might still remain and once we remove its
@@ -1654,8 +1670,8 @@ void ServiceWorkerStorage::DidDeleteDatabase(
     // Give up the corruption recovery until the browser restarts.
     LOG(ERROR) << "Failed to delete the database: "
                << ServiceWorkerDatabase::StatusToString(status);
-    ServiceWorkerMetrics::RecordDeleteAndStartOverResult(
-        ServiceWorkerMetrics::DELETE_DATABASE_ERROR);
+    RecordDeleteAndStartOverResult(
+        DeleteAndStartOverResult::kDeleteDatabaseError);
     std::move(callback).Run(status);
     return;
   }
@@ -1684,14 +1700,13 @@ void ServiceWorkerStorage::DidDeleteDiskCache(DatabaseStatusCallback callback,
   if (!result) {
     // Give up the corruption recovery until the browser restarts.
     LOG(ERROR) << "Failed to delete the diskcache.";
-    ServiceWorkerMetrics::RecordDeleteAndStartOverResult(
-        ServiceWorkerMetrics::DELETE_DISK_CACHE_ERROR);
+    RecordDeleteAndStartOverResult(
+        DeleteAndStartOverResult::kDeleteDiskCacheError);
     std::move(callback).Run(ServiceWorkerDatabase::Status::kErrorFailed);
     return;
   }
   DVLOG(1) << "Deleted ServiceWorkerDiskCache successfully.";
-  ServiceWorkerMetrics::RecordDeleteAndStartOverResult(
-      ServiceWorkerMetrics::DELETE_OK);
+  RecordDeleteAndStartOverResult(DeleteAndStartOverResult::kDeleteOk);
   std::move(callback).Run(ServiceWorkerDatabase::Status::kOk);
 }
 

content/browser/service_worker/service_worker_storage.h

@@ -24,7 +24,6 @@
 #include "components/services/storage/public/mojom/local_storage_control.mojom.h"
 #include "components/services/storage/public/mojom/service_worker_storage_control.mojom.h"
 #include "content/browser/service_worker/service_worker_database.h"
-#include "content/browser/service_worker/service_worker_metrics.h"
 #include "content/common/content_export.h"
 #include "url/gurl.h"