service worker: Attribute purpose to start worker attempts for UMA

content/browser/service_worker/service_worker_version.h

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_VERSION_H_
 #define CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_VERSION_H_
 
 #include <stdint.h>
 
 #include <functional>
@@ skipping 146 matching lines @@
   // if there're any (see RegisterStatusChangeCallback).
   void SetStatus(Status status);
 
   // Registers status change callback. (This is for one-off observation,
   // the consumer needs to re-register if it wants to continue observing
   // status changes)
   void RegisterStatusChangeCallback(const base::Closure& callback);
 
   // 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.
   void StopWorker(const StatusCallback& callback);
 
   // Schedules an update to be run 'soon'.
   void ScheduleUpdate();
 
   // If an update is scheduled but not yet started, this resets the timer
   // delaying the start time by a 'small' amount.
   void DeferScheduledUpdate();
 
   // Starts an update now.
   void StartUpdate();
 
   // Starts the worker if it isn't already running, and calls |task| when the
   // worker is running, or |error_callback| if starting the worker failed.
   // 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.
   // Returns a request id, which should later be passed to FinishRequest when
   // the event finished.
   // The |error_callback| is called if either ServiceWorkerVersion decides the
   // event is taking too long, or if for some reason the worker stops or is
   // killed before the request finishes.
   int StartRequest(ServiceWorkerMetrics::EventType event_type,
                    const StatusCallback& error_callback);
@@ skipping 374 matching lines @@
   void OnClaimClients(int request_id);
   void OnPongFromWorker();
 
   void OnFocusClientFinished(int request_id,
                              const ServiceWorkerClientInfo& client_info);
 
   void OnRegisterForeignFetchScopes(const std::vector<GURL>& sub_scopes,
                                     const std::vector<url::Origin>& origins);
 
   void DidEnsureLiveRegistrationForStartWorker(
+      ServiceWorkerMetrics::EventType purpose,
       const StatusCallback& callback,
       ServiceWorkerStatusCode status,
       const scoped_refptr<ServiceWorkerRegistration>& registration);
   void StartWorkerInternal();
 
   void DidSkipWaiting(int request_id);
 
   void OnGetClientFinished(int request_id,
                            const ServiceWorkerClientInfo& client_info);
 
@@ skipping 10 matching lines @@
   ServiceWorkerStatusCode PingWorker();
   void OnPingTimeout();
 
   // Stops the worker if it is idle (has no in-flight requests) or timed out
   // ping.
   void StopWorkerIfIdle();
   bool HasInflightRequests() const;
 
   // 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);
 
   // Returns the reason the embedded worker failed to start, using information
   // inaccessible to EmbeddedWorkerInstance. Returns |default_code| if it can't
   // deduce a reason.
   ServiceWorkerStatusCode DeduceStartWorkerFailureReason(
       ServiceWorkerStatusCode default_code);
 
@@ skipping 168 matching lines @@
 
   // At this point |this| can have been deleted, so don't do anything other
   // than returning.
 
   return true;
 }
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_VERSION_H_