Commit 1ec589f2 authored by Makoto Shimazu's avatar Makoto Shimazu Committed by Commit Bot

Revert "ServiceWorker: Start the timer immediately after queuing an event"

This reverts commit 8a93811e.

Reason for revert: https://crbug.com/1134552

Original change's description:
> 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}

TBR=falken@chromium.org,shimazu@chromium.org,asamidoi@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 965802, 1134552
Change-Id: I61e03383b7147d40ebfa0413a7fe6345202e015d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2452092Reviewed-by: default avatarMakoto Shimazu <shimazu@chromium.org>
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814273}
parent edb85419
...@@ -129,16 +129,7 @@ void ServiceWorkerEventQueue::EnqueueEvent(std::unique_ptr<Event> event) { ...@@ -129,16 +129,7 @@ void ServiceWorkerEventQueue::EnqueueEvent(std::unique_ptr<Event> event) {
DCHECK(event->type != Event::Type::Pending || did_idle_timeout()); DCHECK(event->type != Event::Type::Pending || did_idle_timeout());
bool can_start_processing_events = bool can_start_processing_events =
!processing_events_ && event->type != Event::Type::Pending; !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) if (!can_start_processing_events)
return; return;
...@@ -150,11 +141,8 @@ void ServiceWorkerEventQueue::EnqueueEvent(std::unique_ptr<Event> event) { ...@@ -150,11 +141,8 @@ void ServiceWorkerEventQueue::EnqueueEvent(std::unique_ptr<Event> event) {
void ServiceWorkerEventQueue::ProcessEvents() { void ServiceWorkerEventQueue::ProcessEvents() {
DCHECK(!processing_events_); DCHECK(!processing_events_);
processing_events_ = true; processing_events_ = true;
while (!queue_.empty() && CanStartEvent(*queue_.begin()->second)) { while (!queue_.IsEmpty() && CanStartEvent(*queue_.front())) {
int event_id = queue_.begin()->first; StartEvent(queue_.TakeFirst());
std::unique_ptr<Event> event = std::move(queue_.begin()->second);
queue_.erase(queue_.begin());
StartEvent(event_id, std::move(event));
} }
processing_events_ = false; processing_events_ = false;
...@@ -166,10 +154,16 @@ void ServiceWorkerEventQueue::ProcessEvents() { ...@@ -166,10 +154,16 @@ void ServiceWorkerEventQueue::ProcessEvents() {
OnNoInflightEvent(); OnNoInflightEvent();
} }
void ServiceWorkerEventQueue::StartEvent(int event_id, void ServiceWorkerEventQueue::StartEvent(std::unique_ptr<Event> event) {
std::unique_ptr<Event> event) { DCHECK(CanStartEvent(*event));
DCHECK(HasEvent(event_id));
running_offline_events_ = event->type == Event::Type::Offline; 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_) if (before_start_event_callback_)
before_start_event_callback_.Run(event->type == Event::Type::Offline); before_start_event_callback_.Run(event->type == Event::Type::Offline);
std::move(event->start_callback).Run(event_id); std::move(event->start_callback).Run(event_id);
...@@ -242,7 +236,6 @@ void ServiceWorkerEventQueue::UpdateStatus() { ...@@ -242,7 +236,6 @@ void ServiceWorkerEventQueue::UpdateStatus() {
new_id_event_map.insert(it.key, std::move(event_info)); new_id_event_map.insert(it.key, std::move(event_info));
continue; continue;
} }
queue_.erase(it.key);
std::move(event_info->abort_callback) std::move(event_info->abort_callback)
.Run(blink::mojom::ServiceWorkerEventStatus::TIMEOUT); .Run(blink::mojom::ServiceWorkerEventStatus::TIMEOUT);
should_idle_delay_to_be_zero = true; should_idle_delay_to_be_zero = true;
...@@ -287,7 +280,7 @@ void ServiceWorkerEventQueue::OnNoInflightEvent() { ...@@ -287,7 +280,7 @@ void ServiceWorkerEventQueue::OnNoInflightEvent() {
running_offline_events_ = false; running_offline_events_ = false;
// There might be events in the queue because offline (or non-offline) events // There might be events in the queue because offline (or non-offline) events
// can be enqueued during running non-offline (or offline) events. // can be enqueued during running non-offline (or offline) events.
if (!queue_.empty()) { if (!queue_.IsEmpty()) {
ProcessEvents(); ProcessEvents();
return; return;
} }
...@@ -296,8 +289,7 @@ void ServiceWorkerEventQueue::OnNoInflightEvent() { ...@@ -296,8 +289,7 @@ void ServiceWorkerEventQueue::OnNoInflightEvent() {
} }
bool ServiceWorkerEventQueue::HasInflightEvent() const { bool ServiceWorkerEventQueue::HasInflightEvent() const {
return id_event_map_.size() - queue_.size() > 0 || return !id_event_map_.IsEmpty() || num_of_stay_awake_tokens_ > 0;
num_of_stay_awake_tokens_ > 0;
} }
void ServiceWorkerEventQueue::ResetIdleTimeout() { void ServiceWorkerEventQueue::ResetIdleTimeout() {
......
...@@ -128,7 +128,7 @@ class MODULES_EXPORT ServiceWorkerEventQueue { ...@@ -128,7 +128,7 @@ class MODULES_EXPORT ServiceWorkerEventQueue {
// ServiceWorkerEventQueue periodically updates the timeout state by // ServiceWorkerEventQueue periodically updates the timeout state by
// kUpdateInterval. // kUpdateInterval.
static constexpr base::TimeDelta kUpdateInterval = static constexpr base::TimeDelta kUpdateInterval =
base::TimeDelta::FromSeconds(10); base::TimeDelta::FromSeconds(30);
private: private:
// Represents an event dispatch, which can be queued into |queue_|. // Represents an event dispatch, which can be queued into |queue_|.
...@@ -177,7 +177,7 @@ class MODULES_EXPORT ServiceWorkerEventQueue { ...@@ -177,7 +177,7 @@ class MODULES_EXPORT ServiceWorkerEventQueue {
bool CanStartEvent(const Event& event) const; bool CanStartEvent(const Event& event) const;
// Starts a single event. // Starts a single event.
void StartEvent(int event_id, std::unique_ptr<Event> event); void StartEvent(std::unique_ptr<Event> event);
// Updates the internal states and fires the event timeout callbacks if any. // Updates the internal states and fires the event timeout callbacks if any.
// TODO(shimazu): re-implement it by delayed tasks and cancelable callbacks. // TODO(shimazu): re-implement it by delayed tasks and cancelable callbacks.
...@@ -246,9 +246,7 @@ class MODULES_EXPORT ServiceWorkerEventQueue { ...@@ -246,9 +246,7 @@ class MODULES_EXPORT ServiceWorkerEventQueue {
bool did_idle_timeout_ = false; bool did_idle_timeout_ = false;
// Event queue to where all events are enqueued. // Event queue to where all events are enqueued.
// We use std::map as a task queue because it's ordered by the `event_id` and Deque<std::unique_ptr<Event>> queue_;
// 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 // Set to true during running ProcessEvents(). This is used for avoiding to
// invoke |idle_callback_| or to re-enter ProcessEvents() when calling // invoke |idle_callback_| or to re-enter ProcessEvents() when calling
......
...@@ -474,7 +474,7 @@ TEST_F(ServiceWorkerEventQueueTest, SetIdleTimerDelayToZero) { ...@@ -474,7 +474,7 @@ TEST_F(ServiceWorkerEventQueueTest, SetIdleTimerDelayToZero) {
} }
} }
TEST_F(ServiceWorkerEventQueueTest, EnqueueOffline) { TEST_F(ServiceWorkerEventQueueTest, EnqueuOffline) {
ServiceWorkerEventQueue event_queue(base::DoNothing(), base::DoNothing(), ServiceWorkerEventQueue event_queue(base::DoNothing(), base::DoNothing(),
task_runner(), task_runner(),
task_runner()->GetMockTickClock()); task_runner()->GetMockTickClock());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment