commit 4c80fd041966f945c35f4f3c42cc0648c06974f4
author Justin Lulejian <jlulejian@chromium.org> Tue Jan 02 21:26:32 2024
committer Chromium LUCI CQ <chromium-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Jan 02 21:26:32 2024
tree 550d99a3676614dce63bd400b26e0c2ae5e9bb3c
parent d3a35d8b6e1f0094ab08007ed9ff38761f86daec

[Reland][Extensions] Avoid starting a worker when it is already running.

Reland notes: This is a reland of crrev.com/c/4868638. It contains a
speculative fix for crbug.com/1490783.

The two significant changes in the reland  are:
  1) ServiceWorkerTaskQueue::ShouldEnqueueTask() now does the same
    running check that we do when we
    ServiceWorkerTaskQueue::AddPendingTask(). This is to avoid the
    infinite loop that could be encountered when
    MessageService::MaybeAddPendingLazyContextOpenChannelTask() calls
    ServiceWorkerTaskQueue::ShouldEnqueueTask(). The loops occurs when
    this is combined with ServiceWorkerTaskQueue::AddPendingTask()
    immediately running the
    MessageService::PendingLazyContextPostMessage() callback from
    MessageService::EnqueuePendingMessageForLazyBackgroundLoad()
    immediately when the worker is running -- not allowing the port
    channel to be created before it stack overflows.
  2) It replaces ServiceWorkerTaskQueue::CanWorkerImmediatelyRunTasks()
    with ServiceWorkerTaskQueue::IsServiceWorkerRunning() from
    crrev.com/c/5042110. This should be a no-op.
  3) The fix in ShouldEnqueueTask() breaks a web navigation test that
    relies on the different behavior between service workers and event
    pages. This feature fixes/change that so I've temporarily duplicated
    the test for SWs until this feature is the default.

=== Original Change ===
[Extensions] Avoid starting a worker when it is already running.

This removes one case where we start a service worker in response to an
event needing to be dispatched.

Before this change we didn't check the running status of a worker and
instead would start a worker if there were no pending tasks. A worker in
blink::EmbeddedWorkerStatus::kRunning status is ready to receive events
immediately though.

This practically means that if multiple tasks built-up momentarily
before ServiceWorkerTaskQueue::DidStartWorkerForScope() finished running
we wouldn't try to start a worker. But all other times we would. For
example: taskA is dispatched and causes the worker to start. TaskB
comes into ServiceWorkerTaskQueue::AddPendingTask() a moment later after
the worker has started (when the worker is most likely still running)
and we redundantly attempt to start the worker.

After this change if the worker has
blink::EmbeddedWorkerStatus::kRunning status when TaskB arrives it will
not cause us to redundantly attempt to start the worker when it is in
this status.

This new behavior is disabled by default and will be enabled with via a
rollout due to its potential wide impact on MV3/service workers.


  service_worker_context_wrapper.h, service_worker_context_core.cc, and
  service_worker_context_core.h are test code.

Low-Coverage-Reason: OTHER uncovered lines in
Bug: 1467015
Change-Id: Ib5455798785684540db9ab51d0ae4d2127bc0c50
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5048271
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Commit-Queue: Justin Lulejian <jlulejian@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1242139}