service worker: Attribute purpose to start worker attempts for UMA

content/browser/service_worker/service_worker_version.h

Index: content/browser/service_worker/service_worker_version.h
diff --git a/content/browser/service_worker/service_worker_version.h b/content/browser/service_worker/service_worker_version.h
index e9e3cc96057ebfb73f601784336ed64269d2a57a..e6586d16da807ce63b7936e47a2a88982fb40b4a 100644
--- a/content/browser/service_worker/service_worker_version.h
+++ b/content/browser/service_worker/service_worker_version.h
@@ -164,7 +164,9 @@ class CONTENT_EXPORT ServiceWorkerVersion
 
   // Starts an embedded worker for this version.
   // This returns OK (success) if the worker is already running.
-  void StartWorker(const StatusCallback& callback);
+  void StartWorker(const StatusCallback& callback,
+                   ServiceWorkerMetrics::EventType purpose =
+                       ServiceWorkerMetrics::EventType::UNKNOWN);

[nhiroki, 2016/03/15 02:18:14]

When is this argument omitted? Is it only in unittests?

[nhiroki, 2016/03/15 02:27:48]

IMHO, if there are no strong needs of the default argument, I'd prefer to
explicitly pass an argument instead of using a default argument because
production code could accidentally use the default argument w/o any compile
failure and UMA results could be mixed up.

[falken, 2016/03/15 05:15:53]

Yes good point. Actually I was just too lazy to change all the callsites, but
that's dangerous as you say. Removed the default.

 
   // Stops an embedded worker for this version.
   // This returns OK (success) if the worker is already stopped.
@@ -185,7 +187,8 @@ class CONTENT_EXPORT ServiceWorkerVersion
   // If the worker is already running, |task| is executed synchronously (before
   // this method returns).
   void RunAfterStartWorker(const base::Closure& task,
-                           const StatusCallback& error_callback);
+                           const StatusCallback& error_callback,
+                           ServiceWorkerMetrics::EventType purpose);

[nhiroki, 2016/03/15 02:18:14]

This argument order is different from StartRequest()'s order. Can you make them
consistent?

[falken, 2016/03/15 05:15:53]

Done.

 
   // Call this while the worker is running before dispatching an event to the
   // worker. This informs ServiceWorkerVersion about the event in progress.
@@ -580,6 +583,7 @@ class CONTENT_EXPORT ServiceWorkerVersion
                                     const std::vector<url::Origin>& origins);
 
   void DidEnsureLiveRegistrationForStartWorker(
+      ServiceWorkerMetrics::EventType purpose,
       const StatusCallback& callback,
       ServiceWorkerStatusCode status,
       const scoped_refptr<ServiceWorkerRegistration>& registration);
@@ -610,7 +614,8 @@ class CONTENT_EXPORT ServiceWorkerVersion
 
   // RecordStartWorkerResult is added as a start callback by StartTimeoutTimer
   // and records metrics about startup.
-  void RecordStartWorkerResult(ServiceWorkerStatusCode status);
+  void RecordStartWorkerResult(ServiceWorkerMetrics::EventType purpose,
+                               ServiceWorkerStatusCode status);
 
   bool MaybeTimeOutRequest(const RequestInfo& info);
   void SetAllRequestExpirations(const base::TimeTicks& expiration);