observe() attempts to run asynchronous steps synchronously
rakuco opened this issue · 11 comments
The observe() steps include:
- Let permissionState be the result of getting the current permission state with "compute-pressure".
- If permissionState is not granted, throw NotAllowedError.
These permission-related steps are often run in a "in parallel" block since they tend to be asynchronous.
If you introduce "in parallel" steps, you'll probably need to signal exceptions like "NotAllowedError" differently as well (and maybe even make the operation return a promise to make @arskama happy :-)
I think one way is if the PermissionState is not granted, we don't throw any error and don't append the observer to the registered observer list. But user is unaware about this. The Other way is making observe() return a promise. I perfer the second way.
The getting the current permission state with "compute-pressure" is sync and might return "prompt" AFAIU.
The getting the current permission state with "compute-pressure" is sync
That algorithm basically only invokes https://www.w3.org/TR/permissions/#dfn-permission-state, which can be asynchronous (it for sure is in Chromium). That's why e.g. https://www.w3.org/TR/permissions/#query-method returns a promise, https://notifications.spec.whatwg.org/#get-the-notifications-permission-state is invoked "in parallel" and https://wicg.github.io/idle-detection/#api-idledetector-start, which follows a similar model, checks it in parallel.
OK, those permission specs could be a bit more clear that those algos were async
It also seems like https://www.w3.org/TR/permissions/#dfn-permission-state already does the policy check, so maybe that can be removed then. Can you confirm? Or what do you think @rakuco
That's something I started wondering myself yesterday.
From an implementation perspective, the Chromium implementation does not do that (I filed https://bugs.chromium.org/p/chromium/issues/detail?id=1380817 yesterday to track it), but from a spec perspective I think it's OK? Honestly I was surprised to find that out because all specs still seem to be checking for permissions policy stuff explicitly.
Hi, @rakuco . Please take a look at here: https://source.chromium.org/chromium/chromium/src/+/main:components/permissions/permission_context_base.cc;l=244;drc=d7ea38be51e84e30faa9864637b21f7006d3a65d Does this means permission has already done the policy check?
Will we change the spec to make observe() method return a promise? @kenchris
Hi, @kenchris @rakuco. Any decision about this? If we decide to make observe() method return a promise, then I can change it in my Permissions patch.
I think it is OK to turn it into a promise
I think it is OK to turn it into a promise
Ok, I will change it in implementation. Can you help modify the spec? @kenchris