Skip to content

Commit

Permalink
editorial: Tidy up uses of "in parallel". (w3c#299)
Browse files Browse the repository at this point in the history 
Steps that run "in parallel" need to take several extra restrictions into
account: we do not have access to documents or global objects, cannot create
IDL interfaces or manipulate promises, for example, all of which we had been
doing, along with needlessly running nested "in parallel" steps.

This change attempts to rectify the situation by only running things in
parallel when absolutely necessary (i.e. when reaching out to the OS), and
queuing global tasks (emphasis on "global" given whatwg/html#4980 and
whatwg/html#5653) from there when we need to manipulate promises or create
objects.

"Obtain permission" algorithm:
* Stop running in parallel. Callers should be responsible for choosing
  whether it should be run in parallel or not.

`WakeLock.request()`:
* Separate returning a promise and running steps in parallel. This style is
  more usual.
* Refer to the "relevant settings object" rather than the "current settings
  object", as we are inside a method of an IDL interface and can rely on it
  being defined. I do not think this is a user-visible change, and it looks
  cleaner.
* Queue a global task to reject `|promise|` when the permission request run
  in parallel is denied.
* Manipulate the `[[ActiveLocks]]` internal slot, check page visibility,
  invoke "acquire a wake lock", create a new WakeLockSentinel and resolve
  the returned promise in a queued global task.

`WakeLockSentinel.release()`:
* Do not run the "release a wake lock" algorithm in parallel (see the
  changes to the algorithm itself below).
* Just return a resolved promise once the rest of the steps run. The
  returned promise does not have much use, but has been kept to avoid
  breaking API compatibility.
* One user-visible consequence is that the "release" event is fired
  synchronously and before the function returns.

"Acquire wake lock" algorithm:
* Stop running everything in parallel.
* Move all `[[ActiveLocks]]` manipulation to `WakeLock.request()` itself,
  and make the algorithm just ask the platform for a wake lock.

"Release wake lock" algorithm:
* Stop running everything in parallel. The only steps that still need to run
  in parallel are the ones asking the platform to release the wake lock.
* Consequently, stop queueing a task to change `[[Released]]` and fire the
  "release" event, and do it directly in the algorithm instead.

Big thanks to Anne van Kesteren, Domenic Denicola and Marcos Cáceres for
their input.

Fixes w3c#293.
  • Loading branch information
Raphael Kubo da Costa authored Mar 22, 2021
1 parent 9fb419f commit 1e380dc
Showing 1 changed file with 69 additions and 70 deletions.
139 changes: 69 additions & 70 deletions index.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ <h2>
</ol>
<p>
To <dfn>obtain permission</dfn> for <a>wake lock type</a> |name|, run
these steps <a>in parallel</a>. This async algorithm returns either
the following steps. This algorithm returns either
{{PermissionState/"granted"}} or {{PermissionState/"denied"}}.
</p>
<aside class="note">
Expand Down Expand Up @@ -338,7 +338,8 @@ <h3>
</p>
<ol class="algorithm">
<li>Let |document:Document| be the [=environment settings object /
responsible document=] of the <a>current settings object</a>.
responsible document=] of <a>this</a>'s <a>relevant settings
object</a>.
</li>
<li data-tests="wakelock-disabled-by-feature-policy.https.sub.html">
If |document| is not [=allowed to use=] the [=policy-controlled
Expand All @@ -363,48 +364,73 @@ <h3>
return [=a promise rejected with=] {{"NotAllowedError"}}
{{DOMException}}.
</li>
<li>Let |record| be the <a>platform wake lock</a>'s <a>state
record</a> associated with |document| and |type|.
</li>
<li>Let |promise:Promise| be [=a new promise=].
</li>
<li>Return |promise| and run the following steps <a>in parallel</a>:
<li>Run the following steps <a>in parallel</a>:
<ol>
<li>
<a>Abort when</a> the steps to <a>determine the visibility
state</a> return `hidden`.
</li>
<li>Let |state:PermissionState| be the result of awaiting
<a>obtain permission</a> steps with "`screen-wake-lock`".
<li>Let |state:PermissionState| be the result of invoking
<a>obtain permission</a> with "`screen-wake-lock`".
</li>
<li data-tests="wakelock-request-denied.https.html">If |state| is
{{PermissionState/"denied"}}, then:
<ol>
<li>Reject |promise| with a {{"NotAllowedError"}}
{{DOMException}}.
<li>
<a>Queue a global task</a> on the <a>screen wake lock task
source</a> given |document|'s <a>relevant global object</a>
to reject |promise| with a {{"NotAllowedError"}}
{{DOMException}}.
</li>
<li>Abort these steps.
</li>
</ol>
</li>
<li>Let |lock:WakeLockSentinel| be a new {{WakeLockSentinel}}
object with its {{WakeLockSentinel/type}} attribute set to
|type|.
</li>
<li>Invoke <a>acquire a wake lock</a> with |lock| and
{{WakeLockType/"screen"}}.
<aside class="note">
The <a>acquire a wake lock</a> algorithm may ultimately be
unable to acquire a lock from the operating system, but this
is indistinguishable from a successful lock acquisition to
avoid user fingerprinting (failure to acquire a lock can
indicate low battery levels, for example).
</aside>
</li>
<li>Resolve |promise| with |lock|.
<li>
<a>Queue a global task</a> on the <a>screen wake lock task
source</a> given |document|'s <a>relevant global object</a> to
run these steps:
<ol>
<li>If the steps to <a>determine the visibility state</a>
return `hidden`, then:
<ol>
<li>Reject |promise| with a {{"NotAllowedError"}}
{{DOMException}}.
</li>
<li>Abort these steps.
</li>
</ol>
</li>
<li>If |record|.{{[[ActiveLocks]]}} [=list/is empty=], then
invoke the following steps <a>in parallel</a>:
<ol>
<li>Invoke <a>acquire a wake lock</a> with
{{WakeLockType/"screen"}}.
<aside class="note">
The <a>acquire a wake lock</a> algorithm may
ultimately be unable to acquire a lock from the
operating system, but this is indistinguishable from
a successful lock acquisition to avoid user
fingerprinting (failure to acquire a lock can
indicate low battery levels, for example).
</aside>
</li>
</ol>
</li>
<li>Let |lock:WakeLockSentinel| be a new {{WakeLockSentinel}}
object with its {{WakeLockSentinel/type}} attribute set to
|type|.
</li>
<li>Add |lock| to |record|.{{[[ActiveLocks]]}}.
</li>
<li>Resolve |promise| with |lock|.
</li>
</ol>
</li>
</ol>
</li>
<li>
<a>If aborted</a>, reject |promise| with a {{"NotAllowedError"}}
{{DOMException}}.
<li>Return |promise|.
</li>
</ol>
</section>
Expand Down Expand Up @@ -508,22 +534,12 @@ <h3>
following steps:
</p>
<ol class="algorithm">
<li>If <a>this</a>'s {{[[Released]]}} internal slot is `true`, return
<a>a promise resolved with</a> `undefined`.
<li>If <a>this</a>'s {{[[Released]]}} is `false`, then run <a>release
a wake lock</a> with |lock:WakeLockSentinel| set to <a>this</a> and
|type:WakeLockType| set to the value of <a>this</a>'s
{{WakeLockSentinel/type}} attribute.
</li>
<li>Otherwise, let |promise:Promise| be <a>a new promise</a>.
</li>
<li>Run the following steps <a>in parallel</a>:
<ol>
<li>Run <a>release a wake lock</a> with |lock:WakeLockSentinel|
set to <a>this</a> and |type:WakeLockType| set to the value of
<a>this</a>'s {{WakeLockSentinel/type}} attribute.
</li>
<li>Resolve |promise|.
</li>
</ol>
</li>
<li>Return |promise|.
<li>Return <a>a promise resolved with</a> `undefined`.
</li>
</ol>
</section>
Expand Down Expand Up @@ -705,27 +721,16 @@ <h3>
Acquire wake lock algorithm
</h3>
<p>
To <dfn>acquire a wake lock</dfn> for a given |lock:WakeLockSentinel|
and |type:WakeLockType|, run these steps <a>in parallel</a>:
To <dfn>acquire a wake lock</dfn> for a given |type:WakeLockType|,
run these steps:
</p>
<ol class="algorithm">
<li>If the wake lock for type |type| is not <a>applicable</a>, abort
these steps.
</li>
<li>If the <a>platform wake lock</a> has an active lock for |type|,
abort these steps.
</li>
<li>Ask the underlying operating system to <a>acquire the wake
lock</a> of type |type|.
</li>
<li>Let |document:Document| be the [=environment settings object /
responsible document=] of the <a>current settings object</a>.
</li>
<li>Let |record| be the <a>platform wake lock</a>'s <a>state
record</a> associated with |document| and |type|.
</li>
<li>Add |lock| to |record|.{{[[ActiveLocks]]}}.
</li>
</ol>
</section>
<section>
Expand All @@ -734,7 +739,7 @@ <h3>
</h3>
<p>
To <dfn>release a wake lock</dfn> for a given |lock:WakeLockSentinel|
and |type:WakeLockType|, run these steps <a>in parallel</a>:
and |type:WakeLockType|, run these steps:
</p>
<ol class="algorithm">
<li>Let |document:Document| be the [=environment settings object /
Expand All @@ -748,8 +753,7 @@ <h3>
</li>
<li>Remove |lock| from |record|.{{[[ActiveLocks]]}}.
</li>
<li>If the internal slot {{[[ActiveLocks]]}} of all the <a>platform
wake lock</a>'s <a>state record</a>s are all empty, then run the
<li>If |record|.{{[[ActiveLocks]]}} [=list/is empty=], then run the
following steps <a>in parallel</a>:
<ol>
<li>Ask the underlying operating system to release the wake lock
Expand All @@ -770,15 +774,10 @@ <h3>
</li>
</ol>
</li>
<li>Set |lock|'s {{[[Released]]}} to `true`.
</li>
<li>
<a>Queue a task</a> to run the following steps:
<ol>
<li>Set |lock|'s {{[[Released]]}} internal slot to `true`.
</li>
<li>
<a>Fire an event</a> named "`release`" at |lock|.
</li>
</ol>
<a>Fire an event</a> named "`release`" at |lock|.
</li>
</ol>
</section>
Expand Down Expand Up @@ -866,8 +865,8 @@ <h2>
<p>
We would like to offer our sincere thanks to Mounir Lamouri, Sergey
Konstantinov, Matvey Larionov, Dominique Hazael-Massieux, Domenic
Denicola, Thomas Steiner, Raphael Kubo da Costa for their contributions
to this work.
Denicola, Thomas Steiner, Anne van Kesteren for their contributions to
this work.
</p>
</section>
<section class="appendix informative" id="changes">
Expand Down

0 comments on commit 1e380dc

Please sign in to comment.