Skip to content
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

Anyone implementing "system" lock? #232

Closed
marcoscaceres opened this issue Oct 8, 2019 · 25 comments · Fixed by #255
Closed

Anyone implementing "system" lock? #232

marcoscaceres opened this issue Oct 8, 2019 · 25 comments · Fixed by #255

Comments

@marcoscaceres
Copy link
Member

Just wondering if we have commitment from anyone to support it?

@kenchris
Copy link
Contributor

kenchris commented Oct 8, 2019

It is implemented in Chrome right now but we are focusing on shipping screen wake lock before. There are interest in shipping other lock types as well (maybe before) like a geolocation one. @tomayac

@marcoscaceres
Copy link
Member Author

Thanks for confirming... I've be interested to hear a bit about what Android provides to do the system wake lock.

@kenchris
Copy link
Contributor

kenchris commented Oct 8, 2019

@marcoscaceres
Copy link
Member Author

marcoscaceres commented Oct 8, 2019

Yes, thank you @kenchris... I'm worried about the CPU/System one. I don't know if we should give developers direct access to that. Seems like a bit of a footgun. I can see background sync or other services using it, but do we really want to give direct access to it? Seems like a battery killer.

@kenchris
Copy link
Contributor

kenchris commented Oct 8, 2019

True, but native apps can also use it today with no indication. When we look at shipping this or something like a more specific geolocation or, then it will require special UX or similar and we might also restrict it to just keeping the CPU alive for a little while (say max 5 min) etc.

There are definitely valid use-cases like navigation which won't work today. Android apps can even turn on the screen from a system wake lock.

Background sync cannot use it, it can be used in dedicated workers, not service workers - or at least that is the indention

@marcoscaceres
Copy link
Member Author

True, but native apps can also use it today with no indication. When we look at shipping this or something like a more specific geolocation or, then it will require special UX or similar and we might also restrict it to just keeping the CPU alive for a little while (say max 5 min) etc.

We should first explore baking that directly into the Geolocation spec tho. Like .watchPosition(success, fail, { background: true }) or something.

@marcoscaceres
Copy link
Member Author

Background sync cannot use it, it can be used in dedicated workers, not service workers - or at least that is the indention

And you help me understand why not? The spec currently says:

Though a system wake lock will usually keep the network connection alive, it is most likely not required for such. Audio streaming on devices usually does the same and if you need it for long running upload and download tasks, such as for a podcast, other more appropriate features exist, like [BACKGROUND-FETCH].

My reading from the above is that "other things handle system lock for you... don't worry about it".

@kenchris
Copy link
Contributor

kenchris commented Oct 8, 2019

Yes just like Android, certain things already implicitly run a wake lock, but those are special cases, like background downloads, and media play back.

If I am processing an image (say resize before upload etc) that won't be handled, or geolocation cases and the likes

@marcoscaceres
Copy link
Member Author

Yes just like Android, certain things already implicitly run a wake lock, but those are special cases, like background downloads, and media play back.

I guess I'm trying to tease out compelling use cases and if I can make a case within Mozilla to support these.

If I am processing an image (say resize before upload etc) that won't be handled, or geolocation cases and the likes.

Let's put the geo one aside, as geo in workers is not a thing AFAIK. The image/data processing in a worker case might be interesting tho. Have we explored having dedicated workers be granted a longer lifetime based on type?... Given that "system" locks can only be used in a Worker, and wakes locks are completely unreliable (can be released for whatever reason), I wonder if we should just have "system lock-like behavior" be a request when the worker is created.

That could hint the the UA that "this worker is long lived" and to treat it specially with a system lock for as long as possible. Just a random thought.

@kenchris
Copy link
Contributor

kenchris commented Oct 8, 2019

Workers might be long lived and page might be frozen/thawed in-between as a page can be back grounded. I see system wake lock as a way to request to finish some work before going to sleep and believe it should have a limit, like rejected (when in battery save mode) or limited to say 5 min.

I can definitely see people do image manipulation or client side image processing, heck, even our reimbursing app will analyse receipts etc (it can of course do that on cloud, but you don't always have good connectivity - if any - when travelling abroad) and my usage pattern is:

  • take pic
  • turn off screen by clicking power button

If that means that it will start sleeping before it is done, then I just end up loosing the whole receipt which is bad.

It is definitely possible to let it request a system wake lock for a few minutes and if not done then, show an ambient notification (site is keeping the phone from sleeping, [block], [ignore]). I kind of like that way. Android 10 does that with geolocation in the background and offers the following options:

image

@reillyeon
Copy link
Member

My suggestion from TPAC was that for the data processing worker case using the system wake lock API should cause the user agent to display a notification indicating that there is work in progress. The API should provide the script the ability to indicate the progress of that work so that it can be reflected in the UI.

This is a generalization of how an API like background fetch would work.

@marcoscaceres
Copy link
Member Author

My suggestion from TPAC was that for the data processing worker case using the system wake lock API should cause the user agent to display a notification indicating that there is work in progress. The API should provide the script the ability to indicate the progress of that work so that it can be reflected in the UI.

I wonder if we need the API tho... consider: when a script on the main thread is taking too long we show the "script is taking too long" alert. I wonder if we could do the same with a dedicated worker? If the user says "yeah, OK! let it work", the UA could automatically manage the system lock without any developer intervention.

@kenchris wrote:

Android 10 does that with geolocation in the background and offers the following options:

I think that's a different use case to this. Allowing Geolocation "all the time" I don't think means "keep a worker alive"? Doesn't it mean more, "periodically wake up when there is a significant location change so I can do a little related work".

@kenchris
Copy link
Contributor

kenchris commented Oct 9, 2019

I think that's a different use case to this.

I wasn't suggesting that we used the exact options, but we could model something similar

@marcoscaceres
Copy link
Member Author

I wasn't suggesting that we used the exact options, but we could model something similar

ah ok, I see what you mean (and yes, it's a good model... BUT!...).

I'm just not sure we are quite there with this: Not to get hung up on Geo, but Geolocation is somewhat special (and the UI above makes sense for it - iOS provides the same UI)... but that's the only service that gets that special treatment. I don't see "system" being in the same league as geo.

@reillyeon
Copy link
Member

My suggestion from TPAC was that for the data processing worker case using the system wake lock API should cause the user agent to display a notification indicating that there is work in progress. The API should provide the script the ability to indicate the progress of that work so that it can be reflected in the UI.

I wonder if we need the API tho... consider: when a script on the main thread is taking too long we show the "script is taking too long" alert. I wonder if we could do the same with a dedicated worker? If the user says "yeah, OK! let it work", the UA could automatically manage the system lock without any developer intervention.

I think that it is useful for the platform to provide a way for script to hint that it would like to perform a long running computation in order to inform how the UA presents that message to the user. Calling this API beforehand would allow the UA to tell the difference between long running script that is a bug and long running scripts that are intentional. This would allow it to implement mitigations, for example against draining the user's battery, without breaking sites.

@marcoscaceres
Copy link
Member Author

marcoscaceres commented Feb 27, 2020

Calling this API beforehand would allow the UA to tell the difference between long running script that is a bug and long running scripts that are intentional. This would allow it to implement mitigations, for example against draining the user's battery, without breaking sites.

I guess what we need is an understanding of the kinds of long running tasks. Questions arise like, why is this task happening on the main thread (instead of a worker)? Could the task be handed off to the browser to do in a specialized API (instead of requesting a "system" lock)? Could be two places to start.

@marcoscaceres
Copy link
Member Author

Going back to @kenchris example, pie in the sky thinking here: but, hear me out... if it's common to do image processing in the way you describe, maybe a dedicated image processing worker might be needed? ... there are dedicated audio worklets, and this could be something similar?

@reillyeon
Copy link
Member

I not sure the comparison to audio worklet (or even workers) is appropriate because this isn't about unloading the main thread.

@marcoscaceres
Copy link
Member Author

Understood (kinda)... something (a thread) needs to do "the work" though, right? I guess I'm wondering how to frame this? Sorry, I'm feeling kinda ignorant about these other locks so my questions might be silly.

@reillyeon
Copy link
Member

I think it will all be clearer once we have a use cases document.

@mitchellwills
Copy link

Are there any plans to continue the work for system wakelocks?

I would definitely like to see this happen to support applications that are doing significant amounts of computing or interacting with hardware (via WebUSB/WebSerial). Especially when interacting with hardware it can be important to try to not interrupt communications when possible (even if the tab is in the background). Additionally, if would be nice if acquiring a lock prevented the browser from performing additional throttling/freezing on the tab, but not sure if that is more of an implementation detail (may not be the right api for it either).

To add more context my specific use case is acquiring a system wake lock during the Android device flashing process on https://flash.android.com. The flashing process should ideally not be interrupted even when running on a laptop on battery. It usually takes longer than 5 minutes (re timing out the lock mentioned above).

@marcoscaceres
Copy link
Member Author

Ironically, flashing firmware is precisely the kinds of things Mozilla is most concerned about.

Having said that, an alternative approach might be to bake the system wake lock into WebSerial as an option passed to the constructor: "I need a SerialPort and it can't sleep!"

@mounirlamouri
Copy link
Member

We would have to add these options to so many APIs (Serial, USB, Bluetooth) that it would become pointless and definitely easier to simply play an audio file and make it inaudible enough for users to not notice but not too much that browsers take a system wake lock.

@reillyeon
Copy link
Member

I think we can find other examples where a system wake lock is desirable that Mozilla won't find objectionable.

The two that come to mind immediately are:

  • Ensuring that data is written to local or cloud storage before the system goes into a low power mode.
  • Performing local processing (think Squoosh or WeVideo) which would be interrupted if the system went to sleep.

The fact that developers have built workarounds for this that abuse wake locks taken incidentally to other APIs is an indication of the value of this capability however this means that the user agent is no longer able to deduce the developer's intention from the behavior of their code, leading to the complex anti-abuse mechanisms that @mounirlamouri discussed. I believe that providing an API to explicitly request these locks and express that intent will allow user agents to better control against these behaviors.

For example, at TPAC2019 I proposed that to acquire a system wake lock the application must be able to express the progress of the operation that it is performing that needs this lock. The user agent could display a notification indicating this progress and provide the user with a "stop" button which would release the lock and signal to the application that it should abort. To avoid abuse the user agent could debounce these notifications so that short tasks do not show one at all (after all, if the tasks runs more quickly than the system takes to enter a low power state then there was no need to take a wake lock at all) and to prevent the application from attempting to hide the notification by making it disappear quickly.

@tomayac
Copy link
Contributor

tomayac commented Jun 4, 2020

+1 to what Reilly wrote. I’m also a big believer in leveraging system-level indicators for signaling wake locks being active: https://blog.tomayac.com/2018/12/18/experimenting-with-the-wake-lock-api/#closing-thoughts.

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

Successfully merging a pull request may close this issue.

6 participants