Editorial: use a dedicated parallel queue for the Clients API

[monica-ch]

Closes#1840. Follow-up to #1755.

Give each {{Clients}} object a dedicated parallel queue and route the operations on {{Clients}} through it, so operations on the same {{Clients}} object do not race with each other (e.g. matchAll iterating over service worker clients while claim is mutating their [=active service worker=]).

Changes

• Add a <dfn export for="Clients">Clients parallel queue</dfn> (a [=parallel queue=]) attached to each {{Clients}} object, following the same pattern as the name to cache map parallel queue introduced in Editorial: serialize CacheStorage access on a dedicated parallel queue #1838.
• Route the following method algorithms through it, replacing their existing top-level Run … in parallel block:
  • {{Clients/get(id)}}
  • {{Clients/matchAll(options)}}
  • {{Clients/openWindow(url)}}
  • {{Clients/claim()}}
• Wrap the two remaining bare Resolve |promise| with undefined. steps (in get() and claim()) in Queue a task on |promise|'s [=responsible event loop=] using the [=DOM manipulation task source=], matching the pattern already used in matchAll() and consistent with the queue-a-task refactor from Editorial: Queue a task to resolve/reject promise or when fire an event. #1755.

Total change: +10 / −6 in index.bs, one commit.

Rationale

Yoshi flagged this concern during #1755 review:

I just wondered what happens if one of the service worker clients has been removed or gets execution ready flag during the sub step execution, and suggest to run Clients API algorithm within the dedicated parallel queue to prevent unexpected modifications to clients.

#1836 landed the queue-a-task-for-resolve fixes for these methods. This PR delivers the sibling parallel-queue work that was explicitly deferred in the split plan.

Out of scope

• {{Client/postMessage(message, options)}} — lives on {{Client}}, not {{Clients}}, and does a single-client lookup rather than iterating the full client list. Can be a further follow-up if needed.
• {{WindowClient/focus()}} and {{WindowClient/navigate(url)}} — already skip the in parallel block entirely and use Queue a task on the client's own event loop, so no parallel-queue treatment is meaningful for them.

Related

• Editorial: Queue a task to resolve/reject promise or when fire an event. #1755 — parent (queue-a-task refactor)
• Editorial: queue a task in Client / Clients API #1836 — Clients API queue-a-task-for-resolve (merged)
• Missing tasks in parallel steps in Service Workers Nightly #1740 — original missing-tasks-in-parallel issue
• Carefully audit all uses of "in parallel" in the spec #1172 — umbrella "Carefully audit all uses of 'in parallel' in the spec"
• Editorial: serialize CacheStorage access on a dedicated parallel queue #1838 — sibling PR (CacheStorage-level parallel queue), same dfn pattern
• Use a per-cache dedicated parallel queue for Cache API methods #1841 — sibling PR (per-cache parallel queue), same dfn pattern

---

Preview | Diff

[asutherland]

Recognizing this patch is a WIP, I think this is currently under-constrained and it probably makes sense to do this like you've done it for the Cache API in #1838 which is basically one-per-storage-key. (Whereas this currently seems to be 1-per-ServiceWorker instance.)

[monica-ch]

Agreed, 1-per-Clients is under-constrained. Will rework to 1-per-storage-key matching #1838.

Quick clarification before I push: #1838 hangs the queue off [=name to cache map=] because that map is itself per-storage-key. Clients don't have an equivalent map — the [=/service worker clients=] list is UA-wide, filtered by storage key. Would you prefer a UA-level <dfn>Clients parallel queue</dfn> map keyed by storage key, or a prose "for each storage key, the UA has an associated Clients parallel queue" form?

[asutherland]

(this got cross-referenced below, but the right answer definitely is not obvious to me, so I asked in the storage spec at whatwg/storage#194)

[monica-ch]

Thanks for kicking off the discussion. Reworked per Anne's suggestion, UA now owns a map keyed by storage key, per-Clients methods enqueue via a relevant service worker clients parallel queue helper (mirrors #1838's relevant name to cache map. PTAL.

[monica-ch]

Closes#1840. Follow-up to #1755.

Give each {{Clients}} object a dedicated parallel queue and route the operations on {{Clients}} through it, so operations on the same {{Clients}} object do not race with each other (e.g. matchAll iterating over service worker clients while claim is mutating their [=active service worker=]).

Changes

• Add a <dfn export for="Clients">Clients parallel queue</dfn> (a [=parallel queue=]) attached to each {{Clients}} object, following the same pattern as the name to cache map parallel queue introduced in Editorial: serialize CacheStorage access on a dedicated parallel queue #1838.
• Route the following method algorithms through it, replacing their existing top-level Run … in parallel block:
  • {{Clients/get(id)}}
  • {{Clients/matchAll(options)}}
  • {{Clients/openWindow(url)}}
  • {{Clients/claim()}}
• Wrap the two remaining bare Resolve |promise| with undefined. steps (in get() and claim()) in Queue a task on |promise|'s [=responsible event loop=] using the [=DOM manipulation task source=], matching the pattern already used in matchAll() and consistent with the queue-a-task refactor from Editorial: Queue a task to resolve/reject promise or when fire an event. #1755.

Total change: +10 / −6 in index.bs, one commit.

Rationale

Yoshi flagged this concern during #1755 review:

I just wondered what happens if one of the service worker clients has been removed or gets execution ready flag during the sub step execution, and suggest to run Clients API algorithm within the dedicated parallel queue to prevent unexpected modifications to clients.

#1836 landed the queue-a-task-for-resolve fixes for these methods. This PR delivers the sibling parallel-queue work that was explicitly deferred in the split plan.

Out of scope

• {{Client/postMessage(message, options)}} — lives on {{Client}}, not {{Clients}}, and does a single-client lookup rather than iterating the full client list. Can be a further follow-up if needed.
• {{WindowClient/focus()}} and {{WindowClient/navigate(url)}} — already skip the in parallel block entirely and use Queue a task on the client's own event loop, so no parallel-queue treatment is meaningful for them.

Related

• Editorial: Queue a task to resolve/reject promise or when fire an event. #1755 — parent (queue-a-task refactor)
• Editorial: queue a task in Client / Clients API #1836 — Clients API queue-a-task-for-resolve (merged)
• Missing tasks in parallel steps in Service Workers Nightly #1740 — original missing-tasks-in-parallel issue
• Carefully audit all uses of "in parallel" in the spec #1172 — umbrella "Carefully audit all uses of 'in parallel' in the spec"
• Editorial: serialize CacheStorage access on a dedicated parallel queue #1838 — sibling PR (CacheStorage-level parallel queue), same dfn pattern
• Use a per-cache dedicated parallel queue for Cache API methods #1841 — sibling PR (per-cache parallel queue), same dfn pattern

---

Preview | Diff

[asutherland]

Recognizing this patch is a WIP, I think this is currently under-constrained and it probably makes sense to do this like you've done it for the Cache API in #1838 which is basically one-per-storage-key. (Whereas this currently seems to be 1-per-ServiceWorker instance.)

[monica-ch]

Agreed, 1-per-Clients is under-constrained. Will rework to 1-per-storage-key matching #1838.

Quick clarification before I push: #1838 hangs the queue off [=name to cache map=] because that map is itself per-storage-key. Clients don't have an equivalent map — the [=/service worker clients=] list is UA-wide, filtered by storage key. Would you prefer a UA-level <dfn>Clients parallel queue</dfn> map keyed by storage key, or a prose "for each storage key, the UA has an associated Clients parallel queue" form?

[asutherland]

(this got cross-referenced below, but the right answer definitely is not obvious to me, so I asked in the storage spec at whatwg/storage#194)

[monica-ch]

Thanks for kicking off the discussion. Reworked per Anne's suggestion, UA now owns a map keyed by storage key, per-Clients methods enqueue via a relevant service worker clients parallel queue helper (mirrors #1838's relevant name to cache map. PTAL.

[monica-ch]

Closes#1840. Follow-up to #1755.

Give each {{Clients}} object a dedicated parallel queue and route the operations on {{Clients}} through it, so operations on the same {{Clients}} object do not race with each other (e.g. matchAll iterating over service worker clients while claim is mutating their [=active service worker=]).

Changes

• Add a <dfn export for="Clients">Clients parallel queue</dfn> (a [=parallel queue=]) attached to each {{Clients}} object, following the same pattern as the name to cache map parallel queue introduced in Editorial: serialize CacheStorage access on a dedicated parallel queue #1838.
• Route the following method algorithms through it, replacing their existing top-level Run … in parallel block:
  • {{Clients/get(id)}}
  • {{Clients/matchAll(options)}}
  • {{Clients/openWindow(url)}}
  • {{Clients/claim()}}
• Wrap the two remaining bare Resolve |promise| with undefined. steps (in get() and claim()) in Queue a task on |promise|'s [=responsible event loop=] using the [=DOM manipulation task source=], matching the pattern already used in matchAll() and consistent with the queue-a-task refactor from Editorial: Queue a task to resolve/reject promise or when fire an event. #1755.

Total change: +10 / −6 in index.bs, one commit.

Rationale

Yoshi flagged this concern during #1755 review:

I just wondered what happens if one of the service worker clients has been removed or gets execution ready flag during the sub step execution, and suggest to run Clients API algorithm within the dedicated parallel queue to prevent unexpected modifications to clients.

#1836 landed the queue-a-task-for-resolve fixes for these methods. This PR delivers the sibling parallel-queue work that was explicitly deferred in the split plan.

Out of scope

• {{Client/postMessage(message, options)}} — lives on {{Client}}, not {{Clients}}, and does a single-client lookup rather than iterating the full client list. Can be a further follow-up if needed.
• {{WindowClient/focus()}} and {{WindowClient/navigate(url)}} — already skip the in parallel block entirely and use Queue a task on the client's own event loop, so no parallel-queue treatment is meaningful for them.

Related

• Editorial: Queue a task to resolve/reject promise or when fire an event. #1755 — parent (queue-a-task refactor)
• Editorial: queue a task in Client / Clients API #1836 — Clients API queue-a-task-for-resolve (merged)
• Missing tasks in parallel steps in Service Workers Nightly #1740 — original missing-tasks-in-parallel issue
• Carefully audit all uses of "in parallel" in the spec #1172 — umbrella "Carefully audit all uses of 'in parallel' in the spec"
• Editorial: serialize CacheStorage access on a dedicated parallel queue #1838 — sibling PR (CacheStorage-level parallel queue), same dfn pattern
• Use a per-cache dedicated parallel queue for Cache API methods #1841 — sibling PR (per-cache parallel queue), same dfn pattern

---

Preview | Diff

[asutherland]

Recognizing this patch is a WIP, I think this is currently under-constrained and it probably makes sense to do this like you've done it for the Cache API in #1838 which is basically one-per-storage-key. (Whereas this currently seems to be 1-per-ServiceWorker instance.)

[monica-ch]

Agreed, 1-per-Clients is under-constrained. Will rework to 1-per-storage-key matching #1838.

Quick clarification before I push: #1838 hangs the queue off [=name to cache map=] because that map is itself per-storage-key. Clients don't have an equivalent map — the [=/service worker clients=] list is UA-wide, filtered by storage key. Would you prefer a UA-level <dfn>Clients parallel queue</dfn> map keyed by storage key, or a prose "for each storage key, the UA has an associated Clients parallel queue" form?

[asutherland]

(this got cross-referenced below, but the right answer definitely is not obvious to me, so I asked in the storage spec at whatwg/storage#194)

[monica-ch]

Thanks for kicking off the discussion. Reworked per Anne's suggestion, UA now owns a map keyed by storage key, per-Clients methods enqueue via a relevant service worker clients parallel queue helper (mirrors #1838's relevant name to cache map. PTAL.

[monica-ch]

Closes#1840. Follow-up to #1755.

Give each {{Clients}} object a dedicated parallel queue and route the operations on {{Clients}} through it, so operations on the same {{Clients}} object do not race with each other (e.g. matchAll iterating over service worker clients while claim is mutating their [=active service worker=]).

Changes

• Add a <dfn export for="Clients">Clients parallel queue</dfn> (a [=parallel queue=]) attached to each {{Clients}} object, following the same pattern as the name to cache map parallel queue introduced in Editorial: serialize CacheStorage access on a dedicated parallel queue #1838.
• Route the following method algorithms through it, replacing their existing top-level Run … in parallel block:
  • {{Clients/get(id)}}
  • {{Clients/matchAll(options)}}
  • {{Clients/openWindow(url)}}
  • {{Clients/claim()}}
• Wrap the two remaining bare Resolve |promise| with undefined. steps (in get() and claim()) in Queue a task on |promise|'s [=responsible event loop=] using the [=DOM manipulation task source=], matching the pattern already used in matchAll() and consistent with the queue-a-task refactor from Editorial: Queue a task to resolve/reject promise or when fire an event. #1755.

Total change: +10 / −6 in index.bs, one commit.

Rationale

Yoshi flagged this concern during #1755 review:

I just wondered what happens if one of the service worker clients has been removed or gets execution ready flag during the sub step execution, and suggest to run Clients API algorithm within the dedicated parallel queue to prevent unexpected modifications to clients.

#1836 landed the queue-a-task-for-resolve fixes for these methods. This PR delivers the sibling parallel-queue work that was explicitly deferred in the split plan.

Out of scope

• {{Client/postMessage(message, options)}} — lives on {{Client}}, not {{Clients}}, and does a single-client lookup rather than iterating the full client list. Can be a further follow-up if needed.
• {{WindowClient/focus()}} and {{WindowClient/navigate(url)}} — already skip the in parallel block entirely and use Queue a task on the client's own event loop, so no parallel-queue treatment is meaningful for them.

Related

• Editorial: Queue a task to resolve/reject promise or when fire an event. #1755 — parent (queue-a-task refactor)
• Editorial: queue a task in Client / Clients API #1836 — Clients API queue-a-task-for-resolve (merged)
• Missing tasks in parallel steps in Service Workers Nightly #1740 — original missing-tasks-in-parallel issue
• Carefully audit all uses of "in parallel" in the spec #1172 — umbrella "Carefully audit all uses of 'in parallel' in the spec"
• Editorial: serialize CacheStorage access on a dedicated parallel queue #1838 — sibling PR (CacheStorage-level parallel queue), same dfn pattern
• Use a per-cache dedicated parallel queue for Cache API methods #1841 — sibling PR (per-cache parallel queue), same dfn pattern

---

Preview | Diff

[asutherland]

Recognizing this patch is a WIP, I think this is currently under-constrained and it probably makes sense to do this like you've done it for the Cache API in #1838 which is basically one-per-storage-key. (Whereas this currently seems to be 1-per-ServiceWorker instance.)

[monica-ch]

Agreed, 1-per-Clients is under-constrained. Will rework to 1-per-storage-key matching #1838.

Quick clarification before I push: #1838 hangs the queue off [=name to cache map=] because that map is itself per-storage-key. Clients don't have an equivalent map — the [=/service worker clients=] list is UA-wide, filtered by storage key. Would you prefer a UA-level <dfn>Clients parallel queue</dfn> map keyed by storage key, or a prose "for each storage key, the UA has an associated Clients parallel queue" form?

[asutherland]

(this got cross-referenced below, but the right answer definitely is not obvious to me, so I asked in the storage spec at whatwg/storage#194)

[monica-ch]

Thanks for kicking off the discussion. Reworked per Anne's suggestion, UA now owns a map keyed by storage key, per-Clients methods enqueue via a relevant service worker clients parallel queue helper (mirrors #1838's relevant name to cache map. PTAL.

[monica-ch]

Closes#1840. Follow-up to #1755.

Give each {{Clients}} object a dedicated parallel queue and route the operations on {{Clients}} through it, so operations on the same {{Clients}} object do not race with each other (e.g. matchAll iterating over service worker clients while claim is mutating their [=active service worker=]).

Changes

• Add a <dfn export for="Clients">Clients parallel queue</dfn> (a [=parallel queue=]) attached to each {{Clients}} object, following the same pattern as the name to cache map parallel queue introduced in Editorial: serialize CacheStorage access on a dedicated parallel queue #1838.
• Route the following method algorithms through it, replacing their existing top-level Run … in parallel block:
  • {{Clients/get(id)}}
  • {{Clients/matchAll(options)}}
  • {{Clients/openWindow(url)}}
  • {{Clients/claim()}}
• Wrap the two remaining bare Resolve |promise| with undefined. steps (in get() and claim()) in Queue a task on |promise|'s [=responsible event loop=] using the [=DOM manipulation task source=], matching the pattern already used in matchAll() and consistent with the queue-a-task refactor from Editorial: Queue a task to resolve/reject promise or when fire an event. #1755.

Total change: +10 / −6 in index.bs, one commit.

Rationale

Yoshi flagged this concern during #1755 review:

I just wondered what happens if one of the service worker clients has been removed or gets execution ready flag during the sub step execution, and suggest to run Clients API algorithm within the dedicated parallel queue to prevent unexpected modifications to clients.

#1836 landed the queue-a-task-for-resolve fixes for these methods. This PR delivers the sibling parallel-queue work that was explicitly deferred in the split plan.

Out of scope

• {{Client/postMessage(message, options)}} — lives on {{Client}}, not {{Clients}}, and does a single-client lookup rather than iterating the full client list. Can be a further follow-up if needed.
• {{WindowClient/focus()}} and {{WindowClient/navigate(url)}} — already skip the in parallel block entirely and use Queue a task on the client's own event loop, so no parallel-queue treatment is meaningful for them.

Related

• Editorial: Queue a task to resolve/reject promise or when fire an event. #1755 — parent (queue-a-task refactor)
• Editorial: queue a task in Client / Clients API #1836 — Clients API queue-a-task-for-resolve (merged)
• Missing tasks in parallel steps in Service Workers Nightly #1740 — original missing-tasks-in-parallel issue
• Carefully audit all uses of "in parallel" in the spec #1172 — umbrella "Carefully audit all uses of 'in parallel' in the spec"
• Editorial: serialize CacheStorage access on a dedicated parallel queue #1838 — sibling PR (CacheStorage-level parallel queue), same dfn pattern
• Use a per-cache dedicated parallel queue for Cache API methods #1841 — sibling PR (per-cache parallel queue), same dfn pattern

---

Preview | Diff

[asutherland]

Recognizing this patch is a WIP, I think this is currently under-constrained and it probably makes sense to do this like you've done it for the Cache API in #1838 which is basically one-per-storage-key. (Whereas this currently seems to be 1-per-ServiceWorker instance.)

[monica-ch]

Agreed, 1-per-Clients is under-constrained. Will rework to 1-per-storage-key matching #1838.

Quick clarification before I push: #1838 hangs the queue off [=name to cache map=] because that map is itself per-storage-key. Clients don't have an equivalent map — the [=/service worker clients=] list is UA-wide, filtered by storage key. Would you prefer a UA-level <dfn>Clients parallel queue</dfn> map keyed by storage key, or a prose "for each storage key, the UA has an associated Clients parallel queue" form?

[asutherland]

(this got cross-referenced below, but the right answer definitely is not obvious to me, so I asked in the storage spec at whatwg/storage#194)

[monica-ch]

Thanks for kicking off the discussion. Reworked per Anne's suggestion, UA now owns a map keyed by storage key, per-Clients methods enqueue via a relevant service worker clients parallel queue helper (mirrors #1838's relevant name to cache map. PTAL.

[monica-ch]

Closes#1840. Follow-up to #1755.

Give each {{Clients}} object a dedicated parallel queue and route the operations on {{Clients}} through it, so operations on the same {{Clients}} object do not race with each other (e.g. matchAll iterating over service worker clients while claim is mutating their [=active service worker=]).

Changes

• Add a <dfn export for="Clients">Clients parallel queue</dfn> (a [=parallel queue=]) attached to each {{Clients}} object, following the same pattern as the name to cache map parallel queue introduced in Editorial: serialize CacheStorage access on a dedicated parallel queue #1838.
• Route the following method algorithms through it, replacing their existing top-level Run … in parallel block:
  • {{Clients/get(id)}}
  • {{Clients/matchAll(options)}}
  • {{Clients/openWindow(url)}}
  • {{Clients/claim()}}
• Wrap the two remaining bare Resolve |promise| with undefined. steps (in get() and claim()) in Queue a task on |promise|'s [=responsible event loop=] using the [=DOM manipulation task source=], matching the pattern already used in matchAll() and consistent with the queue-a-task refactor from Editorial: Queue a task to resolve/reject promise or when fire an event. #1755.

Total change: +10 / −6 in index.bs, one commit.

Rationale

Yoshi flagged this concern during #1755 review:

I just wondered what happens if one of the service worker clients has been removed or gets execution ready flag during the sub step execution, and suggest to run Clients API algorithm within the dedicated parallel queue to prevent unexpected modifications to clients.

#1836 landed the queue-a-task-for-resolve fixes for these methods. This PR delivers the sibling parallel-queue work that was explicitly deferred in the split plan.

Out of scope

• {{Client/postMessage(message, options)}} — lives on {{Client}}, not {{Clients}}, and does a single-client lookup rather than iterating the full client list. Can be a further follow-up if needed.
• {{WindowClient/focus()}} and {{WindowClient/navigate(url)}} — already skip the in parallel block entirely and use Queue a task on the client's own event loop, so no parallel-queue treatment is meaningful for them.

Related

• Editorial: Queue a task to resolve/reject promise or when fire an event. #1755 — parent (queue-a-task refactor)
• Editorial: queue a task in Client / Clients API #1836 — Clients API queue-a-task-for-resolve (merged)
• Missing tasks in parallel steps in Service Workers Nightly #1740 — original missing-tasks-in-parallel issue
• Carefully audit all uses of "in parallel" in the spec #1172 — umbrella "Carefully audit all uses of 'in parallel' in the spec"
• Editorial: serialize CacheStorage access on a dedicated parallel queue #1838 — sibling PR (CacheStorage-level parallel queue), same dfn pattern
• Use a per-cache dedicated parallel queue for Cache API methods #1841 — sibling PR (per-cache parallel queue), same dfn pattern

---

Preview | Diff

[asutherland]

Recognizing this patch is a WIP, I think this is currently under-constrained and it probably makes sense to do this like you've done it for the Cache API in #1838 which is basically one-per-storage-key. (Whereas this currently seems to be 1-per-ServiceWorker instance.)

[monica-ch]

Agreed, 1-per-Clients is under-constrained. Will rework to 1-per-storage-key matching #1838.

Quick clarification before I push: #1838 hangs the queue off [=name to cache map=] because that map is itself per-storage-key. Clients don't have an equivalent map — the [=/service worker clients=] list is UA-wide, filtered by storage key. Would you prefer a UA-level <dfn>Clients parallel queue</dfn> map keyed by storage key, or a prose "for each storage key, the UA has an associated Clients parallel queue" form?

[asutherland]

(this got cross-referenced below, but the right answer definitely is not obvious to me, so I asked in the storage spec at whatwg/storage#194)

[monica-ch]

Thanks for kicking off the discussion. Reworked per Anne's suggestion, UA now owns a map keyed by storage key, per-Clients methods enqueue via a relevant service worker clients parallel queue helper (mirrors #1838's relevant name to cache map. PTAL.

[monica-ch]

Closes#1840. Follow-up to #1755.

Give each {{Clients}} object a dedicated parallel queue and route the operations on {{Clients}} through it, so operations on the same {{Clients}} object do not race with each other (e.g. matchAll iterating over service worker clients while claim is mutating their [=active service worker=]).

Changes

• Add a <dfn export for="Clients">Clients parallel queue</dfn> (a [=parallel queue=]) attached to each {{Clients}} object, following the same pattern as the name to cache map parallel queue introduced in Editorial: serialize CacheStorage access on a dedicated parallel queue #1838.
• Route the following method algorithms through it, replacing their existing top-level Run … in parallel block:
  • {{Clients/get(id)}}
  • {{Clients/matchAll(options)}}
  • {{Clients/openWindow(url)}}
  • {{Clients/claim()}}
• Wrap the two remaining bare Resolve |promise| with undefined. steps (in get() and claim()) in Queue a task on |promise|'s [=responsible event loop=] using the [=DOM manipulation task source=], matching the pattern already used in matchAll() and consistent with the queue-a-task refactor from Editorial: Queue a task to resolve/reject promise or when fire an event. #1755.

Total change: +10 / −6 in index.bs, one commit.

Rationale

Yoshi flagged this concern during #1755 review:

I just wondered what happens if one of the service worker clients has been removed or gets execution ready flag during the sub step execution, and suggest to run Clients API algorithm within the dedicated parallel queue to prevent unexpected modifications to clients.

#1836 landed the queue-a-task-for-resolve fixes for these methods. This PR delivers the sibling parallel-queue work that was explicitly deferred in the split plan.

Out of scope

• {{Client/postMessage(message, options)}} — lives on {{Client}}, not {{Clients}}, and does a single-client lookup rather than iterating the full client list. Can be a further follow-up if needed.
• {{WindowClient/focus()}} and {{WindowClient/navigate(url)}} — already skip the in parallel block entirely and use Queue a task on the client's own event loop, so no parallel-queue treatment is meaningful for them.

Related

• Editorial: Queue a task to resolve/reject promise or when fire an event. #1755 — parent (queue-a-task refactor)
• Editorial: queue a task in Client / Clients API #1836 — Clients API queue-a-task-for-resolve (merged)
• Missing tasks in parallel steps in Service Workers Nightly #1740 — original missing-tasks-in-parallel issue
• Carefully audit all uses of "in parallel" in the spec #1172 — umbrella "Carefully audit all uses of 'in parallel' in the spec"
• Editorial: serialize CacheStorage access on a dedicated parallel queue #1838 — sibling PR (CacheStorage-level parallel queue), same dfn pattern
• Use a per-cache dedicated parallel queue for Cache API methods #1841 — sibling PR (per-cache parallel queue), same dfn pattern

---

Preview | Diff

[asutherland]

Recognizing this patch is a WIP, I think this is currently under-constrained and it probably makes sense to do this like you've done it for the Cache API in #1838 which is basically one-per-storage-key. (Whereas this currently seems to be 1-per-ServiceWorker instance.)

[monica-ch]

Agreed, 1-per-Clients is under-constrained. Will rework to 1-per-storage-key matching #1838.

Quick clarification before I push: #1838 hangs the queue off [=name to cache map=] because that map is itself per-storage-key. Clients don't have an equivalent map — the [=/service worker clients=] list is UA-wide, filtered by storage key. Would you prefer a UA-level <dfn>Clients parallel queue</dfn> map keyed by storage key, or a prose "for each storage key, the UA has an associated Clients parallel queue" form?

[asutherland]

(this got cross-referenced below, but the right answer definitely is not obvious to me, so I asked in the storage spec at whatwg/storage#194)

[monica-ch]

Thanks for kicking off the discussion. Reworked per Anne's suggestion, UA now owns a map keyed by storage key, per-Clients methods enqueue via a relevant service worker clients parallel queue helper (mirrors #1838's relevant name to cache map. PTAL.

[monica-ch]

Closes#1840. Follow-up to #1755.

Give each {{Clients}} object a dedicated parallel queue and route the operations on {{Clients}} through it, so operations on the same {{Clients}} object do not race with each other (e.g. matchAll iterating over service worker clients while claim is mutating their [=active service worker=]).

Changes

• Add a <dfn export for="Clients">Clients parallel queue</dfn> (a [=parallel queue=]) attached to each {{Clients}} object, following the same pattern as the name to cache map parallel queue introduced in Editorial: serialize CacheStorage access on a dedicated parallel queue #1838.
• Route the following method algorithms through it, replacing their existing top-level Run … in parallel block:
  • {{Clients/get(id)}}
  • {{Clients/matchAll(options)}}
  • {{Clients/openWindow(url)}}
  • {{Clients/claim()}}
• Wrap the two remaining bare Resolve |promise| with undefined. steps (in get() and claim()) in Queue a task on |promise|'s [=responsible event loop=] using the [=DOM manipulation task source=], matching the pattern already used in matchAll() and consistent with the queue-a-task refactor from Editorial: Queue a task to resolve/reject promise or when fire an event. #1755.

Total change: +10 / −6 in index.bs, one commit.

Rationale

Yoshi flagged this concern during #1755 review:

I just wondered what happens if one of the service worker clients has been removed or gets execution ready flag during the sub step execution, and suggest to run Clients API algorithm within the dedicated parallel queue to prevent unexpected modifications to clients.

#1836 landed the queue-a-task-for-resolve fixes for these methods. This PR delivers the sibling parallel-queue work that was explicitly deferred in the split plan.

Out of scope

• {{Client/postMessage(message, options)}} — lives on {{Client}}, not {{Clients}}, and does a single-client lookup rather than iterating the full client list. Can be a further follow-up if needed.
• {{WindowClient/focus()}} and {{WindowClient/navigate(url)}} — already skip the in parallel block entirely and use Queue a task on the client's own event loop, so no parallel-queue treatment is meaningful for them.

Related

• Editorial: Queue a task to resolve/reject promise or when fire an event. #1755 — parent (queue-a-task refactor)
• Editorial: queue a task in Client / Clients API #1836 — Clients API queue-a-task-for-resolve (merged)
• Missing tasks in parallel steps in Service Workers Nightly #1740 — original missing-tasks-in-parallel issue
• Carefully audit all uses of "in parallel" in the spec #1172 — umbrella "Carefully audit all uses of 'in parallel' in the spec"
• Editorial: serialize CacheStorage access on a dedicated parallel queue #1838 — sibling PR (CacheStorage-level parallel queue), same dfn pattern
• Use a per-cache dedicated parallel queue for Cache API methods #1841 — sibling PR (per-cache parallel queue), same dfn pattern

---

Preview | Diff

[asutherland]

Recognizing this patch is a WIP, I think this is currently under-constrained and it probably makes sense to do this like you've done it for the Cache API in #1838 which is basically one-per-storage-key. (Whereas this currently seems to be 1-per-ServiceWorker instance.)

[monica-ch]

Agreed, 1-per-Clients is under-constrained. Will rework to 1-per-storage-key matching #1838.

Quick clarification before I push: #1838 hangs the queue off [=name to cache map=] because that map is itself per-storage-key. Clients don't have an equivalent map — the [=/service worker clients=] list is UA-wide, filtered by storage key. Would you prefer a UA-level <dfn>Clients parallel queue</dfn> map keyed by storage key, or a prose "for each storage key, the UA has an associated Clients parallel queue" form?

[asutherland]

(this got cross-referenced below, but the right answer definitely is not obvious to me, so I asked in the storage spec at whatwg/storage#194)

[monica-ch]

Thanks for kicking off the discussion. Reworked per Anne's suggestion, UA now owns a map keyed by storage key, per-Clients methods enqueue via a relevant service worker clients parallel queue helper (mirrors #1838's relevant name to cache map. PTAL.