Initial round of cleanups now all events go through StartRequest.

The RequestType enum is no longer needed, and AddRequest can get merged
into StartRequest.

BUG=570820

Review URL: https://codereview.chromium.org/1652353003

Cr-Commit-Position: refs/heads/master@{#378537}

2 files changed, 50 insertions, 131 deletions

diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc
index 40081be..3645c01 100644
--- a/content/browser/service_worker/service_worker_version.cc
+++ b/content/browser/service_worker/service_worker_version.cc
@@ -90,33 +90,6 @@ void RunCallbacks(ServiceWorkerVersion* version,
     callback.Run(arg);
 }
 
-template <typename IDMAP, typename... Params>
-void RunIDMapCallbacks(IDMAP* requests, const Params&... params) {
-  typename IDMAP::iterator iter(requests);
-  while (!iter.IsAtEnd()) {
-    TRACE_EVENT_ASYNC_END0("ServiceWorker", "ServiceWorkerVersion::Request",
-                           iter.GetCurrentValue());
-    iter.GetCurrentValue()->callback.Run(params...);
-    iter.Advance();
-  }
-  requests->Clear();
-}
-
-template <typename CallbackType, typename... Params>
-bool RunIDMapCallback(IDMap<CallbackType, IDMapOwnPointer>* requests,
-                      int request_id,
-                      const Params&... params) {
-  CallbackType* request = requests->Lookup(request_id);
-  if (!request)
-    return false;
-
-  TRACE_EVENT_ASYNC_END0("ServiceWorker", "ServiceWorkerVersion::Request",
-                         request);
-  request->callback.Run(params...);
-  requests->Remove(request_id);
-  return true;
-}
-
 void RunStartWorkerCallback(
     const StatusCallback& callback,
     scoped_refptr<ServiceWorkerRegistration> protect,
@@ -486,17 +459,9 @@ void ServiceWorkerVersion::DeferScheduledUpdate() {
 int ServiceWorkerVersion::StartRequest(
     ServiceWorkerMetrics::EventType event_type,
     const StatusCallback& error_callback) {
-  OnBeginEvent();
-  DCHECK_EQ(RUNNING, running_status())
-      << "Can only start a request with a running worker.";
-  DCHECK(event_type == ServiceWorkerMetrics::EventType::INSTALL ||
-         event_type == ServiceWorkerMetrics::EventType::ACTIVATE ||
-         event_type == ServiceWorkerMetrics::EventType::MESSAGE ||
-         status() == ACTIVATED)
-      << "Event of type " << static_cast<int>(event_type)
-      << " can only be dispatched to an active worker: " << status();
-  return AddRequest(error_callback, &custom_requests_, REQUEST_CUSTOM,
-                    event_type);
+  return StartRequestWithCustomTimeout(
+      event_type, error_callback,
+      base::TimeDelta::FromMinutes(kRequestTimeoutMinutes), KILL_ON_TIMEOUT);
 }
 
 int ServiceWorkerVersion::StartRequestWithCustomTimeout(
@@ -507,9 +472,23 @@ int ServiceWorkerVersion::StartRequestWithCustomTimeout(
   OnBeginEvent();
   DCHECK_EQ(RUNNING, running_status())
       << "Can only start a request with a running worker.";
-  return AddRequestWithExpiration(
-      error_callback, &custom_requests_, REQUEST_CUSTOM, event_type,
-      base::TimeTicks::Now() + timeout, timeout_behavior);
+  DCHECK(event_type == ServiceWorkerMetrics::EventType::INSTALL ||
+         event_type == ServiceWorkerMetrics::EventType::ACTIVATE ||
+         event_type == ServiceWorkerMetrics::EventType::MESSAGE ||
+         status() == ACTIVATED)
+      << "Event of type " << static_cast<int>(event_type)
+      << " can only be dispatched to an active worker: " << status();
+
+  PendingRequest<StatusCallback>* request = new PendingRequest<StatusCallback>(
+      error_callback, base::TimeTicks::Now(), event_type);
+  int request_id = custom_requests_.Add(request);
+  TRACE_EVENT_ASYNC_BEGIN2("ServiceWorker", "ServiceWorkerVersion::Request",
+                           request, "Request id", request_id, "Event type",
+                           ServiceWorkerMetrics::EventTypeToString(event_type));
+  base::TimeTicks expiration_time = base::TimeTicks::Now() + timeout;
+  requests_.push(
+      RequestInfo(request_id, event_type, expiration_time, timeout_behavior));
+  return request_id;
 }
 
 bool ServiceWorkerVersion::FinishRequest(int request_id, bool was_handled) {
@@ -521,7 +500,16 @@ bool ServiceWorkerVersion::FinishRequest(int request_id, bool was_handled) {
   ServiceWorkerMetrics::RecordEventDuration(
       request->event_type, base::TimeTicks::Now() - request->start_time,
       was_handled);
-  RemoveCallbackAndStopIfRedundant(&custom_requests_, request_id);
+
+  RestartTick(&idle_time_);
+  TRACE_EVENT_ASYNC_END1("ServiceWorker", "ServiceWorkerVersion::Request",
+                         request, "Handled", was_handled);
+  custom_requests_.Remove(request_id);
+  if (is_redundant()) {
+    // The stop should be already scheduled, but try to stop immediately, in
+    // order to release worker resources soon.
+    StopWorkerIfIdle();
+  }
   return true;
 }
 
@@ -747,12 +735,10 @@ ServiceWorkerVersion::GetMainScriptHttpResponseInfo() {
 
 ServiceWorkerVersion::RequestInfo::RequestInfo(
     int id,
-    RequestType type,
     ServiceWorkerMetrics::EventType event_type,
     const base::TimeTicks& expiration,
     TimeoutBehavior timeout_behavior)
     : id(id),
-      type(type),
       event_type(event_type),
       expiration(expiration),
       timeout_behavior(timeout_behavior) {}
@@ -783,8 +769,8 @@ ServiceWorkerVersion::BaseMojoServiceWrapper::~BaseMojoServiceWrapper() {
   while (!iter.IsAtEnd()) {
     PendingRequest<StatusCallback>* request = iter.GetCurrentValue();
     if (request->mojo_service == service_name_) {
-      TRACE_EVENT_ASYNC_END0("ServiceWorker", "ServiceWorkerVersion::Request",
-                             request);
+      TRACE_EVENT_ASYNC_END1("ServiceWorker", "ServiceWorkerVersion::Request",
+                             request, "Error", "Service Disconnected");
       request->callback.Run(SERVICE_WORKER_ERROR_FAILED);
       worker_->custom_requests_.Remove(iter.GetCurrentKey());
     }
@@ -1587,65 +1573,16 @@ void ServiceWorkerVersion::RecordStartWorkerResult(
                             EmbeddedWorkerInstance::STARTING_PHASE_MAX_VALUE);
 }
 
-template <typename IDMAP>
-void ServiceWorkerVersion::RemoveCallbackAndStopIfRedundant(IDMAP* callbacks,
-                                                            int request_id) {
-  RestartTick(&idle_time_);
-  auto* request = callbacks->Lookup(request_id);
-  if (request) {
-    TRACE_EVENT_ASYNC_END0("ServiceWorker", "ServiceWorkerVersion::Request",
-                           request);
-  }
-  callbacks->Remove(request_id);
-  if (is_redundant()) {
-    // The stop should be already scheduled, but try to stop immediately, in
-    // order to release worker resources soon.
-    StopWorkerIfIdle();
-  }
-}
-
-template <typename CallbackType>
-int ServiceWorkerVersion::AddRequest(
-    const CallbackType& callback,
-    IDMap<PendingRequest<CallbackType>, IDMapOwnPointer>* callback_map,
-    RequestType request_type,
-    ServiceWorkerMetrics::EventType event_type) {
-  base::TimeTicks expiration_time =
-      base::TimeTicks::Now() +
-      base::TimeDelta::FromMinutes(kRequestTimeoutMinutes);
-  return AddRequestWithExpiration(callback, callback_map, request_type,
-                                  event_type, expiration_time, KILL_ON_TIMEOUT);
-}
-
-template <typename CallbackType>
-int ServiceWorkerVersion::AddRequestWithExpiration(
-    const CallbackType& callback,
-    IDMap<PendingRequest<CallbackType>, IDMapOwnPointer>* callback_map,
-    RequestType request_type,
-    ServiceWorkerMetrics::EventType event_type,
-    base::TimeTicks expiration,
-    TimeoutBehavior timeout_behavior) {
-  PendingRequest<CallbackType>* request = new PendingRequest<CallbackType>(
-      callback, base::TimeTicks::Now(), event_type);
-  int request_id = callback_map->Add(request);
-  TRACE_EVENT_ASYNC_BEGIN2("ServiceWorker", "ServiceWorkerVersion::Request",
-                           request, "Request id", request_id, "Event type",
-                           ServiceWorkerMetrics::EventTypeToString(event_type));
-  requests_.push(RequestInfo(request_id, request_type, event_type, expiration,
-                             timeout_behavior));
-  return request_id;
-}
-
 bool ServiceWorkerVersion::MaybeTimeOutRequest(const RequestInfo& info) {
-  switch (info.type) {
-    case REQUEST_CUSTOM:
-      return RunIDMapCallback(&custom_requests_, info.id,
-                              SERVICE_WORKER_ERROR_TIMEOUT);
-    case NUM_REQUEST_TYPES:
-      break;
-  }
-  NOTREACHED() << "Got unexpected request type: " << info.type;
-  return false;
+  PendingRequest<StatusCallback>* request = custom_requests_.Lookup(info.id);
+  if (!request)
+    return false;
+
+  TRACE_EVENT_ASYNC_END1("ServiceWorker", "ServiceWorkerVersion::Request",
+                         request, "Error", "Timeout");
+  request->callback.Run(SERVICE_WORKER_ERROR_TIMEOUT);
+  custom_requests_.Remove(info.id);
+  return true;
 }
 
 void ServiceWorkerVersion::SetAllRequestExpirations(
@@ -1749,7 +1686,15 @@ void ServiceWorkerVersion::OnStoppedInternal(
   // Let all message callbacks fail (this will also fire and clear all
   // callbacks for events).
   // TODO(kinuko): Consider if we want to add queue+resend mechanism here.
-  RunIDMapCallbacks(&custom_requests_, SERVICE_WORKER_ERROR_FAILED);
+  IDMap<PendingRequest<StatusCallback>, IDMapOwnPointer>::iterator iter(
+      &custom_requests_);
+  while (!iter.IsAtEnd()) {
+    TRACE_EVENT_ASYNC_END1("ServiceWorker", "ServiceWorkerVersion::Request",
+                           iter.GetCurrentValue(), "Error", "Worker Stopped");
+    iter.GetCurrentValue()->callback.Run(SERVICE_WORKER_ERROR_FAILED);
+    iter.Advance();
+  }
+  custom_requests_.Clear();
 
   // Close all mojo services. This will also fire and clear all callbacks
   // for messages that are still outstanding for those services.
diff --git a/content/browser/service_worker/service_worker_version.h b/content/browser/service_worker/service_worker_version.h
index 95109ba..cdc829b 100644
--- a/content/browser/service_worker/service_worker_version.h
+++ b/content/browser/service_worker/service_worker_version.h
@@ -380,21 +380,14 @@ class CONTENT_EXPORT ServiceWorkerVersion
   class Metrics;
   class PingController;
 
-  enum RequestType {
-    REQUEST_CUSTOM,
-    NUM_REQUEST_TYPES
-  };
-
   struct RequestInfo {
     RequestInfo(int id,
-                RequestType type,
                 ServiceWorkerMetrics::EventType event_type,
                 const base::TimeTicks& expiration,
                 TimeoutBehavior timeout_behavior);
     ~RequestInfo();
     bool operator>(const RequestInfo& other) const;
     int id;
-    RequestType type;
     ServiceWorkerMetrics::EventType event_type;
     base::TimeTicks expiration;
     TimeoutBehavior timeout_behavior;
@@ -608,25 +601,6 @@ class CONTENT_EXPORT ServiceWorkerVersion
   // and records metrics about startup.
   void RecordStartWorkerResult(ServiceWorkerStatusCode status);
 
-  template <typename IDMAP>
-  void RemoveCallbackAndStopIfRedundant(IDMAP* callbacks, int request_id);
-
-  template <typename CallbackType>
-  int AddRequest(
-      const CallbackType& callback,
-      IDMap<PendingRequest<CallbackType>, IDMapOwnPointer>* callback_map,
-      RequestType request_type,
-      ServiceWorkerMetrics::EventType event_type);
-
-  template <typename CallbackType>
-  int AddRequestWithExpiration(
-      const CallbackType& callback,
-      IDMap<PendingRequest<CallbackType>, IDMapOwnPointer>* callback_map,
-      RequestType request_type,
-      ServiceWorkerMetrics::EventType event_type,
-      base::TimeTicks expiration,
-      TimeoutBehavior timeout_behavior);
-
   bool MaybeTimeOutRequest(const RequestInfo& info);
   void SetAllRequestExpirations(const base::TimeTicks& expiration);