-
-
Notifications
You must be signed in to change notification settings - Fork 298
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
feat: disable background throttling #1445
feat: disable background throttling #1445
Conversation
webkit only for now
Package Changes Through 80061a0There are 1 changes which include wry with patch Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a minor doc thing left
); | ||
#[cfg(any(target_os = "ios", target_os = "macos"))] | ||
{ | ||
let is_supported_os = (cfg!(target_os = "ios") && os_major_version >= 17) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could replace is_supported_os
with custom_data_store_available
from above since that has the same version requirement but i'm not sure if that's good for visibility 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can do so, but indeed, that could leave the impression that it has something to do with the custom data store.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late question, but I think it would be nice to give it more flexibility.
Because it defaults to suspend
instead of throttling
on macOS, there might be developers who want to set it to throttling
. Also, if we want to support other platforms in the future, it may require a breaking change to this property to support different policies.
Related issue: #1246 |
…tner/wry into feat/background-throttling-pr
@pewsheen @FabianLars I changed it to use a policy enum of
I tested it successfully with my app and it seems to work. But I had to add the dependency I also updated the tauri PR. |
@@ -1997,6 +2033,17 @@ pub enum PageLoadEvent { | |||
Finished, | |||
} | |||
|
|||
/// Background throttling policy | |||
#[derive(Debug, Clone)] | |||
pub enum BackgroundThrottlingPolicy { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pewsheen Should this be non_exhaustive from the start if future compat was the main concern for the enum?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be okay for now, as long as the client won't build fail after we add policies.
And Wry will build fail if we forget to handle the new policy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we go back to the boolean disableBackgroundThrottling
, like electron does. And maybe add a backgroundThrottlingPolicy
later that could be used for advanced cases. But then there'd be a dependency of the policy on the disable flag.
I would assume that almost no developer would decide for a semi-throttled/semi-unthrottled mode. In that case, they should rather focus on optimising the usage of timers in their code and keep it fully disabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we go back to the boolean
disableBackgroundThrottling
, like electron does. And maybe add abackgroundThrottlingPolicy
later that could be used for advanced cases. But then there'd be a dependency of the policy on the disable flag.I would assume that almost no developer would decide for a semi-throttled/semi-unthrottled mode. In that case, they should rather focus on optimising the usage of timers in their code and keep it fully disabled.
Because Wry is an underlying library, I prefer to give higher-level libs/applications more flexibility in designing their API.
For example, we could add disableBackgroundThrottling
in Tauri as a public API, and let tauri-wry-runtime decide which policy should be used.
@bastiankistner Cool! Thanks for your contribution! ❤️ |
Thank you for your help! |
This PR relates to tauri-apps/tauri#5250
It requires a change of tauri for which I've opened another PR: tauri-apps/tauri#12181
I've added an option
disableBackgroundThrottling
to allow developers to choose if their view should automatically be throttled / suspended by the browser after a certain time of being hidden or minimised. In the worst case, this default behaviour of browsers can lead to broken applications running in the background.For instance if a websocket connection is the only reason not to suspend a "tab" and the machine goes into sleep mode and wakes up again, the connection is often lost and the browser won't have any reason to keep the background tab active and therefore unload it, which will not allow the hidden view to "heal" itself.
This default behaviour effectively renders any browser capabilities unusable for long running background processes. One alternative might be moving logic from the frontend to the backend, but browsers have meanwhile become a great choice for local first apps and if the browser already offers the required features easily accessible, why force developers to leave a territory they feel comfortable with?
As of now, this is only available for WebKit based browsers on iOS 17.0+,iPadOS 17.0+,Mac Catalyst 17.0+, macOS 14.0+, visionOS 1.0+ ( see https://developer.apple.com/documentation/webkit/wkpreferences/inactiveschedulingpolicy-swift.enum?language=objc ).
It can be assumed that it will also be implemented for WebView2 (Windows). And it might be that it's already usable for linux and android, although I couldn't find any documentation about it and also cannot test it locally due to a lack of devices.
As an alternative for windows and non-webkit browsers, one can setup a pending WebLock transaction, which, according to the docs, can also keep the browser from suspending it.
I needed to update a snapshot test and am unsure if this is at all related to this change. Please let me know if I did something wrong here.