w3ctag / design-reviews

W3C specs and API reviews
Creative Commons Zero v1.0 Universal
327 stars 55 forks source link

Review of WakeLock API and suitability for overall platform requested by 31 August 2016 #126

Closed fhirsch closed 7 years ago

fhirsch commented 8 years ago

RfC: Wide review of Wake Lock API; deadline August 31st

The Device & Sensors Working Group is soliciting the review of the Wake Lock API on our way to Candidate Recommendation status: https://www.w3.org/TR/wake-lock/

From the TAG we hope to get a review of the overall API and its insertion in the rest of the platform (as well as potentially comments on aspects we have asked other groups about in a call for review)

From APA, PING and WebAppSec, we hope to get a review from an accessibility, privacy and security perspective of the specification.

We particularly call upon the attention of the WebAppSec WG on the proposed approach to manage permissions to use the Wake Lock API, whereby an embedded cross-origin browsing context is never allowed, as described in the first note in section 5.

For both WebAppSec and PING, we note that the group used the self-review questionnaire in the development of this specification:

https://lists.w3.org/Archives/Public/public-device-apis/2016Mar/att-0038/00-part

From WebPlatform and TAG, we hope to get a review of the overall API and its insertion in the rest of the platform.

Since the API extends the Screen interface defined by the CSS WG in the CSSOM View module, the CSS WG might wish to confirm this extension is in-line with the design of the interface.

Likewise, since the API relies on the Page Visibility state defined by the WebPerf WG, that group might wish to comment on the proper usage of that signal.

Reviews from other groups are also naturally welcome.

We would appreciate to receive your feedback before the end of August; the preferred method for feedback is to file issues in our github repository: https://github.com/w3c/wake-lock/issues ;

alternatively, send a mail to our public mailing list public-device-apis@w3.org with a subject prefixed with [wake-lock].

regards, Frederick

Frederick Hirsch, Device & Sensors Working Group Chair

dbaron commented 8 years ago

Also see the use cases document.

travisleithead commented 8 years ago

My review comments:

travisleithead commented 8 years ago

Additional notes from our review on 8/24: https://pad.w3ctag.org/p/24-08-2016-minutes.md

torgo commented 8 years ago

Discussed on 31-aug-2016 call in context of pointer lock API... is there a lack of consistency?

travisleithead commented 8 years ago

As discussed today on the call, the API is well-specified in its own right (see https://github.com/w3ctag/spec-reviews/issues/126#issuecomment-242101530). One of the concerns voiced today was that when placed alongside other APIs of relatively similar character (pointerLock, fullscreen API), (1) the shape of the API is not consistent (e.g., there's a pattern of request*() methods being used and this property doesn't quite fit that pattern), and (2) other platform APIs are able to convey the actual state of the requested thing (through related events, etc.--in wakeLock, the actual state is not disclosed).

travisleithead commented 8 years ago

@fhirsch feedback posted as a new issue in the wake-lock GitHub as requested.

slightlyoff commented 7 years ago

@triblondon raised an issue that has recently resurfaced in discussions with folks building progressive web apps on our team: it's unclear how this API can support leaving an app running in the foreground without the screen staying on an unlocked (in a mobile phone, e.g.).

The recent RioRun app suffered from this issue: https://www.theguardian.com/sport/2016/aug/06/rio-running-app-marathon-course-riorun

It appears this was accounted for in the use-case doc: https://w3c-webmob.github.io/wake-lock-use-cases/#keeping-the-system-awake

That said, it isn't clear (in the current API) how to support system-wakefulness while also allowing screen dimming. Enabling this seems crucial and a central design challenge for the current API. I don't see how a simple boolean can support both types of wake locking.

dontcallmedom commented 7 years ago

The API has been reworked in depth by its editor @andrey-logvinov based on the feedback from the TAG - we would appreciate if the TAG could take another look at it to see if this is getting in a more promising direction: https://w3c.github.io/wake-lock/

hadleybeeman commented 7 years ago

We've just noticed in the Security and Privacy Considerations section that if you request a wakeLock and it were denied because the battery was low, this might accidentally expose the device's battery level to the site. You might want to mention that.

triblondon commented 7 years ago

Thanks, this looks like it is much improved. Aside from Hadley's point above, we also came up with the idea that for some use cases currently addressed by WakeLock, there may be a more elegant solution involving a subscription to events in a ServiceWorker. For example, if my jogging application requires the continuous tracking of a person's GPS position, with (say) a 20 second interval sample rate, and then wants to write that data to storage for later processing, then a Serviceworker could be woken up with an event each time the GPS position changed outside of that time-window debounce tolerance. This might be more power efficient, and we wonder if the spec wants to anticipate that future solution at all.

Finally, I wonder if a system wakelock is cancelled by locking the device, and we feel this should be in the spec. If a screen wakelock exists, then manually placing the device in standby (using a hardware lock button) would presumably cancel the wakelock, whereas manually hitting the lock button on a device with an active system wakelock would (we assume) not affect the wakelock and the page would continue to run as if it were foregrounded on an active screen.

andrey-logvinov commented 7 years ago

@triblondon in the jogging application case, I think you shouldn't need to explicitly request a wake lock at all, as you want the device to be woken up on external events vs not going to sleep in the first place. I think this case is best addressed by the API which provides your GPS position, such as Generic Sensors API. In the end, the CPU already needs to wake up to process each sample and decide whether the position has changed beyond your tolerance threshold, so no wake lock should be needed there.

triblondon commented 7 years ago

@andrey-logvinov Would the generic sensors API provide continuous data even if the application is backgrounded? Sorry I'm not so familiar with how generic sensor events work. If that use case is already enabled by generic sensors, brilliant. It seems like your use cases could do with updating to be clearer about the specific scenarios you are targeting with system wake locks. Currently it is very heavy on screen lock scenarios.

Let me bring mine and Hadley's feedback together for convenience:

tobie commented 7 years ago

Would the generic sensors API provide continuous data even if the application is backgrounded? Sorry I'm not so familiar with how generic sensor events work.

We have some future plans for that (involving both background and service worker-based scenario), but nothing concrete to share for now.

FWIW, using a wake-lock for geolocation or pedometer use cases (e.g. running app) would be such a battery-drainer it's a no-go from the start.

triblondon commented 7 years ago

OK, sounds good. We'll close this review.