New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
fhirsch opened this Issue Jul 12, 2016 · 14 comments

Comments

Projects
None yet
@fhirsch

fhirsch commented Jul 12, 2016

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

This comment has been minimized.

Member

dbaron commented Jul 30, 2016

Also see the use cases document.

@travisleithead

This comment has been minimized.

Contributor

travisleithead commented Aug 24, 2016

My review comments:

  • Great use cases--made it clear why the feature is needed and important!
  • The API is a simple Boolean property. As such, it has graceful fallback for user agents that don't intend to support it (setting the property when the property does not exist does not cause an exception to program control flow). This is a strong benefit of the current design.
  • Conversely, as a simple Boolean property, there is no ability to extend the feature (if needed). For example, if the application would like to specify some kind of timeout period, then a new property would need to be added (this is not bad per-se, just that the functionality could not be conveniently combined into a single API call). Such things are generally extended through dictionaries, and there's no way to provide a dictionary to a Boolean property.
  • Since the property only corresponds to the request for the wake lock, there is no mechanism provided to discover if the wake lock request succeeded or failed--e.g., is wake lock actually going to be applied. IMO this lack of awareness on behalf of the application is OK because (in the case that the lock cannot be applied for some reason) the app is no worse off than the current status-quo in user agents today.
  • the Lifecycle model of the wake lock is well-described.
  • Section 7 (and other relevant sections) allow large latitude for user agents to take environmental factors into play when granting or revoking a wake lock. This is appropriate.
@travisleithead

This comment has been minimized.

Contributor

travisleithead commented Aug 24, 2016

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

@torgo

This comment has been minimized.

Member

torgo commented Aug 31, 2016

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

@travisleithead

This comment has been minimized.

Contributor

travisleithead commented Aug 31, 2016

As discussed today on the call, the API is well-specified in its own right (see #126 (comment)). 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

This comment has been minimized.

Contributor

travisleithead commented Sep 1, 2016

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

@slightlyoff

This comment has been minimized.

Member

slightlyoff commented Oct 30, 2016

@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

This comment has been minimized.

dontcallmedom commented Jan 13, 2017

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

This comment has been minimized.

hadleybeeman commented Feb 8, 2017

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

This comment has been minimized.

triblondon commented Feb 8, 2017

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

This comment has been minimized.

andrey-logvinov commented Feb 9, 2017

@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

This comment has been minimized.

triblondon commented Feb 9, 2017

@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:

  • Use cases could be improved to include system lock use cases and comment on the RioRun scenario (eg describe scenarios in which developers might be tempted to use a wakelock but should not)
  • We'd like the security and privacy implications to acknowledge the information leakage risk if wakelocks are denied when battery level is low. You might want to say you're happy with that leakage but it's worth acknowledging it.
  • Specify how wakelocks interact with the user manually activating the hardware lock mechanism of the device
@tobie

This comment has been minimized.

tobie commented Feb 9, 2017

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

This comment has been minimized.

triblondon commented Feb 9, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment