ServiceWorker: Start the timer immediately after queuing an event

This CL makes the timer start immediately after an event is enqueued,
and erases the event from the queue when it has not started to run yet
and the timeout happens. Also, this CL updates kUpdateInterval because
the minimum possible timeout is 10 seconds for offline events.

Bug: 965802
Change-Id: Ib84c6f3164aa00d525d2f936385a152fdad1fa49
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2423698
Commit-Queue: Asami Doi <asamidoi@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812028}

third_party/blink/renderer/modules/service_worker/service_worker_event_queue.cc

@@ -129,7 +129,16 @@ void ServiceWorkerEventQueue::EnqueueEvent(std::unique_ptr<Event> event) {
   DCHECK(event->type != Event::Type::Pending || did_idle_timeout());
   bool can_start_processing_events =
       !processing_events_ && event->type != Event::Type::Pending;
-  queue_.emplace_back(std::move(event));
+
+  const int event_id = NextEventId();
+  // Start counting the timer when an event is enqueued.
+  id_event_map_.insert(
+      event_id, std::make_unique<EventInfo>(
+                    tick_clock_->NowTicks() +
+                        event->custom_timeout.value_or(kEventTimeout),
+                    WTF::Bind(std::move(event->abort_callback), event_id)));
+
+  queue_.emplace(event_id, std::move(event));
 
   if (!can_start_processing_events)
     return;
@@ -141,8 +150,11 @@ void ServiceWorkerEventQueue::EnqueueEvent(std::unique_ptr<Event> event) {
 void ServiceWorkerEventQueue::ProcessEvents() {
   DCHECK(!processing_events_);
   processing_events_ = true;
-  while (!queue_.IsEmpty() && CanStartEvent(*queue_.front())) {
-    StartEvent(queue_.TakeFirst());
+  while (!queue_.empty() && CanStartEvent(*queue_.begin()->second)) {
+    int event_id = queue_.begin()->first;
+    std::unique_ptr<Event> event = std::move(queue_.begin()->second);
+    queue_.erase(queue_.begin());
+    StartEvent(event_id, std::move(event));
   }
   processing_events_ = false;
 
@@ -154,16 +166,10 @@ void ServiceWorkerEventQueue::ProcessEvents() {
     OnNoInflightEvent();
 }
 
-void ServiceWorkerEventQueue::StartEvent(std::unique_ptr<Event> event) {
-  DCHECK(CanStartEvent(*event));
+void ServiceWorkerEventQueue::StartEvent(int event_id,
+                                         std::unique_ptr<Event> event) {
+  DCHECK(HasEvent(event_id));
   running_offline_events_ = event->type == Event::Type::Offline;
-  const int event_id = NextEventId();
-  DCHECK(!HasEvent(event_id));
-  id_event_map_.insert(
-      event_id, std::make_unique<EventInfo>(
-                    tick_clock_->NowTicks() +
-                        event->custom_timeout.value_or(kEventTimeout),
-                    WTF::Bind(std::move(event->abort_callback), event_id)));
   if (before_start_event_callback_)
     before_start_event_callback_.Run(event->type == Event::Type::Offline);
   std::move(event->start_callback).Run(event_id);
@@ -236,6 +242,7 @@ void ServiceWorkerEventQueue::UpdateStatus() {
       new_id_event_map.insert(it.key, std::move(event_info));
       continue;
     }
+    queue_.erase(it.key);
     std::move(event_info->abort_callback)
         .Run(blink::mojom::ServiceWorkerEventStatus::TIMEOUT);
     should_idle_delay_to_be_zero = true;
@@ -280,7 +287,7 @@ void ServiceWorkerEventQueue::OnNoInflightEvent() {
   running_offline_events_ = false;
   // There might be events in the queue because offline (or non-offline) events
   // can be enqueued during running non-offline (or offline) events.
-  if (!queue_.IsEmpty()) {
+  if (!queue_.empty()) {
     ProcessEvents();
     return;
   }
@@ -289,7 +296,8 @@ void ServiceWorkerEventQueue::OnNoInflightEvent() {
 }
 
 bool ServiceWorkerEventQueue::HasInflightEvent() const {
-  return !id_event_map_.IsEmpty() || num_of_stay_awake_tokens_ > 0;
+  return id_event_map_.size() - queue_.size() > 0 ||
+         num_of_stay_awake_tokens_ > 0;
 }
 
 void ServiceWorkerEventQueue::ResetIdleTimeout() {

third_party/blink/renderer/modules/service_worker/service_worker_event_queue.h

@@ -128,7 +128,7 @@ class MODULES_EXPORT ServiceWorkerEventQueue {
   // ServiceWorkerEventQueue periodically updates the timeout state by
   // kUpdateInterval.
   static constexpr base::TimeDelta kUpdateInterval =
-      base::TimeDelta::FromSeconds(30);
+      base::TimeDelta::FromSeconds(10);
 
  private:
   // Represents an event dispatch, which can be queued into |queue_|.
@@ -177,7 +177,7 @@ class MODULES_EXPORT ServiceWorkerEventQueue {
   bool CanStartEvent(const Event& event) const;
 
   // Starts a single event.
-  void StartEvent(std::unique_ptr<Event> event);
+  void StartEvent(int event_id, std::unique_ptr<Event> event);
 
   // Updates the internal states and fires the event timeout callbacks if any.
   // TODO(shimazu): re-implement it by delayed tasks and cancelable callbacks.
@@ -246,7 +246,9 @@ class MODULES_EXPORT ServiceWorkerEventQueue {
   bool did_idle_timeout_ = false;
 
   // Event queue to where all events are enqueued.
-  Deque<std::unique_ptr<Event>> queue_;
+  // We use std::map as a task queue because it's ordered by the `event_id` and
+  // the entries can be effectively erased in random order.
+  std::map<int /* event_id */, std::unique_ptr<Event>> queue_;
 
   // Set to true during running ProcessEvents(). This is used for avoiding to
   // invoke |idle_callback_| or to re-enter ProcessEvents() when calling

third_party/blink/renderer/modules/service_worker/service_worker_event_queue_test.cc

@@ -474,7 +474,7 @@ TEST_F(ServiceWorkerEventQueueTest, SetIdleTimerDelayToZero) {
   }
 }
 
-TEST_F(ServiceWorkerEventQueueTest, EnqueuOffline) {
+TEST_F(ServiceWorkerEventQueueTest, EnqueueOffline) {
   ServiceWorkerEventQueue event_queue(base::DoNothing(), base::DoNothing(),
                                       task_runner(),
                                       task_runner()->GetMockTickClock());